fix: focus the closest visible row when the focused Tree/Table item is collapsed by maricastroc · Pull Request #10650 · adobe/react-spectrum

maricastroc · 2026-09-25T20:10:25Z

Summary

Intent: a Tree (or a Table with tree rows) should always keep exactly one tab stop on a row the user can actually see, even when the selected or focused item is hidden inside a collapsed parent. The hidden item should stay selected.

Problem: the RAC TreeCollection and TableCollection keep collapsed descendants in their key map, so getItem() returns them and they pass canSelectItem. When such a key became the focused key (the selected item picked by autoFocus or on Tab entry, or a focused row whose parent is collapsed programmatically), no row was rendered for it:

DOM focus stayed on the collection element.
Nothing was tabbable: the collection gets tabIndex=-1 once a focused key is set, and the only row that would get tabIndex=0 isn't rendered. Tabbing out and back skipped the tree entirely.
Arrow keys navigated from the hidden item, so in the issue's example ArrowDown went to p2, skipping p1.

Approach: Tree and Table now move the focused key to the closest visible ancestor when it's hidden under a collapsed item. This reuses the closestVisibleKey logic NavigationTree already had for route focus (moved to utils so all three share it). It mirrors the existing "focused item was removed" handling and the APG treeview behavior of moving focus to a node when it collapses around the focused node. Selection is not touched, so the hidden item still shows as selected once expanded. If that ancestor is disabled, the existing disabled-item handling clears the focused key and the collection itself becomes focusable again.

Why here rather than in useSelectableCollection: that hook has no notion of expansion, and checking for a rendered element would break virtualized collections, where an off-screen selected item is legitimately not rendered yet when autoFocus runs. Doing it where the expansion state lives also covers the programmatic-collapse case, not only autoFocus and Tab entry.

Items that are being dragged are skipped, and the repair doesn't run just because a drag ended. Drag and drop keeps the focused key on the source until the drop completes so focus can follow a moved item. I checked this against #10623 (keyboard drag while collapsing the source's parent): with both changes applied, its tests pass. Without the drag guard, its assertion that focus follows the moved item fails.

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests). Unit tests only; no story changes.
Filled out test instructions.
Updated documentation (if it already exists for this component). N/A: behavior fix, no documented API changes.
Looked at the Accessibility Practices for this feature - Aria Practices
I understand every change in this PR and can explain why it's there.
If AI-assisted, I followed our AI contribution guidance and pointed my assistant at CLAUDE.md.

📝 Test Instructions:

Unit tests:

yarn jest packages/react-aria-components/test/Tree.test.tsx -t "focus on items inside collapsed items"
yarn jest packages/react-aria-components/test/Treeble.test.js -t "closest visible ancestor"

Manually, with the example from #10645 (Tree with autoFocus, selectionMode="single" and defaultSelectedKeys={['child']}, where child is inside the collapsed p1):

On mount, focus is on the p1 row, not on the tree, and ArrowDown moves to p2.
Tab out, then Shift+Tab back: focus returns to p1.
Expand p1: child is still selected.
With controlled expandedKeys, focus child, then collapse p1 from outside the tree: p1 becomes the tab stop.
Table with treeColumn: select a nested row, leave its parent collapsed and Tab into the table: the parent row is focused.

What I tested: keyboard (Tab, Shift+Tab, arrow keys), autoFocus, programmatic collapse, a disabled ancestor, a virtualized Tree (an off-screen but visible selected item is still focused directly), and keyboard drag and drop while collapsing the source's parent, both on its own and combined with #10623. Jest plus Chromium browser tests for Tree, GridList, and S2 TreeView/TableView. Not tested: screen readers, touch, RTL.

🧢 Your Project:

N/A

…s collapsed When the focused key pointed at an item under a collapsed ancestor (a selected item chosen by autoFocus or on Tab entry, or a focused row whose parent was collapsed programmatically), no row was rendered for it. DOM focus stayed on the collection, nothing in the collection was tabbable, and arrow keys navigated from the hidden item, skipping its ancestor. Tree and Table now move focus to the closest visible ancestor in that case, leaving the selection unchanged. Items that are being dragged are skipped so drag and drop keeps control of focus until the drop completes. The closestVisibleKey helper is shared with NavigationTree. Closes adobe#10645

nwidynski · 2026-09-26T21:59:34Z

Thanks for the PR! I suppose an alternative to this would be to auto-expand until the focused key is visible? Perhaps there should be an option for both behaviors?

@snowystinger Do you know whether that is something the team discussed? I suppose controlled state would require the closesVisibleKey as fallback anyways, but wanted to ask.

maricastroc · 2026-09-27T00:30:16Z

@nwidynski Thanks for taking a look! I considered auto-expanding, but it conflicts with the programmatic collapse case this PR also covers: if focus is on child and p1 is collapsed from outside the tree, expanding to keep child visible would immediately undo that collapse. And with controlled expandedKeys the tree can't expand on its own, only request it through onExpandedChange, so moving focus to the closest visible ancestor is still needed as the fallback, as you mentioned.

Auto-expanding on entry (autoFocus / Tab) could work as an opt-in on top of this, but that felt like an API decision for the team, so I kept this PR to the focus fix. Happy to follow up if you'd like it.

nwidynski · 2026-09-27T09:10:26Z

but it conflicts with the programmatic collapse case this PR also covers: if focus is on child and p1 is collapsed from outside the tree, expanding to keep child visible would immediately undo that collapse.

Not necessarily. If state was decoupled from render one could toggle expansion and it would just auto-collapse upon leaving the expanded node. I suppose that's a larger refactor though.

PS: I wonder whether that problem already exists if focusedKey is moved programmatically to a collapsed node. Will have to check if I find the time.

maricastroc · 2026-09-27T11:35:46Z

@nwidynski Fair point, a transient expansion that collapses again when focus leaves would avoid that conflict. Agreed it's a larger refactor though.

On the PS: this PR covers it. The effect runs whenever focusedKey changes, not only on collapse, so moving focusedKey programmatically to a hidden item redirects it to the closest visible ancestor. I added a test for that case in 6dc3be9.

snowystinger

In general, looks promising. I think I'm against auto expanding, it's hard to know when it's appropriate. We've discussed it internally before and the conversation hasn't really gone any definite direction.

I'll share my personal reasoning as of this time so that there's some history to refer to.

In NavigationTree, where you got the logic to find the closestVisibleKey (nice), I didn't auto expand it. That's because that component may appear in places where expanding isn't an option. See upcoming work on the S2 SidePanel.

In addition, we've found it can be difficult to emit an event changing a controlled prop like expandedKeys after initial render (in a collection it's delayed 2 renders). It's lead to some fiddly bugs. Tabs do this, emitting an onSelectionChange.

I think there's enough available to people that if they wanted to expand everything down to their first selected/focused item they could do that themselves. Given that they know their data, they could have that calculated on first render. They'd also know more if auto expanding should happen after first render/focus or only on the first "mount".

snowystinger · 2026-09-29T10:07:51Z

+    if (visibleKey !== rowKey) {
+      selectionState.setFocusedKey(visibleKey);
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps


what was omitted and why?

It leaves out dragState and state.selectionManager (selectionState in Table). Both are new objects on every render, so listing them makes the effect run on every render. For selectionManager that's harmless. For dragState it isn't: endDrag calls setDragging(false), and on that render the dragged key is no longer "dragging" but the move hasn't re-parented it yet. Running there would redirect focus to the collapsed ancestor, and focus would no longer follow the moved item. So the effect only re-runs when the collection, the expanded keys or the focused key change. useTreeState uses the same pattern for its own focused-key cleanup effect.

I checked this on top of #10623: with dragState in the deps, its keeps the drag usable until Enter test fails, with focus on projects instead of project-1.

snowystinger · 2026-09-29T10:18:36Z

+      </>
+    );
+
+    let expectSingleTabStop = (tree: HTMLElement, row: HTMLElement) => {


This seems like a bit of an implementation detail. I think it's fine to just assert that autoFocus went to the right element

Removed the helper and the tabindex assertions, here and in the Treeble tests. Two tests (disabled ancestor and drag) only caught the bug through the tabindex check, so there I assert with Tab/Shift+Tab instead. All of them still fail without the fix.

snowystinger · 2026-09-29T10:24:21Z

+      await user.keyboard('{ArrowDown}');
+      expect(document.activeElement).toBe(rows[1]);
+
+      act(() => state.selectionManager.setFocusedKey('child'));


if you use fake timers you could move this inside a component effect with a timeout and trigger it with an advance timers, that'd probably be closer to a real use case

Done. It's now a component inside the tree that calls setFocusedKey from a timeout in an effect, triggered with jest.advanceTimersByTime.

snowystinger · 2026-09-29T10:42:41Z

I haven't quite followed the dragging aspect far enough to know if there's something to worry about there. It feels like the most difficult part of this to determine if the change is ok.

maricastroc · 2026-09-29T12:01:41Z

@snowystinger
Here's the drag part in more detail.

On main, collapsing the drag source's parent during a keyboard drag ends the drag right away (#10599), so on its own this PR never reaches the drag guard. Every test on this branch passes without it. It only matters once #10623 lands and the drag survives the collapse.

During a drag, DnD keeps the focused key on the dragged item. A move keeps the key, so when the item appears at its new position, focus is already on it. There are two ways this PR could break that, and #10623's keeps the drag usable until Enter catches both (drag Project 1, collapse Projects mid-drag, drop between Projects and Reports):

Without the isDragging check, collapsing Projects makes the effect move focus to projects, and it stays there after the drop.
With the check but with dragState in the deps, the effect also runs on the endDrag render. project-1 isn't dragging anymore but hasn't moved yet, so it gets redirected. Same failure.

With the current code, both the Enter and Escape cases pass with #10623 merged in. One tradeoff: the effect doesn't re-run just because a drag ended. So a drag cancelled while the source is hidden would keep the hidden key focused until the next change. #10623 already moves focus to the nearest mounted ancestor on cancel, so the two PRs together have no gap, but that part does rely on it.

If you'd rather keep the drag interaction out of this PR, I can drop the guard here, since nothing on main reaches it. Whichever PR lands second would then add it back, with #10623's test as coverage.

maricastroc · 2026-09-29T12:03:28Z

In general, looks promising. I think I'm against auto expanding, it's hard to know when it's appropriate. We've discussed it internally before and the conversation hasn't really gone any definite direction.

I'll share my personal reasoning as of this time so that there's some history to refer to.

In NavigationTree, where you got the logic to find the closestVisibleKey (nice), I didn't auto expand it. That's because that component may appear in places where expanding isn't an option. See upcoming work on the S2 SidePanel.

In addition, we've found it can be difficult to emit an event changing a controlled prop like expandedKeys after initial render (in a collection it's delayed 2 renders). It's lead to some fiddly bugs. Tabs do this, emitting an onSelectionChange.

I think there's enough available to people that if they wanted to expand everything down to their first selected/focused item they could do that themselves. Given that they know their data, they could have that calculated on first render. They'd also know more if auto expanding should happen after first render/focus or only on the first "mount".

Thanks for writing this down. Agreed. This PR doesn't expand anything. It only moves the focused key to the closest visible row and leaves expansion to the app, so I'll keep auto-expanding out of it.

github-actions Bot added RAC Table Tree labels Sep 25, 2026

maricastroc added 2 commits September 25, 2026 17:36

chore: remove unnecessary comments

fc7bd78

test: skip the keyboard drag collapse test on React 16

eb0c886

test: cover moving the focused key to a hidden Tree item

6dc3be9

snowystinger reviewed Sep 29, 2026

View reviewed changes

test: assert focus behavior instead of tabindex in collapsed-item tests

c7e2c63

Merge branch 'main' into fix/tree-collapsed-selection-focus

9fa6db9

Conversation

maricastroc commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

nwidynski commented Sep 26, 2026

Uh oh!

maricastroc commented Sep 27, 2026

Uh oh!

nwidynski commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maricastroc commented Sep 27, 2026

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

snowystinger Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

maricastroc Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

maricastroc Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

maricastroc Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger commented Sep 29, 2026

Uh oh!

maricastroc commented Sep 29, 2026

Uh oh!

maricastroc commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maricastroc commented Sep 25, 2026 •

edited

Loading

nwidynski commented Sep 27, 2026 •

edited

Loading