tleunen / react-mdl

React Components for Material Design Lite
https://tleunen.github.io/react-mdl/
MIT License
1.76k stars 255 forks source link

MenuItems not hiding menu after onClick event #429

Open marceux opened 7 years ago

marceux commented 7 years ago

In a project I'm working on, the MenuItem component onClick behavior is inconsistent. Sometimes it hides the parent Menu and other times it leaves the Menu open.

The following MenuItem implementation closes the parent Menu when the MenuItem is clicked.

function foo(x) {
  // code
}

// dispatch comes from our react-redux implementation
const bar => (x) = dispatch(foo(x));

return (
  <Menu target="data-table-page-selector" align="right">
    <MenuItem onClick={() => bar(10)}>10</MenuItem>
    <MenuItem onClick={() => bar(25)}>25</MenuItem>
    <MenuItem onClick={() => bar(50)}>50</MenuItem>
    <MenuItem onClick={() => bar(-1)}>All</MenuItem>
  </Menu>
);

The next example DOES NOT close the parent Menu component when used.

selectOption(option) {
  this.setState({
    option,
  });
}

renderOption(option) {
  return (
    <MenuItem onClick={() => this.selectOption(option)}>{option.label}</MenuItem>
  );
}

render() {
  return (
    <Menu>
      {this.props.options.map(renderOption)}
    </Menu>
  );
}

I started browsing over the event handlers assigned to the li.mdl-menu__item element, and it appears that some file called, emptyFunction.js from the fbjs library assigns an empty function to the onClick handler. Why?

marceux commented 7 years ago

Alright, I did some further research.

In attempting to understand what the hell is going on, I tried using the explicit style in the first example for the render method in the second example and this is what I got:

render() {
    return (
        <Menu>
            <MenuItem onClick={this.selectOption}>Test 1</MenuItem>
            <MenuItem onClick={this.selectOption}>Test 2</MenuItem>
            <MenuItem onClick={this.selectOption}>Test 3</MenuItem>
        </Menu>
    );
}

This works just fine, but the options I'm rendering are not always predetermined; they can be an array of various sizes.

So, I'm starting to think: When I use .map to render <MenuItem>s from an array of options, does this circumvent material.js from registering its own event handlers to the elements that will be rendered?

Anyways, that's the best I could come up with.

Hopefully this helps other people that may have similar problems.

DirtyHairy commented 7 years ago

The issue is the same as #400 , and your explanation is correct. As a workaround, you can set a key on the menu component that changes whenever content is updated; this will force react to unmount and recreate the component. I have code ready that does this automatically, but I have to provide a PR for #426 (that depends on #421, merged today) before I can do a PR for that :smirk:

marceux commented 7 years ago

Thanks @DirtyHairy

I don't mind looking around and seeing if I can hack at a PR too, but I was wondering if you knew of any literature/blogs/anything where I can read up more on the matter.

Here are the topics I'm interested in:

Cheers :beer: