Skip to content

Commit e03b03d

Browse files
committed
refactor(ui): drop the tree's disabled rows
VS Code trees have no disabled row: TreeItem has no such field, and every aria-disabled in the workbench is an action item or a toggle, never a list row. Ours cost a guard in the policy and seven filters in the transitions, plus a CSS rule and a branch on every row, and nothing used it. A row that should read as unavailable can still be dimmed through `className`, which is how native styles one.
1 parent 6fec430 commit e03b03d

14 files changed

Lines changed: 39 additions & 132 deletions

packages/ui/README.md

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,10 @@ the row. Actions stay live on plain hover, as in the native list, and are
8989
isolated from row selection and expansion.
9090

9191
Arrow Up/Down, Home, End, PageUp/PageDown, and buffered prefix/fuzzy typing
92-
move the active row through visible rows, including disabled rows. Arrow Right
92+
move the active row through visible rows. Arrow Right
9393
expands a branch or enters it; Arrow Left collapses it or moves to its parent.
94-
Disabled rows have `aria-disabled`, remain keyboard-navigation targets, and
95-
cannot be selected or expanded.
9694

97-
`expandMode="singleClick"` is the default: clicking an enabled branch selects
95+
`expandMode="singleClick"` is the default: clicking a branch selects
9896
and toggles it, and Enter does the same. With `expandMode="doubleClick"`, a
9997
single click or Enter only selects and a double click toggles expansion. Space
10098
toggles a branch without selecting it, or selects a leaf. A normal-row twistie
@@ -111,7 +109,7 @@ can intercept shortcuts with `preventDefault()`.
111109
`aria-multiselectable`. `multiSelectModifier` chooses the toggle modifier:
112110
`"ctrlCmd"` (the default) uses Ctrl/Cmd and `"alt"` uses Alt. Shift-click and
113111
Shift+Arrow extend from the selection anchor; modifier clicks take precedence
114-
over expansion. Ctrl/Cmd+A selects enabled visible rows in the active sibling
112+
over expansion. Ctrl/Cmd+A selects the visible rows in the active sibling
115113
scope.
116114

117115
`stickyScroll` pins ancestors against the nearest scrolling ancestor. `true`

packages/ui/src/components/Tree/Tree.css

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@
5959
}
6060

6161
/* Native skips hover on selected and focused rows, keeping their outlines. */
62-
.ui-tree-item:not([aria-disabled="true"]):not([aria-selected="true"]):not(
63-
.ui-tree-item--focused
64-
)
62+
.ui-tree-item:not([aria-selected="true"]):not(.ui-tree-item--focused)
6563
> .ui-tree-item__row:hover {
6664
color: var(--ui-list-hover-foreground);
6765
background: var(--ui-list-hover-background);
@@ -81,11 +79,6 @@
8179
background: var(--ui-list-active-selection-background);
8280
}
8381

84-
.ui-tree-item[aria-disabled="true"] > .ui-tree-item__row {
85-
color: var(--ui-disabled-foreground, currentColor);
86-
cursor: default;
87-
}
88-
8982
.ui-tree-item__indent {
9083
position: absolute;
9184
inset-block: 0;
@@ -186,8 +179,7 @@
186179
}
187180

188181
@media (forced-colors: active) {
189-
.ui-tree-item:not([aria-disabled="true"]):not([aria-selected="true"])
190-
> .ui-tree-item__row:hover,
182+
.ui-tree-item:not([aria-selected="true"]) > .ui-tree-item__row:hover,
191183
.ui-tree-item[aria-selected="true"] > .ui-tree-item__row {
192184
color: HighlightText;
193185
background: Highlight;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const FILES: readonly TreeDemoNode[] = [
3232
}),
3333
node("styles", { label: "Tree.css", icon: "symbol-color" }),
3434
]),
35-
node("tests", { icon: "beaker", disabled: true }),
35+
node("tests", { icon: "beaker" }),
3636
],
3737
{ label: "src" },
3838
),
@@ -102,7 +102,6 @@ const ROW_STATES = [
102102
className: "story-row-action",
103103
action: { icon: "trash", label: "Delete item" },
104104
}),
105-
node("disabled", { label: "Disabled item", disabled: true }),
106105
];
107106
const rowStateParameters = {
108107
pseudo: { hover: [".story-row-action > .ui-tree-item__row"] },

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ function TreeRowSurface({
5050
{tooltip ? <Tooltip content={tooltip}>{label}</Tooltip> : label}
5151
{/* Native keeps a row's actions live on plain hover; CSS reveals them. */}
5252
{node.action ? (
53-
<span className="ui-tree-item__action" inert={node.disabled}>
54-
{node.action}
55-
</span>
53+
<span className="ui-tree-item__action">{node.action}</span>
5654
) : null}
5755
</div>
5856
);
@@ -91,8 +89,7 @@ export function TreeRow(props: TreeRowProps): React.JSX.Element {
9189
aria-level={row.level}
9290
aria-posinset={row.posInSet}
9391
aria-setsize={row.setSize}
94-
aria-selected={node.disabled ? undefined : selected}
95-
aria-disabled={node.disabled ? true : undefined}
92+
aria-selected={selected}
9693
aria-expanded={expanded}
9794
tabIndex={-1}
9895
className={cx(

packages/ui/src/components/Tree/treeModel.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ export type TreeNode = TreeNodeLabel & {
1919
icon?: CodiconName;
2020
/** Hover content; defaults to the text value, `null` opts out. */
2121
tooltip?: ReactNode;
22-
disabled?: boolean;
2322
action?: ReactNode;
2423
className?: string;
2524
children?: readonly TreeNode[];

packages/ui/src/components/Tree/treePolicy.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,6 @@ export function pointerCommands(
116116
input: PointerCommandInput,
117117
): readonly TreeCommand[] {
118118
const { row, source, onTwistie, modifiers } = input;
119-
if (row.node.disabled) return [];
120119
const id = row.node.id;
121120
if (isSelectionGesture(modifiers, input)) {
122121
const select = selectCommand(id, {
@@ -188,7 +187,7 @@ export function keyboardCommands(input: KeyboardCommandInput) {
188187
const selectionModifier = isSelectionModifier(modifiers, input);
189188
switch (key) {
190189
case "ArrowRight": {
191-
if (row.expanded === false && !row.node.disabled) {
190+
if (row.expanded === false) {
192191
return result([toggleCommand(id)]);
193192
}
194193
const child = row.expanded ? rows[rows.indexOf(row) + 1] : undefined;
@@ -197,14 +196,13 @@ export function keyboardCommands(input: KeyboardCommandInput) {
197196
);
198197
}
199198
case "ArrowLeft": {
200-
if (row.expanded === true && !row.node.disabled) {
199+
if (row.expanded === true) {
201200
return result([toggleCommand(id)]);
202201
}
203202
const parent = parentId(row);
204203
return result(parent ? [rowCommand("focus", parent)] : []);
205204
}
206205
case "Enter":
207-
if (row.node.disabled) return result([]);
208206
if (selectionModifier && modifiers.shiftKey) {
209207
return result([selectCommand(id, { toggle: true })]);
210208
}
@@ -215,7 +213,6 @@ export function keyboardCommands(input: KeyboardCommandInput) {
215213
: []),
216214
]);
217215
case " ":
218-
if (row.node.disabled) return result([]);
219216
return result([
220217
row.expanded === undefined
221218
? selectCommand(id, { toggle: selectionModifier })

packages/ui/src/components/Tree/treeTransition.ts

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ export function deriveTreeInteractionView(
6666
}
6767

6868
const controlledKey = selectionKey(controlledIds);
69-
const selectedIds = new Set(
70-
controlledIds.filter((id) => !rowsById.get(id)?.node.disabled),
71-
);
69+
const selectedIds = new Set(controlledIds);
7270
const focusedId =
7371
nextState.focusTarget && visibleIds.has(nextState.focusTarget.id)
7472
? nextState.focusTarget.id
@@ -145,21 +143,18 @@ function selectionRange(
145143
anchorId: string,
146144
targetId: string,
147145
): Set<string> | undefined {
148-
const enabledIds = rows
149-
.filter((row) => !row.node.disabled)
150-
.map((row) => row.node.id);
151-
const anchor = enabledIds.indexOf(anchorId);
152-
const target = enabledIds.indexOf(targetId);
146+
const rowIds = rows.map((row) => row.node.id);
147+
const anchor = rowIds.indexOf(anchorId);
148+
const target = rowIds.indexOf(targetId);
153149
if (anchor < 0 || target < 0) return undefined;
154150

155151
const ids = new Set(selectedIds);
156152
let start = anchor;
157153
let end = anchor;
158-
while (start > 0 && ids.has(enabledIds[start - 1] ?? "")) start--;
159-
while (end < enabledIds.length - 1 && ids.has(enabledIds[end + 1] ?? ""))
160-
end++;
161-
for (const id of enabledIds.slice(start, end + 1)) ids.delete(id);
162-
for (const id of enabledIds.slice(
154+
while (start > 0 && ids.has(rowIds[start - 1] ?? "")) start--;
155+
while (end < rowIds.length - 1 && ids.has(rowIds[end + 1] ?? "")) end++;
156+
for (const id of rowIds.slice(start, end + 1)) ids.delete(id);
157+
for (const id of rowIds.slice(
163158
Math.min(anchor, target),
164159
Math.max(anchor, target) + 1,
165160
)) {
@@ -176,7 +171,7 @@ function selectRow(
176171
row: TreeRowModel | undefined,
177172
{ toggle, range, preserveHidden = true }: SelectCommandOptions = {},
178173
): readonly [Set<string>, string] | undefined {
179-
if (!row || row.node.disabled) return undefined;
174+
if (!row) return undefined;
180175
const id = row.node.id;
181176
if (!multiSelect) return [new Set([id]), id];
182177

@@ -204,15 +199,12 @@ function scopedSelection(
204199
): Set<string> {
205200
const scopeId = parentId(row);
206201
const descendants = model.rows.filter(
207-
(candidate) =>
208-
!candidate.node.disabled &&
209-
(scopeId === undefined || candidate.pathIds.includes(scopeId)),
202+
(candidate) => scopeId === undefined || candidate.pathIds.includes(scopeId),
210203
);
211204
const ids = new Set(descendants.map((candidate) => candidate.node.id));
212205
const scope = scopeId ? model.rowsById.get(scopeId) : undefined;
213206
if (
214207
scope &&
215-
!scope.node.disabled &&
216208
descendants.every((candidate) => selectedIds.has(candidate.node.id))
217209
) {
218210
ids.add(scope.node.id);
@@ -230,7 +222,6 @@ function toggleBranches(
230222
const affected = recursive
231223
? model.allRows.filter(
232224
(candidate) =>
233-
!candidate.node.disabled &&
234225
candidate.node.children &&
235226
(candidate === row || candidate.pathIds.includes(row.node.id)),
236227
)
@@ -298,7 +289,7 @@ export function transitionTree(
298289
};
299290
const emit = (ids: ReadonlySet<string>, nextAnchor?: string): void => {
300291
selection = model.allRows
301-
.filter((row) => ids.has(row.node.id) && !row.node.disabled)
292+
.filter((row) => ids.has(row.node.id))
302293
.map((row) => row.node.id);
303294
selectedIds = new Set(selection);
304295
currentKey = selectionKey(selection);
@@ -354,7 +345,7 @@ export function transitionTree(
354345
break;
355346
}
356347
case "toggle":
357-
if (row && !row.node.disabled && row.expanded !== undefined) {
348+
if (row?.expanded !== undefined) {
358349
expandedIds = toggleBranches(
359350
row,
360351
model,

packages/ui/storybook/Tree.demo.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ export interface TreeDemoNode {
1212
label: string;
1313
icon?: CodiconName;
1414
action?: { icon: CodiconName; label: string };
15-
disabled?: boolean;
1615
className?: string;
1716
/** Branches start expanded unless this says otherwise. */
1817
collapsed?: boolean;
@@ -51,7 +50,6 @@ function toTreeNodes(nodes: readonly TreeDemoNode[]): readonly TreeNode[] {
5150
id: node.id,
5251
label: node.label,
5352
icon: node.icon,
54-
disabled: node.disabled,
5553
className: node.className,
5654
action: node.action && (
5755
<IconButton icon={node.action.icon} label={node.action.label} />

test/webview/ui/tree.core.test.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@ describe("Tree", () => {
6565
expect(child).toHaveAttribute("aria-selected", "true");
6666
expect(child).toHaveAttribute("aria-posinset", "1");
6767
expect(child).toHaveAttribute("aria-setsize", "2");
68-
expect(treeItem("Disabled")).toHaveAttribute("aria-disabled", "true");
69-
expect(treeItem("Disabled")).not.toHaveAttribute("aria-selected");
7068
expect(treeItem("Last")).not.toHaveAttribute("aria-expanded");
7169
expect(screen.queryByRole("group")).toBeNull();
7270
});

test/webview/ui/tree.keyboard.test.tsx

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ const NAV_NODES: readonly TreeNode[] = [
3838
id: "alpha",
3939
label: "Alpha",
4040
children: [
41-
{ id: "disabled", label: "Disabled", disabled: true },
4241
{ id: "apricot", label: "Apricot" },
4342
{ id: "amber", label: "Amber" },
4443
],
@@ -80,23 +79,17 @@ function NavTree({
8079
}
8180

8281
describe("Tree keyboard navigation", () => {
83-
it("moves through visible and disabled rows with arrows, Home, and End", () => {
82+
it("moves through visible rows with arrows, Home, and End", () => {
8483
render(<NavTree />);
8584
for (const [from, key, to] of [
86-
["Alpha", "ArrowDown", "Disabled"],
87-
["Disabled", "ArrowDown", "Apricot"],
85+
["Alpha", "ArrowDown", "Apricot"],
8886
["Apricot", "ArrowDown", "Amber"],
8987
["Amber", "End", "Bravo"],
9088
["Bravo", "Home", "Alpha"],
9189
["Alpha", "ArrowUp", "Alpha"],
9290
] as const) {
9391
press(from, key, to);
9492
}
95-
const disabled = treeItem("Disabled");
96-
act(() => disabled.focus());
97-
press("Disabled", "ArrowDown", "Apricot");
98-
act(() => disabled.focus());
99-
press("Disabled", "ArrowUp", "Alpha");
10093
});
10194
it("expands, enters, returns to, and collapses a branch", () => {
10295
const onExpandedChange = vi.fn();

0 commit comments

Comments
 (0)