[Modal] Prevent backdrop to stay open #16694
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ValentinH
changed the title
Details of bundle changes.Comparing: 4d8206d...ea9d330
|
oliviertassinari
added
bug 🐛
This should address mui#12831. After following the comments in the above issue, I indeed noticed that `onExited` was not called. However, I've also noticed that `onEntered` was not called either. This is because the transition is not even rendered. To solve this, I have added a flag that tells if the transition was entered. If this is not the case, we can remove the modal from the DOM when open is false.
|
@ValentinH Thanks for giving this issue a try! If you don't mind, I propose a different solution: |
oliviertassinari
force-pushed
the
fix-12831
branch
4 times, most recently
from
cdd2697 to
5d41805
Compare
oliviertassinari
changed the title
oliviertassinari
approved these changes
Jul 24, 2019
oliviertassinari
force-pushed
the
fix-12831
branch
from
5d41805 to
ea9d330
Compare
|
Your solution is exactly what I had in mind for the "another solution". It's also good that you added the fix for the Popper and Snackbar, thanks for this! 😊 |
|
@ValentinH I'm not aware of any downside either of the exited initially true approach :) |
|
Yes it should be fine: I think it makes sense to only set the exited state when the onExited callback is called, regardless of the open state. |
joshwooding
approved these changes
Jul 24, 2019
|
@ValentinH Thanks men, I remember seeing this issue before the hook migration, the logic was scaring, looks à bit simpler now. I really appreciate that you looked for different possible solutions. |
|
Indeed, the code looked quite straightforward even for a newcomer like me :) hooks helped a lot I think but also the clean implementation. |
This was referenced Aug 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #12831.
After following the comments in the above issue, I indeed noticed that
onExitedwas not called. However, I've also noticed thatonEnteredwas not called either. This is because the transition is not even rendered.To solve this, I have added a flag that tells if the transition was entered. If this is not the case, we can remove the modal from the DOM when open is false.
Another simpler solution would be to always set the
exitedstate totrueon mount:but I wasn't sure of what all the implications could be.