feat: Speed up tree navigation by returning to parent when collapsing…

[Sidnioulz]

… a non-collapsible item

Closes #9546

To address the issue, I add a new opt-in prop to useTreeState that enables the behaviour supported by Primer and the APG, and which we know our users will need (as our own tree implementation, which we want to replace with RAC Tree, currently has it).

✅ 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).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

Caution

Please note! I do not know how to run your linter, how to see the RAC Tree documentation, and there are failing tests (though I do not understand the subtleties of KeyboardDelegates and don't understand how to fix them).
I have likely forgotten some code paths in my PR.

📝 Test Instructions:

1. Run the local Storybook
2. Navigate to the new story: https://localhost:?????/?path=/story/react-aria-components-tree--nav-to-nearest-collapsible-parent
3. Kb navigate to a leaf in the tree
4. Press ArrowLeft multiple times
5. Navigate to another Tree story without the added prop
6. Repeat steps 3-4 and notice that now nothing happens

🧢 Your Project:

https://github.com/storybookjs/storybook

[snowystinger]

thanks for the PR

you can run lint and tests via

yarn lint
yarn test

To run the tests on other versions of react you should do

yarn
yarn install-16
yarn test

// once done with that version, run
make clean_all
// discard package.json and yarn.lock changes
yarn

[Sidnioulz]

yarn lint
yarn test

I had odd behaviour with yarn lint. It took several minutes to run and returned 500k errors. I also notice some type errors in files that appear to be due to unresolved dependencies. Is there a particular repo setup that's needed, with sibling repositories?

I also understand that your ESLint config does not have formatting rules, is that correct? If so, is there a formatting command or preferred IDE setup? My IDE autoformatting made a mess on my branch.

To run the tests on other versions of react you should do

yarn
yarn install-16
yarn test

// once done with that version, run
make clean_all
// discard package.json and yarn.lock changes
yarn

Thanks! I did manage to run the tests, but they fail and I'm not sure why. The story I added shows the feature to work, but I suspect the test setup is slightly different, and probably involves some layers of code that I haven't accounted for. I'll need guidance to move forward on that.

[snowystinger]

I had odd behaviour with yarn lint. It took several minutes to run and returned 500k errors. I also notice some type errors in files that appear to be due to unresolved dependencies. Is there a particular repo setup that's needed, with sibling repositories?

Nope, nothing special should be needed, you should be able to

1. clone repo
2. run yarn
3. run yarn lint

Some things that might affect it:

• Node version (we use 22 right now)
• OS (we develop on Mac's with M chips, so sometimes linux/windows issues come up)

I also understand that your ESLint config does not have formatting rules, is that correct? If so, is there a formatting command or preferred IDE setup? My IDE autoformatting made a mess on my branch.

yes, we frequently run yarn eslint packages --fix

Expected the element to have attribute:
  data-expanded="false"
Received:
  null

  849 |         expect(rows[0]).toHaveAttribute('data-expanded', 'true');
  850 |         await user.keyboard('{ArrowLeft}');
> 851 |         expect(rows[0]).toHaveAttribute('data-expanded', 'false');

This is the first failure I see on CI, it's in one of the tests you added, I'd start by looking at the activeElement.outerHTML and maybe `debug()` from the render call to make sure you're on the element you think you are. From looking at other tests in the file I think the setup should be fine, but you'll need to do some debugging.
I was specifically comparing to the test `'left and right arrows should navigate between interactive elements in the row'`

[Sidnioulz]

yes, we frequently run yarn eslint packages --fix

Oddly enough, running ESLint locally does not fix whitespace issues for me. But hey, CI told me what was missing :)

Expected the element to have attribute:
  data-expanded="false"
Received:
  null

  849 |         expect(rows[0]).toHaveAttribute('data-expanded', 'true');
  850 |         await user.keyboard('{ArrowLeft}');
> 851 |         expect(rows[0]).toHaveAttribute('data-expanded', 'false');

This is the first failure I see on CI, it's in one of the tests you added, I'd start by looking at the activeElement.outerHTML and maybe `debug()` from the render call to make sure you're on the element you think you are. From looking at other tests in the file I think the setup should be fine, but you'll need to do some debugging.

I found the issue 😬 that test was poorly written. I had to remove the useTreeState I added though, as ArrowLeft/ArrowRight seems to be replaced by Enter on the story used in tests. useTreeState is indirectly tested via the Tree test though.

[snowystinger]

Thanks for the updates

[snowystinger]

This isn't really accurate, you can still use the arrow keys to cycle through actions, it's only if the focus is on the row, it won't wrap to the end of the actions. But if you're among the actions already, then it works just fine.

[snowystinger]

Question to the team, do we want a prop for this, or should this just be the default for all trees?

[snowystinger]

what does "pressing" mean here? clicking on the arrow with your mouse, the keyboard arrow keys?
I don't really have a better suggestion yet, maybe we can just remove this section