FE-1545: Adopt the worksheet focus layer in the left sidebar lists and search - #9420
FE-1545: Adopt the worksheet focus layer in the left sidebar lists and search#9420kube wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
area/infra
PR SummaryMedium Risk Overview Filterable lists (nodes, types, equations, parameters, entities tree) now use one Tab stop per list with roving Row menus and group “+” actions are reachable via a new The nets list follows the same pattern: arrows between Root and subnets, Enter/Space to activate, Delete/Backspace to remove a subnet, and ArrowRight to the delete button (no longer a hidden tab stop on every row). Search wraps the input and results in a vertical
User docs in Reviewed by Cursor Bugbot for commit f4a887d. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| <RowActionSlot | ||
| registerButton={registerTarget(actionTarget)} | ||
| onArrowKeyDown={onStopsKeyDown(actionTarget)} | ||
| onButtonFocus={() => onFocusTarget(actionTarget)} |
There was a problem hiding this comment.
Action focus drops list tab stop
Medium Severity
tabIndexFor is only applied to the row (column 0), but ArrowRight into a row menu or delete button records column 1 as the remembered tab stop. After that, every row gets tabIndex={-1}, and the action control is a native button that is display: none once the list loses :focus-within. Tabbing away and back therefore skips the list entirely until a click restores row focus.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 75fa218. Configure here.
| row: { item: T; isGroup: boolean }, | ||
| ) => { | ||
| event.stopPropagation(); | ||
| setFocusedIndex(index); |
There was a problem hiding this comment.
Background click steals list keyboard focus
Medium Severity
The listbox container is still click-focusable via tabIndex={-1}, but arrow handling now lives only on the rows. Clicking empty space to clear the selection focuses the container, after which ArrowUp/ArrowDown do nothing until a row is focused again.
Reviewed by Cursor Bugbot for commit 75fa218. Configure here.
75fa218 to
f4a887d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f4a887d. Configure here.
| }} | ||
| > | ||
| {children} | ||
| </span> |
There was a problem hiding this comment.
Action slots break list tab order
Medium Severity
RowActionSlot registers the trailing button as column 1 for arrow movement, but that button never receives tabIndexFor and stays a native tab stop whenever :focus-within reveals it. After ArrowRight, useFocusStops remembers column 1, so every row is tabIndex={-1} while the action is display: none once focus leaves — the list then drops out of the tab order. Tab from a focused row also lands on the menu or delete button, so the list is no longer one Tab stop.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f4a887d. Configure here.
1 participant
🌟 What is the purpose of this PR?
Stacked on #9418. Adopt the worksheet keyboard-flow layer from #9411 across the left sidebar. The sidebar carried three hand-rolled focus models: the filterable lists tracked a virtual focus index in React state over unfocusable rows with hover-only row menus, the nets list made every row and its invisible delete button a tab stop with no arrow keys, and search hopped focus between input and results through DOM queries while remounting the results on every keystroke.
🔗 Related links
🔍 What does this change?
useFocusStops: each list is one Tab stop, arrows move and select as they move, Shift+Arrow extends the range, and group headers collapse/expand with ArrowLeft/ArrowRight. The virtualfocusedIndexstate, the row-ref bookkeeping, its scroll effect, and a dead per-row key handler are deleted.RowActionSlotregisters the row's trailing button as its second column, so ArrowRight reaches it and ArrowLeft returns. The slots reveal on focus as well as hover.FocusStackat the panel level: ArrowDown in the input hands focus to the list and ArrowUp from the first result hands it back, replacing theclosest("[data-panel]")/querySelectorfocus hops. Thekey={query}remount of the results is gone, so the list no longer resets on every keystroke.useFocusStops.tabIndexForgains a fallback: when the remembered position's stop disappears (a row deleted or filtered away), the table's tab stop falls back to the first stop instead of vanishing from the tab order. Covered by a new layer test.clampIndexhelper is deleted.:focusstyling; Escape clears the selection via the list container (as before) and the editor's global Escape; the nets list's row highlight follows focus; list collapse state and click semantics (plain/Shift/Cmd click) are unchanged.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
v/h/n/t) still fire while a list row holds focus, as they did before this PR. Settling that globally is a follow-up to the focus-layer adoption.🐾 Next steps
SubViewHeader, tab bars, toolbars) is the next adoption target after this and FE-1544 (internal).🛡 What tests cover this?
filterable-list-sub-view.test.tsx: roving tabindex, select-as-you-move with Shift ranges, ArrowRight to the row menu and back, group collapse/expand with hidden children skipped.focus-flow.test.tsxfor thetabIndexForfallback after the remembered stop unmounts.focus-flow.test.tsxcovers the movement contract.❓ How to test this?
yarn dev(or the website demo), open the left sidebar.The sidebar's visuals are unchanged apart from focus highlighting; existing docs screenshots stay valid.
🤖 Generated with Claude Code