Reach-ui: MenuItem doesn't take react-router's Link component

Created on 11 Sep 2018  路  9Comments  路  Source: reach/reach-ui

Hey there,

I was trying out MenuButton and noticed that MenuLink doesn't take react-router's Link as component prop:

Warning: Failed prop type: Invalid prop component supplied to ForwardRef.

Is that because of what's written in the docs?

If other routers' Link component uses the React.forwardRef API, you can pass them in as well. If they don鈥檛 it won't work because we will not be able to manage focus on the element the component renders.

If so, am I missing an obvious work-around (we're not quite ready yet to go all in with reach-router)? Otherwise, would it make sense for the docs to state the component's incompatibility with the widest used router a bit more explicitly?

Thanks a lot for building this!
Nicolas


Broken example:

import { Menu, MenuButton, MenuLink, MenuList } from "@reach/menu-button";
import { Link } from "react-router-dom";

const Nav = () => (
  <Menu>
    <MenuButton>My Button</MenuButton>
    <MenuList>
      <MenuLink component={Link} to="/route">Click me!</MenuLink>
    </MenuList>
  </Menu>
);

Update: That seems to work - but might kill accessibility (?):

<MenuItem onSelect={() => history.push("/my/route")}>Click Me!</MenuItem>

Most helpful comment

Hi @nicolasschabram.

react-router's link doesn't make use of forwardRef so you're not missing something obvious, it won't work with MenuLink. I think you make a good point though about the documentation - I will submit a PR later that adds a line about react-router compatibility.

I am 99% sure your attempted workaround will kill accessibility so you shouldn't go down that route :+1:

All 9 comments

Hi @nicolasschabram.

react-router's link doesn't make use of forwardRef so you're not missing something obvious, it won't work with MenuLink. I think you make a good point though about the documentation - I will submit a PR later that adds a line about react-router compatibility.

I am 99% sure your attempted workaround will kill accessibility so you shouldn't go down that route :+1:

Wouldn't using MenuLink's onClick prop to manually push to React Router's history work? E.g.

<MenuLink
  to="/my/route"
  onClick={() => this.props.history.push('/my/route')}
>Link to url</MenuLink>

That way it's still an accessible anchor tag with an href. We intercept its click event, instead of running into trouble trying to pass React Router's <Link> as component?

I can imagine a lot of users are in @nicolasschabram's (and my) situation, and would benefit from a workaround solution while we consider migrating our projects to reach-router.

Cheers

React Router needs to use forwardRef. I'll be adding it soon.

hi @ryanflorence, any update on this?

@ryanflorence Any update?

Possible workaround (works with new Materia-UI v 4).

const NavigationLink = React.forwardRef((props, ref) => {
  return <Link {...props} ref={ref} />
})

Possible workaround (works with new Materia-UI v 4).

const NavigationLink = React.forwardRef((props, ref) => {
  return <Link {...props} ref={ref} />
})

it doesn't work with typescript.

Any update on this?

@tofful try like this:

// src/Link.tsx
import React from 'react'
import { Link, LinkProps } from '@reach/router'

type Ref = React.HTMLAttributes<HTMLAnchorElement>

const NavigationLink = React.forwardRef<Ref, LinkProps<any>>((props, ref) => {
  return <Link {...props} ref={ref as any} />
})

export default NavigationLink

@ryanflorence Any update?

I still don't see this working correctly with react-router

Was this page helpful?
0 / 5 - 0 ratings

Related issues

gdsrosa picture gdsrosa  路  3Comments

rumsky picture rumsky  路  6Comments

raunofreiberg picture raunofreiberg  路  3Comments

denkristoffer picture denkristoffer  路  5Comments

arackaf picture arackaf  路  6Comments