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

LeftNav componentWillReceiveProps open-state conflict #2158

Closed
rhythnic opened this issue Nov 13, 2015 · 6 comments
Closed

LeftNav componentWillReceiveProps open-state conflict #2158

rhythnic opened this issue Nov 13, 2015 · 6 comments
Labels
component: menu This is the name of the generic UI component, not the React module!

Comments

@rhythnic
Copy link

For me, the componentWillReceiveProps callback is setting state.open to true before the call to setState({open: false}) inside of the close function has had time to take effect, preventing the menu from closing when the close or toggle methods are called. I've commented out the line to set state.open inside of component will receive props as as workaround. To fix this, I think that state.open should only be set inside of componentWillReceiveProps if props.docked !== nextProps.docked.

@craig-jennings
Copy link
Contributor

I'm also having this problem. The related code change is #2044. I'm calling leftNav.close() and then routing my app causing a re-render. When componentsWillReceiveProps is called, since docked hasn't changed it sets open = this.state.open which hasn't yet been updated from the close() call. This blocks the leftNav from closing because now open has been reset to true.

@oliviertassinari
Copy link
Member

Sounds like we can do an easy fix. Do you want to give a try?

@rhythnic
Copy link
Author

If you don't want to change the API to use props.open, this should fix the bug. Pull Request
Otherwise, just ignore the pull request.

@oliviertassinari
Copy link
Member

If you don't want to change the API to use props.open

I think that It will be better to remove this imperative close method as we did with the Dialog.
Anyway, that's a regression and we have to fix it.
Thanks guys.

@alitaheri
Copy link
Member

I think this should be fixed with #2180 can you check again @rhythnic ?

@rhythnic
Copy link
Author

rhythnic commented Dec 7, 2015

@subjectix yes, it's working now.

@rhythnic rhythnic closed this as completed Dec 7, 2015
@zannager zannager added the component: menu This is the name of the generic UI component, not the React module! label Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

5 participants