3

I have this simple code in a react component:

render(){
    return(
        <List>
            <MenuItem onClick={this.onItemClick} key={1}>Menu item 1</MenuItem>
            <MenuItem onClick={this.onItemClick} key={2}>Menu item 2</MenuItem>
            <MenuItem onClick={this.onItemClick} key={3}>Menu item 3</MenuItem>
        </List>
    )
}

onItemClick = (event) => {
    console.log(event, event.target, event.target.key); //???
}

When I click on a list item, I want to retrieve its reference to assign the selected property of that item. The event.target gives a <li> object instead of a <MenuItem> object, so event.target.key returns undefined

The material-ui docs don't give any explanation on how to handle events unfortunately.

What am I doing wrong?

Dany Dhondt
  • 881
  • 9
  • 27
  • I'm using `MenuItem` instead of `ListItem` to get the `selected` property. See [this issue](https://github.com/mui-org/material-ui/issues/8667) – Dany Dhondt May 16 '18 at 09:45
  • 1
    Its defined in the doc ! If you want to see examples, you need to go in `Demo` section. See following link : https://material-ui-next.com/demos/menus/ – LaPoule May 16 '18 at 09:48
  • Thanks Sam, I missed that one! – Dany Dhondt May 16 '18 at 10:20

3 Answers3

2

In order to get the clicked Item, you can either use arrow function or bind in render which isn't the most correct approach

<MenuItem onClick={() => this.onItemClick(1)} key={1}>Menu item 1</MenuItem>

or write a wrapper around MenuItemClick like

class ExtendedMenuItem extends React.Component {
   onClick=() => {
        this.props.onClick(this.props.id);
   }
   render(){
      const {id, children, ...others} = this.props;
      return <MenuItem onClick={this.onClick} key={id} {...others}>children</MenuItem>
   }
}

and then use it like

<ExtendedMenuItem onClick={this.onItemClick} id={1}>Menu item 1</ExtendedMenuItem >
Shubham Khatri
  • 270,417
  • 55
  • 406
  • 400
  • Indeed, this works. Thx! But does this mean that - to use a correct approach - I should always use the wrapper method? Why is the arrow function approach wrong? – Dany Dhondt May 16 '18 at 10:16
  • 1
    Using arrow function create a new function on every render, check https://stackoverflow.com/questions/45053622/how-to-avoid-binding-in-render-method/45053753#45053753 – Shubham Khatri May 16 '18 at 10:41
  • 1
    That makes sense indeed. So my conclusion would be to only use the wrapper solution in case of heavily updated components. Maybe the guys of material-ui should add some built in event handlers in components like `` and `` which takes care of these wrappers – Dany Dhondt May 16 '18 at 11:11
1

event.target gives a li element because the browser works with the DOM, not the virtual structure that React keeps in memory.

There is no MenuItem element on the rendered page. Instead, there's the elements that are rendered by MenuItems.

Click events come from actual DOM elements like div, input, li. Why not from MenuItem? Because there's no <MenuItem> on the page. When you click on the elements, you click on <li>s, not <MenuItem>s.

What you want to do is achievable by setting an "imaginary" click handler on the virtual <MenuItem>:

<VirtualElement onClick={virtualClickHandler} />

Then, when rendering the real DOM element <li>, the VirtualElement will set on it a real click handler:

class VirtualElement {
    realClickHandler (e) { // Let's assume you've bound this in the constructor
        // do stuff with "e" and "this.props"
    }
    render () {
        return <li onClick={realClickHandler} />
    }
}

Now, realClickHandler's scope contains this and e. This means, it can read both the state & (virtual) props of the VirtualElements and the event object.

The final part - the only thing left is the virtualClickHandler. Where does it reside? In this.props. Because when earlier you said <VirtualElement onClick={virtualClickHandler} />, this onClick handler gets written on the props object of the VirtualElement.

What you want to do is call the virtualClickHandler from the realClickHandler, which sees both the event and the this object:

realClickHandler (e) {
    // Here you can tell the virtualClickHandler whatever it needs to know:
    this.props.onClick({
        event: e,
        key: this.props.key
    });
}
Al.G.
  • 4,327
  • 6
  • 31
  • 56
  • Thx for this explanation. I use to use another UI library which had event handlers built in. Material-ui doesn't, so that was my mistake – Dany Dhondt May 16 '18 at 11:03
0

you can do something like that :

render(){
    return(
        <List>
            <MenuItem onClick={this.onItemClick(1)} key={1}>Menu item 1</MenuItem>
            <MenuItem onClick={this.onItemClick(2)} key={2}>Menu item 2</MenuItem>
            <MenuItem onClick={this.onItemClick(3)} key={3}>Menu item 3</MenuItem>
        </List>
    )
}

onItemClick = itemId=>(event) => {
    console.log(itemId,event, event.target, event.target.key); //???
}
benjamin Rampon
  • 1,316
  • 11
  • 18
  • Indeed, this is not correct since the `onItemClick` function will be executed during rendering. Instead, `() => this.onItemClick(1)` does work (like Khatri mentioned) – Dany Dhondt May 16 '18 at 10:14