are menu items allowed to be links?

[stefanprobst]

❔ Question

the docs state that NOTE: menu items cannot contain interactive content (e.g. buttons, checkboxes, etc.).. does that include links as well?

🔦 Context

e.g. @reach/menu-button has a MenuLink, i was expecting to be able to do something similar with react-aria, but it looks like i can only programmatically navigate in the onAction callback?

🌍 Your Environment

Software	Version(s)
react-spectrum	latest

[snowystinger]

From WAI Aria, which we use to build all of our components, you may find this section of interest. https://www.w3.org/TR/wai-aria-practices/#wai-aria-roles-states-and-properties-13
Note, each of our Aria hooks has a link to the the corresponding WAI Aria practices. This is the section about what an 'a' element in a menuitem should do https://www.w3.org/TR/html51/interactive-elements.html#using-the-a-element-to-define-a-command

So you cannot have interactive content inside a menu item, but a menu item can be other things.

In order to support something like this, we'd need to support an elementType much like we do with Button, because Reach's MenuLink renders this
<a role="menuitem" tabindex="-1" to="view" data-reach-menu-link="" data-reach-menu-item="" data-valuetext="View" id="option-1--menu--2">View</a>

[stefanprobst]

thanks, that's helpful! the "menu button" section has a "Navigation Menu Button" example, which is what i'm trying to do.

[stefanprobst]

In order to support something like this, we'd need to support an elementType much like we do with Button

is this something that you're planning to support in the future?

[snowystinger]

This is already possible to achieve in React Aria. It is not something we would likely support in React Spectrum any time soon.

[XMethues]

This is already possible to achieve in React Aria. It is not something we would likely support in React Spectrum any time soon.

Hi, I'm kind of confusing with this topic too.
So, with React Aria, the following codes are correct?

export function MenuItem<T extends object>({
    item,
    state,
}: Props<T>): ReactElement {
    const ref = React.useRef<HTMLAnchorElement>();
    const { menuItemProps } = useMenuItem(
        {
            key: item.key,
        },
        state,
        ref
    );
    const { linkProps } = useLink({} as AriaLinkOptions, ref);
    const [isFocused, setFocused] = useState(false);
    const { focusProps } = useFocus({
        onFocusChange: setFocused,
    });
    return (
        <li role="none">
            <a ref={ref} {...mergeProps(menuItemProps, focusProps, linkProps)}>
                {item.rendered}
            </a>
        </li>
    );
}

[stefanprobst]

to make this work with links as menu items, i had to still do the actual navigation programmatically with router.push in onAction, because (as far as i can tell) state.isOpen gets toggled in onPressUp, which then removes the menu popup (plus all contained menuitems), which means the default onClick action on the menuitem-link never executes (?).

would be great to get some official guidance on this in the docs, as i think a navigation menu is quite a common ui element.

[peteragarclover]

This is already possible to achieve in React Aria. It is not something we would likely support in React Spectrum any time soon.

Hi @snowystinger, thanks for looking into this for all of us. Is there any documentation you can point to that shows how react-aria supports the navigation menu button ARIA authoring practice?

As @stefanprobst mentions it looks like we would need to programmatically activate the link which seems unexpected. Is that correct?

Is @Tatamethues question about how to implement it correct?

[snowystinger]

Hey, sorry, I totally forgot to respond. Yes, @Tatamethues suggestion is correct from what I can tell. I've built it out into a codesandbox to help illustrate the idea https://codesandbox.io/s/adoring-forest-krl0p?file=/src/App.js and to see if I could tell what issue @stefanprobst was running into. I'm so far able to navigate just by relying on the anchor element doing the right thing. No problems with the menu disappearing. I tried it out in Mac Chrome/Safari/FF as well as iOS Safari.

For reference, I double checked @Tatamethues against https://www.w3.org/TR/wai-aria-practices/examples/menu-button/menu-button-links.html

[stefanprobst]

hi, thanks for your time with this!

• your codesandbox does indeed work, but it breaks when adding onClose to useMenuItem (see https://codesandbox.io/s/hopeful-joliot-jm9ge):

let { menuItemProps } = useMenuItem(
  {
    key: item.key,
    isDisabled: item.isDisabled,
    onAction,
+    # Added this, which is also in the docs example for [useMenuTrigger](https://react-spectrum.adobe.com/react-aria/useMenuTrigger.html)
+    # but not for [useMenu](https://react-spectrum.adobe.com/react-aria/useMenu.html)
+    onClose
  },
  state,
  ref
);

• my issue above had to do with not using regular <a> but with next.js's <Link> for client-side transitions. not sure if this is the same issue, i'll try to come up with a CSB example.

[stefanprobst]

see https://codesandbox.io/s/pedantic-brook-iuyph?file=/src/App.js

client-side transitions in this CRA+react-router example work when closeOnSelect: false - but then the menu stays open after nav. when setting closeOnSelect: true, no navigation happens.

[snowystinger]

Ah, you could always watch the location and just make sure the menu is closed after a change to that?
Essentially, use closeOnSelect: false and a useEffect that relies on useLocation from React Router, whenever that changes just call state.close
something along these lines? https://codesandbox.io/s/nice-engelbart-4uou9?file=/src/App.js
Otherwise handling the navigation in onAction seems reasonable.

I do see the point about it probably should navigate. Unfortunately, as you noted, they use onClick. Which means that with us using pointer/mouse/touch events, no matter when in chain we hide the menu, onClick will never get fired. See https://jsfiddle.net/snowystinger/zw081bre/3/
And we can't know if you put in a menu item that does rely on onClick.

However, and this is a very big however, you can use onClick yourself since you know that you're putting react router links into this component. As such you could build something like this: https://codesandbox.io/s/nice-engelbart-4uou9?file=/src/App.js
I give this one with caution just because I haven't thought through any of the repercussions, it may be totally fine, it may have unintended consequences.

To summarise, there are three ways we've thought of to work around the react router issue, in no particular order

1. handle the onAction and perform navigation
2. watch for location changes and close menus in response
3. make an assumption about how you're building your component and use onClick on purpose

[stefanprobst]

thanks! i've gone with the onAction option, which also allows mixing link and non-link menuitems. in case anyone want to use option 2 with next.js, closing on routeChangeStart event works 👍

[alexandernanberg]

@snowystinger Could this issue be re-opened until there is a good workaround? Programmatically navigating with onAction has quite poor UX (cmd + click for new tab and all other native features don't work).

The sandbox doesn't seem to work with the latest react-aria release, I can't "click" the items with the mouse or keyboard

[snowystinger]

Hey, thanks for the note. I checked the updated code in the sandbox and it does indeed seem to be broken. I'll reopen.