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

Hide Modal on unmount #735

Closed
wants to merge 1 commit into from
Closed

Hide Modal on unmount #735

wants to merge 1 commit into from

Conversation

AndyHubert
Copy link

@AndyHubert AndyHubert commented Nov 21, 2019

Please read and mark the following check list before creating a pull request:

Short description of what this resolves:

This prevents the Modal from getting stuck open when unmounted in a visible state. Accords with Issue #718.

Closes #808
Closes #813

Accords with Issue akveo#718. This prevents the Modal from getting stuck open when unmounted in a visible state.
@artyorsh
Copy link
Collaborator

Thanks for opening PR.
Btw, did you able to test this? In my opinion, this can bring some inconsistency, for example, when visibility is handled by parent component, which means visible is passed to modal from the parent component state or maybe mobx / redux store

@artyorsh artyorsh added 📱 Components components module-specific needs review labels Nov 21, 2019
@AndyHubert
Copy link
Author

I did not run tests. I was tripped up in the PR submission process you guys outline and did not have the time to trouble-shoot. However, I did run the new code on my project, where it worked as expected.

I am not sure why the examples you share would have any effect on this. Visibilty is only handled by the visible prop sent from the parent component. And the added code is in the componentWillUnmount life cycle method and so is only called when the component has been removed from the return of the parent's render function. What would it matter how the parent component determines the visibility prop it will pass?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📱 Components components module-specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select list still showing when i navegate to a new screen Datepicker doesnt dismiss on back press on Android
2 participants