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

[TableHeaderColumn][TableRow][TableRowColumn] Remove props.key calls #3918

Merged
merged 3 commits into from
Apr 10, 2016
Merged

[TableHeaderColumn][TableRow][TableRowColumn] Remove props.key calls #3918

merged 3 commits into from
Apr 10, 2016

Conversation

jakeboone02
Copy link
Contributor

  • PR has tests / docs demo, and is linted.
  • 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.key will throw a warning in React v15 (see here).

This commit removes calls to props.key in TableHeaderColumn, TableRow, and TableRowColumn. TableRow children are given an auto-generated key, and children of TableHeaderColumn and TableRowColumn can define their own keys as needed. As far as I can tell, passing a component's key prop down to the root element of that component is unnecessary, even if a key is required for the component.

This PR is referenced here: #3913 (comment)

@mbrookes mbrookes added PR: Needs Review external dependency Blocked by external dependency, we can’t do anything about it labels Apr 9, 2016
@mbrookes
Copy link
Member

mbrookes commented Apr 9, 2016

Contributes to #3642.

@nathanmarks nathanmarks added this to the 0.15.0 Release milestone Apr 10, 2016
@nathanmarks
Copy link
Member

@mbrookes This looks good to me... however I had some issues with the complex example table demo in the docs. But it seems as though that example is even buggy on 0.14.4.

Can you confirm my findings? if so this can be merged.

@mbrookes
Copy link
Member

@nathanmarks - yes, I've noticed that before, but was too afraid to go near it! 😄

@mbrookes mbrookes merged commit 6f64e8c into mui:master Apr 10, 2016
@jakeboone02 jakeboone02 deleted the remove-props-key-calls branch April 11, 2016 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependency Blocked by external dependency, we can’t do anything about it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants