Skip to content

Commit 2fad7bf

Browse files
committed
fix(ui): scope the tree's instant hover to a row's action bar
Reaching a new row skipped the show delay whenever a hover was already up, which native never does for a list: its zero-delay window is gated on a per-delegate `instantHover` flag, and the only three places that set it are a notebook cell toolbar and two inline widgets, never a list or a tree. The delay now returns for every new target, and the exception follows the precedent instead of ignoring it: a dense cluster of targets keeps the instant handoff, which for a tree means one row's action bar. Measured in Chrome: first hover 518ms, row to row 511ms, label to a button 513ms, button to button 19ms.
1 parent e03b03d commit 2fad7bf

3 files changed

Lines changed: 50 additions & 28 deletions

File tree

packages/ui/README.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,11 @@ styling.
157157

158158
Labels hover with the node's text value, so truncated rows stay readable.
159159
Set `tooltip` for richer content or `null` to opt out. One bubble serves the
160-
whole tree, as in the native list: an invisible anchor moves to the label
161-
under the pointer, and once it is showing it follows to the next row without
162-
waiting again. Ctrl+K Ctrl+I opens the focused row's hover with no delay at
163-
all, and moving the focus closes it.
160+
whole tree, as in the native list: an invisible anchor moves to whatever the
161+
pointer reaches. Each new target waits out the show delay, except within a
162+
row's action bar, where crossing between buttons is instant, the exception
163+
native grants a dense cluster of targets. Ctrl+K Ctrl+I opens the focused
164+
row's hover with no delay at all, and moving the focus closes it.
164165

165166
## Overlays
166167

@@ -189,9 +190,8 @@ or `null` for a button that stays quiet.
189190

190191
`HoverDelegateScope` hands every `Tooltip` inside it to one shared bubble
191192
instead of a bubble each, the way a VS Code list serves its rows and their
192-
action bars from a single hover widget. `Tree` uses it, so the bubble crosses
193-
from a row's label to its action buttons and between those buttons with no
194-
second delay.
193+
action bars from a single hover widget. `Tree` uses it, which is also what
194+
lets one place decide when a hover is instant rather than delayed.
195195

196196
Overlay content is portalled to `body`, inherits webview typography from
197197
there, and shares the `.ui-overlay` base for stacking, border, shadow,

packages/ui/src/components/Tree/TreeHover.tsx

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ const GRACE_MS = 100;
2222
/** Native drops the delay to zero for a hover hidden this recently. */
2323
const INSTANT_MS = 200;
2424

25+
/** It grants that only to dense clusters of targets, such as an action bar. */
26+
const DENSE_CLUSTER = ".ui-tree-item__action";
27+
2528
/** How the keyboard chord reaches the hover. */
2629
export type TreeHoverControl = RefObject<HoverDelegate | undefined>;
2730

@@ -35,8 +38,9 @@ interface Shown extends HoverTarget {
3538
/**
3639
* One hover for the whole tree, like the native list's shared widget. Rows and
3740
* anything inside them report the element under the pointer and an invisible
38-
* anchor moves to it, so the bubble crosses from a label to an action bar
39-
* button without waiting out a second delay.
41+
* anchor moves to it. Reaching a new row waits out the show delay as it does
42+
* natively; crossing an action bar's buttons is instant, the exception native
43+
* grants a dense cluster.
4044
*/
4145
export function TreeHover({
4246
children,
@@ -51,6 +55,7 @@ export function TreeHover({
5155
const [shown, setShown] = useState<Shown>();
5256
const openRef = useRef(false);
5357
const hiddenAtRef = useRef(0);
58+
const clusterRef = useRef<Element | null>(null);
5459
const timerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
5560

5661
const hide = useCallback((): void => {
@@ -67,6 +72,7 @@ export function TreeHover({
6772
const bounds = tree.getBoundingClientRect();
6873
const rect = target.element.getBoundingClientRect();
6974
openRef.current = true;
75+
clusterRef.current = target.element.closest(DENSE_CLUSTER);
7076
setShown({
7177
...target,
7278
top: rect.top - bounds.top,
@@ -82,13 +88,19 @@ export function TreeHover({
8288
const setTarget = useCallback<HoverDelegate>(
8389
(target, immediate = false) => {
8490
clearTimeout(timerRef.current);
85-
const instant =
86-
immediate ||
87-
openRef.current ||
88-
Date.now() - hiddenAtRef.current < INSTANT_MS;
89-
if (!target?.content) timerRef.current = setTimeout(hide, GRACE_MS);
90-
else if (instant) show(target);
91-
else timerRef.current = setTimeout(() => show(target), delay);
91+
if (!target?.content) {
92+
timerRef.current = setTimeout(hide, GRACE_MS);
93+
return;
94+
}
95+
const cluster = target.element.closest(DENSE_CLUSTER);
96+
const recent =
97+
openRef.current || Date.now() - hiddenAtRef.current < INSTANT_MS;
98+
if (immediate || (recent && cluster && cluster === clusterRef.current)) {
99+
show(target);
100+
return;
101+
}
102+
hide();
103+
timerRef.current = setTimeout(() => show(target), delay);
92104
},
93105
[delay, hide, show],
94106
);

test/webview/ui/tree.rows.test.tsx

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,34 +71,44 @@ describe("Tree rows", () => {
7171
await userEvent.hover(label("Quiet"));
7272
expect(screen.queryByRole("tooltip")).toBeNull();
7373
});
74-
it("hands the same hover to a row's action bar", async () => {
74+
it("waits for a new target but crosses an action bar at once", async () => {
7575
render(
76-
<TooltipProvider delayDuration={0}>
76+
<TooltipProvider delayDuration={60}>
7777
<Tree
7878
aria-label="Handoff"
79-
selectedItemId="row"
8079
nodes={[
8180
{
8281
id: "row",
8382
label: "Workspace",
84-
action: <IconButton icon="play" label="Start workspace" />,
83+
action: (
84+
<>
85+
<IconButton icon="play" label="Start workspace" />
86+
<IconButton icon="gear" label="Workspace settings" />
87+
</>
88+
),
8589
},
8690
]}
8791
/>
8892
</TooltipProvider>,
8993
);
90-
const row = treeItem("Workspace");
91-
fireEvent.click(row);
92-
await userEvent.hover(
93-
row.getElementsByClassName("ui-tree-item__content")[0],
94-
);
94+
const label = treeItem("Workspace").getElementsByClassName(
95+
"ui-tree-item__content",
96+
)[0];
97+
fireEvent.pointerEnter(label);
9598
expect(await screen.findByRole("tooltip")).toHaveTextContent("Workspace");
96-
// The bubble is already up, so the action swaps in without a new delay.
99+
// A different kind of target, so the bubble hides and waits again.
97100
fireEvent.pointerEnter(
98101
screen.getByRole("button", { name: "Start workspace" }),
99102
);
100-
expect(screen.getAllByRole("tooltip")).toHaveLength(1);
101-
expect(screen.getByRole("tooltip")).toHaveTextContent("Start workspace");
103+
expect(screen.queryByRole("tooltip")).toBeNull();
104+
expect(await screen.findByRole("tooltip")).toHaveTextContent(
105+
"Start workspace",
106+
);
107+
// The same action bar is dense enough to skip the delay.
108+
fireEvent.pointerEnter(
109+
screen.getByRole("button", { name: "Workspace settings" }),
110+
);
111+
expect(screen.getByRole("tooltip")).toHaveTextContent("Workspace settings");
102112
});
103113
it("treats an empty children array as an expandable branch", () => {
104114
const onExpandedIdsChange = vi.fn();

0 commit comments

Comments
 (0)