react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

Navigation hook in react-aria using Disclosure pattern

Open vbudovski opened this issue 2 years ago • 6 comments

Is there any interest in a navigation component in react-aria utilising the disclosure pattern?

🙋 Feature Request

A navigation is a common component in web apps, and none of the existing hooks meet the specific requirement. useMenu might be the closest, but isn't quite right.

My proposed implementation would consist of 2 hooks: userNavigation and useNavigationItem and utilise TreeState from react-stately. Initial attempt at implementing https://codesandbox.io/s/cranky-field-u85oox

💻 Examples

https://www.w3.org/WAI/ARIA/apg/example-index/disclosure/disclosure-navigation.html

vbudovski avatar Dec 06 '22 01:12 vbudovski

Interesting, that feels like a common enough pattern to exist in our Spectrum design system (and therefore would have a good chance having an aria hook made for it) but I'm having a hard time finding it. From a quick glance, this experience looks like a list/collection of menu triggers where list navigation could be handled via extending the existing list layout/keyboard delegate code and reusing some of our list hooks (albeit with some modifications for horizontal navigation rather than vertical). This seems roughly in line with what you've implemented, but I'd have to do some additional research to see if there is anything else needed.

LFDanLu avatar Dec 06 '22 21:12 LFDanLu

I also wonder if a useMenuBar hook would cover this use case, where the children use both useMenuItem and useMenuTrigger. And not seeing much about menus on Spectrum Headers page.

reidbarber avatar Dec 15 '22 17:12 reidbarber

@vbudovski we ran into a need for RAC navigation as well and began implementing your solution here (which is an awesome start!). Fairly new to RAC and discovered that implementation at https://codesandbox.io/s/cranky-field-u85oox is not handling click/touch events in Safari. Trying to debug and figure out why that might be, but curious if you ever put this into production and discovered a similar thing / fix for it?

nicksergeant avatar Feb 11 '24 02:02 nicksergeant

@vbudovski quick GIF demo screencast here - Safari on the left (no errors in console), Chrome on the right:

CleanShot 2024-02-10 at 21 22 11

nicksergeant avatar Feb 11 '24 02:02 nicksergeant

@nicksergeant I can't say that I've tested it much, as this was more of a proof-of-concept. You can try to update the packages to the lastest version as this sandbox was written a year ago. Recent versions of React Aria now contain the Toolbar component. You could try to use that instead of this custom hook. Perhaps some accessibility experts can chime in on how accessible that approach is. Here's a sandbox using this new approach: https://codesandbox.io/p/sandbox/intelligent-darwin-snslfn?file=%2Fsrc%2FApp.tsx

vbudovski avatar Feb 11 '24 22:02 vbudovski

@vbudovski thanks for the reply! That approach is using the Menu component (role="menu" under the hood) which should be avoided for site navigations: https://adrianroselli.com/2017/10/dont-use-aria-menu-roles-for-site-nav.html

I'll keep tinkering with your example. It's very strange, no onClick handlers are firing anywhere in the stack of Navigation components in Safari, but Chrome / FF works just fine. My guess is this is related to the (apparently many) Safari / touch bugs in RAC: https://github.com/adobe/react-spectrum/issues?q=is%3Aissue+is%3Aopen+safari+touch

nicksergeant avatar Feb 11 '24 22:02 nicksergeant