Keep the modal state unchanged during the close animation #100
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
Update dependencies, convert config to new formats, convert stories to CSF. Also added a story for "close on parent unmount".
Using "state == null" to indicate "not open" means that during the fade-out transition the options all revert to defaults, so you see a brief flash of the default title/content. Switched to using an explicit 'open' flag within state, so the options are preserved during the close transition. State will still be null before the first dialog is opened, but otherwise will retain the last options used.
|
Thanks for the PR! Let's have options as a separate state: // Note that we don't reset options on close, so that we render
// the same modal during close animation
const [options, setOptions] = useState({});It was like this initially, and I am pretty sure exactly for this reason, but I forgot about it during refactoring. The other state is grouped, such that we can set it to |
|
That makes a lot of sense. Do you want me to update my PR or have you already started on fixing it yourself? |
|
@DuncanMackintosh please go ahead and update the PR :) |
Clear the state immediately to avoid dangling references to resolve/reject, but keep the options separate (and untouched) so the dialog appearance stays the same during the close animation.
jonatanklosko
changed the title
|
Thanks! |
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.
I couldn't get Storybook 6 to run when I first cloned the repo, and it seemed easier to upgrade to the latest version than struggle to work out what was going wrong. Obviously if you've got a reason for keeping it at the previous reason (or just don't like the way I refactored the stories) then feel free to cherry-pick out only the bugfix commit.
The actual bug fix is fairly simple, just adding an 'open' flag to the state object and setting that to false rather than nulling the whole state when closing the dialog.