Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MenuItem] Links within Menu items #204

Closed
0x0ece opened this issue Jan 6, 2015 · 43 comments
Closed

[MenuItem] Links within Menu items #204

0x0ece opened this issue Jan 6, 2015 · 43 comments
Labels

Comments

@0x0ece
Copy link
Contributor

0x0ece commented Jan 6, 2015

Hi, I was looking at the current implementation of menu items and noticed that there is no actual link (meaning, html <a> tag). As a result, for instance, even in the navigation bar there is no actual link, and transitions to other pages are managed via onclick events.

I think having links would be an improvement, especially considering server-side rendered pages.

Do you think this make sense, or do you have specific reasons to not go in this direction?

@WRidder
Copy link
Contributor

WRidder commented Jan 6, 2015

I'm wondering about this as well. I'm using server-side rendering in my application, this is something to look into.

@tschaub
Copy link
Contributor

tschaub commented Jan 7, 2015

Using anchor elements with href attributes would also improve accessibility and allow people to use browser behavior they are accustomed to (e.g. ⌘-Click to open in a new tab).

@ZMYaro
Copy link

ZMYaro commented Jan 12, 2015

+1

4 similar comments
@ButuzGOL
Copy link
Contributor

+1

@batazor
Copy link

batazor commented Apr 8, 2015

+1

@mewben
Copy link

mewben commented Apr 28, 2015

+1

@batazor
Copy link

batazor commented May 1, 2015

+1

@hai-cea hai-cea changed the title Links within Menu items [MenuItem] Links within Menu items Jun 18, 2015
@nikhildaga
Copy link

+1

1 similar comment
@rhythnic
Copy link

rhythnic commented Nov 3, 2015

+1

@shaurya947
Copy link
Contributor

@ecesena do you think this would still be an issue with the new MenuItem component? See here. The MenuItems can be surrounded by <a>..</a>, no?

@alitaheri
Copy link
Member

this is possible with what @shaurya947 suggested.

@inkorange
Copy link

I'm attempting to use MenuItems in my LeftNav like this:

        let menuItems = (
            <div>
                <MenuItem primaryText="Doors"           leftIcon={<FontIcon className="material-icons" color={GlobalStyles.default.activeColor}>home</FontIcon>} /></a>
                <MenuItem primaryText="Load"           leftIcon={<FontIcon className="material-icons" color={GlobalStyles.default.activeColor}>home</FontIcon>} />
                <MenuItem primaryText="Notes"           leftIcon={<FontIcon className="material-icons" color={GlobalStyles.default.activeColor}>home</FontIcon>} />
                <MenuItem primaryText="Alerts"          leftIcon={<FontIcon className="material-icons" color={GlobalStyles.default.activeColor}>home</FontIcon>} />
                <MenuItem primaryText="Admin"           leftIcon={<FontIcon className="material-icons" color={GlobalStyles.default.activeColor}>home</FontIcon>} />
            </div>
        );

        return (
            <LeftNav
                ref="leftNav"
                docked={false}
                onChange={this._onLeftNavChange}
            >
                {menuItems}
            </LeftNav>
        )

Wrapping a <a> around a <MenuItem> tag throws a warning validateDOMNesting(...): <a> cannot appear as a descendant of <a>. See MainLayout > a > ... > MenuItem > ListItem > EnhancedButton > a"

If you do a React inspection of the output you get the following.

<a href="/">
    <MenuItem..>
        <ListItem..>
            <div>
                <EnhancedButton..>
                    <a>
                        <TouchRipple ..>
                    </a>
                </EnhancedButton>
            </div>
        </ListItem>
    </MenuItem>
</a>

which having nested <a> elements is offensive.

<MenuItems> generate <a> elements within the Button declaration. What prop is used from MenuItem to populate the href attribute within the generated <EnhancedButton>?

Using a MenuItem object as a prop for LeftNav gets the linking working, but I have not been able to figure out how to add left/right Icons this way.

ie:

let menuItems = [
         { route: '/load_areas/'+ this.state.area + '/doors', text: 'Doors' },
         { route: '/load_areas/'+ this.state.area + '/trailer_loads', text: 'Load' },
         { route: 'notes', text: 'Notes' },
         { route: 'alerts', text: 'Alerts' },
         { route: 'admin', text: 'Admin' }
         ];

<LeftNav
                ref="leftNav"
                docked={false}
                menuItems={menuItems}
                onChange={this._onLeftNavChange}
            />

@alitaheri
Copy link
Member

this is a separate issue, please open one, so we can label it and add it to milestones, tnx 😁

@DaxChen
Copy link

DaxChen commented Dec 29, 2015

Use the containerElement prop!

see:
http://stackoverflow.com/questions/32106513/material-ui-menu-using-routes/34507786#34507786

<MenuItem
  containerElement={<Link to="/profile" />}
  primaryText="Profile"
  leftIcon={
    <FontIcon className="material-icons">people</FontIcon>
  } />

@nodejh
Copy link

nodejh commented Sep 12, 2016

Now the prop linkButton of EnhancedButton is deprecated. LinkButton is no longer required when the href property is provided. It will be removed with v0.16.0.

For example:

<MenuItem primaryText="Primary Text" href="/your/link" />

@cezarneaga
Copy link

href reloads page. how about if i want to use react router

@Oduig
Copy link

Oduig commented Nov 5, 2016

@cezarneaga try href="#/your/link"

@cezarneaga
Copy link

<MenuItem
  containerElement={<Link to="/profile" />}
.../>

documentation mentions you need to enter also linkButton. if you remove this, it works without errors.

@Faolain
Copy link

Faolain commented Dec 25, 2016

@DaxChen I saw your stackoverflow updated answer for Dec 2016(Extremely Recent haha) and I was wondering if you knew if this worked for a MenuItem within a Drawer? I'm trying the following below and have been totally unable to get Link working with the MenuItem as containerElement seems to do nothing.


<Drawer
  docked={false}
  width={300}
  onRequestChange={this.closeDrawer}
  open={this.state.open}>
  <AppBar title="Title"
 />
  <MenuItem primaryText="home" containerElement={<Link to="/home" />} />
</Drawer>

@kgregory
Copy link
Member

kgregory commented Feb 9, 2017

containerElement works but isn't documented, so I guess this thread will have to serve as the official documentation for now. Thanks!

@DaxChen
Copy link

DaxChen commented Feb 12, 2017

Hi @Faolain !

Really sorry for the late reply...
I tried you code on an clean app with create-react-app, but everything seemed working okay?!
Maybe you configured react-router differently, or is the version different?

Anyway, here's an Example Repo, and the Live Demo Here.

If you still can't find the cause of your problem, can you provide a sample repo to reproduce?

@janzenz
Copy link

janzenz commented Apr 14, 2017

None of the solutions above worked on Safari and iOS. So here's what I did as a work around for react-router

          <MenuItem
            onTouchTap={() => {
              this._yourMethod()
              browserHistory.push('/howItWorks')
            }}
            primaryText="Menu Link"
          />

@DaxChen
Copy link

DaxChen commented Apr 16, 2017

@janzenz is this example here not working in your browser?

I tried it in Safari on macOS and Safari on iOS and they all work. Is this not working for you?

Although your workaround works, it may cause bad SEO and you'll lose some native behavior such as link-preview and option-click to open in new tab.

@janzenz
Copy link

janzenz commented Apr 16, 2017

@DaxChen thanks for the insights. I'm not sure if my issue is related with your first link. I've already tried the 2nd one but once I add a onTouchTap prop the <Link /> doesn't work anymore.

@DaxChen
Copy link

DaxChen commented Apr 16, 2017

@janzenz I've added onTouchTap prop in my previous example and it still works.
Don't know if your setting is different somewhere or something... 😢
You can check out the updated code and see if it works for you?

@janzenz
Copy link

janzenz commented Apr 17, 2017

@DaxChen I see you're using material-ui at version ~0.16.0. Have you tried the latest ~0.17.0 and see if that is still not a problem?

@DaxChen
Copy link

DaxChen commented Apr 17, 2017

@janzenz I was using an older version, just updated to the latest 0.17.4 and it still works!

@janzenz
Copy link

janzenz commented Apr 17, 2017

Touché! Probably my setup then. Thanks for confirming that @DaxChen I'll get back to this thread if I have found the real issue.

@hhimanshu
Copy link

@DaxChen , I tried the example you gave me, but it fails

Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in. Check the render method of `EnhancedButton`.
    at invariant (invariant.js:44)
    at ReactCompositeComponentWrapper.instantiateReactComponent [as _instantiateReactComponent] (instantiateReactComponent.js:74)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:367)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactDOMComponent.mountChildren (ReactMultiChild.js:238)
    at ReactDOMComponent._createInitialChildren (ReactDOMComponent.js:697)
    at ReactDOMComponent.mountComponent (ReactDOMComponent.js:516)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactDOMComponent.mountChildren (ReactMultiChild.js:238)
    at ReactDOMComponent._createInitialChildren (ReactDOMComponent.js:697)
    at ReactDOMComponent.mountComponent (ReactDOMComponent.js:516)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactDOMComponent.mountChildren (ReactMultiChild.js:238)
    at ReactDOMComponent._createInitialChildren (ReactDOMComponent.js:697)
    at ReactDOMComponent.mountComponent (ReactDOMComponent.js:516)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactDOMComponent.mountChildren (ReactMultiChild.js:238)
    at ReactDOMComponent._createInitialChildren (ReactDOMComponent.js:697)
    at ReactDOMComponent.mountComponent (ReactDOMComponent.js:516)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at ReactCompositeComponentWrapper.performInitialMount (ReactCompositeComponent.js:371)
    at ReactCompositeComponentWrapper.mountComponent (ReactCompositeComponent.js:258)
    at Object.mountComponent (ReactReconciler.js:46)
    at mountComponentIntoNode (ReactMount.js:104)
    at ReactReconcileTransaction.perform (Transaction.js:140)
    at batchedMountComponentIntoNode (ReactMount.js:126)
    at ReactDefaultBatchingStrategyTransaction.perform (Transaction.js:140)
    at Object.batchedUpdates (ReactDefaultBatchingStrategy.js:62)
    at Object.batchedUpdates (ReactUpdates.js:97)
    at Object._renderNewRootComponent (ReactMount.js:320)
    at Object._renderSubtreeIntoContainer (ReactMount.js:401)
    at Object.render (ReactMount.js:422)
    at Object.<anonymous> (index.js:36)
    at __webpack_require__ (bootstrap e3e2367…:555)
    at fn (bootstrap e3e2367…:86)
    at Object.<anonymous> (bootstrap e3e2367…:578)
    at __webpack_require__ (bootstrap e3e2367…:555)
    at bootstrap e3e2367…:578
    at bootstrap e3e2367…:578

My code is available at https://github.com/hhimanshu/spicyveggie/tree/cards (cards branch)

I am not sure what is different, but it just does not work

@janzenz
I also tried browserHistory.push('/howItWorks') and imported import browserHistory from 'react-router-dom', but got undefuned.

@Oduig,
I tried #/your/link, but it does not take me anywhere
I tried /your/link, but it reloads tha page and I lose Drawer from Material-UI

None of these things worked for me. My code is available at https://github.com/hhimanshu/spicyveggie/tree/cards (cards branch), specific commit is hhimanshu/spicyveggie@844b7b7, Could someone please help?

@DaxChen
Copy link

DaxChen commented Apr 24, 2017

@hhimanshu change this line to

import { Link } from 'react-router-dom'

more info on MDN

@hhimanshu
Copy link

@DaxChen, thanks. That was it. However, not when my page loads with {<Link to="/menu"/>}, it loads the entire page and my Drawer is gone.

screen shot 2017-04-24 at 1 54 23 pm

@DaxChen
Copy link

DaxChen commented Apr 24, 2017

@hhimanshu that's because you used only the Menu component in your \menu route, so App won't be rendered.

Nest your Menu and Summary inside App or some layout components, and pass in respective children.

You can use react-devtools to check the rendered component hierarchy and its states!

This is not related to material-ui, please read more in react-router's tutorials/docs.

@hhimanshu
Copy link

@IveTou
Copy link

IveTou commented Dec 9, 2017

+1

@xemasiv
Copy link

xemasiv commented Dec 10, 2017

Workarounds for links HANDLED & NOT HANDLED by React Router.


If target link is handled by React Router

MenuItemLink.js

import React from 'react';
import { MenuItem } from 'material-ui/Menu';
import { Route } from 'react-router-dom';

class MenuItemLink extends React.Component {
  render() {
    const {
      to, also,
      ...rest
    } = this.props;

    return (
      <Route
        render={({ history, location }) => (
          <MenuItem
            onClick={() => {
              history.push(to);
              if (typeof also === 'function') {
                also();
              }
            }}
            {...rest}
          />
        )}
      />
    );
  }
}
MenuItemLink.muiName = 'MenuItem';
export default MenuItemLink;

Sample Code:

import MenuItemLink from './MenuItemLink';
<Menu ... >
        <MenuItemLink to="/users"
          also={this.handleRequestClose}>Users</MenuItemLink>
</Menu>

If target link is NOT handled by React Router, ie: request is forwarded to an API server or whatever

onClick function:

    function facebookLoginRedirect () {
      if (window) window.location.href = "/api/auth/facebook"
      return true;
    }

Sample Code:

<MenuItem onClick={ facebookLoginRedirect }>Login with Facebook</MenuItem>

@mririgoyen
Copy link

I'm coming to add to this, in case someone else stumbled upon it as well.

On v3.4, you can achieve this like so:

import { Link } from 'react-router-dom';
import MenuItem from '@material-ui/core/MenuItem';

...

<MenuItem component={Link} to="/your-path">...</MenuItem>

@eladlevy
Copy link

eladlevy commented Mar 1, 2019

The only solution that worked for me without breaking the Menu layout was:

<MenuList>
  <Link to='/your-path' style={{ textDecoration: 'none' }}>
    <MenuItem>
      Notifications
     </MenuItem>
  </Link>
  <Link to='/your-path' style={{ textDecoration: 'none' }}>
    <MenuItem>
      Profile
     </MenuItem>
  </Link>
</MenuList>

@NaderIkladious
Copy link

The only solution that worked for me without breaking the Menu layout was:

<MenuList>
  <Link to='/your-path' style={{ textDecoration: 'none' }}>
    <MenuItem>
      Notifications
     </MenuItem>
  </Link>
  <Link to='/your-path' style={{ textDecoration: 'none' }}>
    <MenuItem>
      Profile
     </MenuItem>
  </Link>
</MenuList>

@eladlevy This would end up in a poor semantics HTML.

<ul>
  <a><li/></a>
  <a><li/></a>
</ul>

@MarkLeMerise
Copy link

Leaving this here in case it helps anyone (running v3.9.0). I needed a <Select> to act as a menu of react-router-dom <Link>s. Each link corresponded to a language setting, so the dropdown would display the value of the current language from the query parameter.

<Select value={currentLanguage}>
  {languages.map(language => (
    <ListItem
      button
      component={btnProps => (
        <Link
          to={ `/things?lang=${ language }` }
          {...btnProps as any}
        />
      )}
      value={language}
      key={language}
    >
      {language}
    </ListItem>
  ))}
</Select>

This was the cleanest solution I could find that required minimal TypeScript casting and maintained visuals consistent with Material UI.

@elibenjii
Copy link

@goyney has the solution, thanks

@MelMacaluso
Copy link
Contributor

I'm coming to add to this, in case someone else stumbled upon it as well.

On v3.4, you can achieve this like so:

import { Link } from 'react-router-dom';
import MenuItem from '@material-ui/core/MenuItem';

...

<MenuItem component={Link} to="/your-path">...</MenuItem>

Working and clean solution, thanks ❤️

@jtreminio
Copy link

jtreminio commented Oct 23, 2019

For external links you'll want to use component="a", but also surround the MenuItem in <li>:

<MenuList>
    <li>
        <MenuItem
            component="a"
            href="https://google.com"
            target="_blank"
        >Google</MenuItem>
    </li>
</MenuList>

This generates the following HTML:

<ul class="MuiList-root MuiList-padding" role="menu" tabindex="-1">
    <li>
        <a class="[...]" tabindex="-1" aria-disabled="false" role="menuitem" href="https://google.com" target="_blank">Google</a>
    </li>
</ul>

@stliang
Copy link

stliang commented Feb 28, 2020

Suppose I have a popup menu and use

import { Link } from 'react-router-dom';
import MenuItem from '@material-ui/core/MenuItem';

<MenuItem component={Link} to="/your-path"></MenuItem>

The Link will bring up the /your-path page, how would one close the popup menu?

@mui mui locked as resolved and limited conversation to collaborators Feb 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests