-
Notifications
You must be signed in to change notification settings - Fork 377
fix(nav): Allow nav to expand by prop change #1345
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
Conversation
2513c06 to
ca68e53
Compare
|
PatternFly-React preview: https://1345-pr-patternfly-react-patternfly.surge.sh |
Pull Request Test Coverage Report for Build 4370
💛 - Coveralls |
dlabaj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
tlabaj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@ibolton336, this fix appears to have introduced a bug where previously expanded sections that have been collapsed are re-expanded on next navigate. cc: @spadgett, @janwright73 |
| value: this.props.defaultValue | ||
| }; | ||
|
|
||
| componentWillReceiveProps(nextProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this lifecycle method is soon to be deprecated. It works with our target version of React, but I recommend looking at getDerivedStateFromProps and componentDidUpdate.
https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops
https://egghead.io/lessons/react-refactor-componentwillreceiveprops-to-getderivedstatefromprops-in-react-16-3

What: fixes #1016
Additional issues: