-
Notifications
You must be signed in to change notification settings - Fork 169
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
Portal reopens if parent component sets its state immediately after .closePortal() #91
Conversation
I believe I'm also running into this bug, but only in IE - not sure if React uses a slightly different rendering strategy there. @bevesce are you seeing it in all browsers? [EDIT: scrap that, I do see it in all browsers. The IE behaviour I'm seeing is caused by something else] |
This is interesting, thanks! I can confirm the weird behaviour. I think you are right with the explanation. I don't see a problem with replacing If not, can you please rebase this PR so it's mergeable? Thanks. |
I've resolved conflicts |
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.
Nuke the state in the constructor and this LGTM
https://github.com/tajo/react-portal/pull/91/files#diff-247298a69e22df733d1aac4bffc9fb06R12
Done |
This solution works for me too! However, it solves a different issue than #124. |
Hey, please check the new major version (complete rewrite) of react-portal: #157 It's React v16 only since its uses the new official Portal API. There is the first beta released and I would like to get your feedback. I don't have bandwidth to maintain v3 which is very different and full of hacks. If you feel your PR still applies, please rebase it against the master and re-open it. Sorry for the lack of response in past! Thanks! |
Minimal example:
Expected behaviour:
NotClosingPortalContainer
displays counter and button to open portalWhat actually happens:
I think this happens:
Portal#closePortal
this.setState({ active: false })
is called, but it works asynchronouslyNotClosingPortalContainer#handleClick
inthis.setState(...)
is calledNotClosingPortalContainer
rerenders beforePortal#state.active
actual value is set tofalse
Portal
receives new props and opens portal (becausePortal#state.active
still equalstrue
)I fixed this by basically replacing
Portal#state.active
withPortal#active
but it feels dirty. I'm pretty new to React so maybe there is a better solution?Tests are passing.