[IconMenu] Removed props.ref call

[jakeboone02]

• PR has tests / docs demo, and is linted. (Feature had no demo before, so none was created for this PR)
• Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
• Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

Calls to props.ref will throw a warning in React v15.

This commit adds a prop to IconMenu called iconButtonRef that, if
provided, will be passed down to the child IconButton element as its ref.
The iconButtonRef element has been removed from the state object, since it
never changes after the component is initialized.

The new React warnings are discussed in this PR:
facebook/react#5744

[mbrookes]

This is the relevant note: https://fb.me/react-special-props

[jakeboone02]

Thanks, @mbrookes, I meant to add that link, too, but forgot. Should I look at removing the calls to props.key as well (in a new PR)? There are three of them: one each in TableHeaderColumn, TableRow, and TableRowColumn.

[jakeboone02]

Sorry for not waiting very long for a response, but #3918 was such a simple change I went ahead and did it.

[mbrookes]

Contributes to #3642.

[nathanmarks]

@jakeboone02 quick Q:

Would this break any functionality where iconButtonRef is used if someone was previously relying on it? I'm looking at the close(reason, isKeyboard) function right now and was wondering...

We obviously need to get this compatible with react 15, I just want to be aware of anything that we're breaking so we can decide what to do about it.

[jakeboone02]

I did a little more testing and from what I can tell, the feature never worked in the first place. If you pass ref="whatever" down to an IconMenu component, then access that component through this.refs.whatever and call on that component's refs, you still get 'iconButton' as the ref for the button. For example:

class IconMenuExampleSimple extends React.Component {
  componentDidMount() {
    console.log(this.refs.whatever.refs);
  }

  render() {
    return (
      <div>
        <IconMenu
          ref="whatever"
          iconButtonElement={<IconButton><MoreVertIcon /></IconButton>}
          anchorOrigin={{horizontal: 'left', vertical: 'top'}}
          targetOrigin={{horizontal: 'left', vertical: 'top'}}
        >
          <MenuItem primaryText="Refresh" />
...

logs this Object:

{
  iconButton: IconButton,
  iconMenuContainer: div
}

With the new code in place, passing iconButtonRef="whatever" results in this object:

{
  whatever: IconButton,
  iconMenuContainer: div
}

I suppose you could document it now that it works, but I don't think you need to include it in the list of breaking changes in 0.15.0.

[nathanmarks]

@jakeboone02 thanks 👍

[nathanmarks]

@mbrookes another one needed for react 15

[oliviertassinari]

I did a little more testing and from what I can tell, the feature never worked in the first place.

In that case, I don't think that we should expose an API to change the ref.
Relying on anything else that the property API is often a bad idea an go against the React paradigme.
What do you think about removing iconButtonRef?

[jakeboone02]

Yeah, I was thinking about that this morning and I agree. The only reason the ref is there is to "// Set focus on the icon button when the menu close" in the close(reason, isKeyboard) method. I can't imagine anyone actually needing to rename it, so I'll remove the option to do so.

[nathanmarks]

@jakeboone02 if you're around, can you squash?

[jakeboone02]

Sure. Never done a squash before so cross your fingers. ;)

[nathanmarks]

@jakeboone02 funnily enough I just got this error while booting up with react 15 😄

https://dl.dropboxusercontent.com/s/jypgxsv48cemif2/2016-04-11%20at%207.04%20PM.png