React Router Link inside `MenuButton`

[OliverJAsh]

I am trying to create a menu whereby each item is a link. The reason I want a link rather than a button:

• accessibility, e.g. right click -> open in new tab
• semantics, links are important for SEO

I understand that MenuButton supports a tag prop, so you could do something like <MenuButton tag="a" href="…" />, however in this case we need to use React Router's Link component rather than the a element. This is because React Router's Link component has special behaviour, such as calling history.pushState when the link is clicked (as opposed to performing a full page navigation).

For that reason, I tried the following:

• nest a Link inside of a MenuButton
• implement Wrapper's onSelection to run history.push, so it behaves the same as the link

Example: https://react-ts-bkz4fo.stackblitz.io/
Code: https://stackblitz.com/edit/react-ts-bkz4fo

One downside to this is we have to repeat the link in both Wrapper's onSelection and Link. But more importantly, doing this means that on click, two events handlers would be called rather than one: Link's onClick (which calls history.push) and Wrapper's onSelection. This in turn means history.push is called twice, resulting in an extraneous history entry.

If you select an item from the menu, you'll see that the history contains double entries.

To workaround this, my first thought was to remove the Wrapper's onSelection and just use Link's onClick, however this means the link would not be actioned when navigating the menu via the keyboard.

Another idea I had was to preventDefault inside the Link's onClick. This fixes the issue of "two event handlers are called" whilst preserving the semantics/a11y of a link, except that command + click (shortcut to open in new tab) will not work.

Reach UI’s MenuButton solves this problem by providing a MenuLink that works just like a MenuItem except you can pass a custom link component to use instead of a: https://reacttraining.com/reach-ui/menu-button/#menulink-as. Furthermore, there is no need to implement a selection handler on the menu wrapper. Perhaps this library could do something similar?

Related: #85

[shirshendubhowmick]

Thanks for sharing, do you have any action plan on this ? Even if we add this functionality I would prefer not to make any breaking changes here.

[OliverJAsh]

Thanks for sharing, do you have any action plan on this ?

Unfortunately I won't have the time to look into fixing this, but if anyone does pick this up, I would like to refer them to my suggestion at the end of my original post.

At some point we might switch to Reach UI. If we do, I can report back on how successful that approach is and any trade offs it brings with it.

[OliverJAsh]

For reference, we ended up working around this by:

• avoiding Wrapper's onSelection prop
• passing tag="a" to MenuItem along with an onClick that behaves just like Link