Fix elementParentNode class list re-render issue in 1.x #216
Conversation
| @@ -94,6 +94,20 @@ class TetherComponent extends Component { | |||
| if (this._targetComponent && this._elementComponent) { | |||
| this._createContainer(); | |||
| } | |||
|
|
|||
| // Verify if className props have changed | |||
| if (this._elementParentNode && this.props.className !== props.className) { | |||
There was a problem hiding this comment.
Copied over from the other commit in (#211)
This is in the componentWillUpdate to avoid refactoring _update() and closely mimics the other PR even though that one uses componentDidUpdate.
|
strange, the component test passed before and still does locally |
|
Yeah, really strange. I'm not sure what's up |
|
Edit: this was not the issue |
|
oh i know, its the enzyme issue with react-test-renderer caused by the It should have been caught here: #215 Basically the |
Coverage in All Files
Generated by 🚫 dangerJS |
|
Yep, short of forking the repo, we may want to publish these changes as a thanks for taking the time to merge these changes though, will definitely help us clean up all those warnings we are getting |
|
@j3tan I think I'm happy with this last change. Thoughts? I really don't want to break compatibility |
|
Fix looks good to me. ship it! |
| @@ -286,4 +291,18 @@ class TetherComponent extends Component { | |||
| } | |||
| } | |||
|
|
|||
| function componentWillUpdate(nextProps) { | |||
There was a problem hiding this comment.
wish we could use componentDidUpdate but I don't think it's viable for React 15.x due to the way it'll render. We may want to test it, but I think this solution works for now
|
Do you plan on publishing a minor version? |
|
Published in 1.0.5 |
Fixing re-render issue caused by a too-simplistic
classNamecheck. The tether library adds its own classes to the tether parent node so it always mismatches the prop valuefixes #201