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

Ensure calls to @close run before the focus-out deactivation #596

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

pichfl
Copy link
Contributor

@pichfl pichfl commented Apr 11, 2022

This wasn't covered by our test suite before.

Expected behavior: Calling @close on a modal will close the modal and in turn should resolve the modal promise with any arguments passed to that function.

Actual behavior: Closing the modal also triggers focus-out, which in turn also calls the modal close callback but without any return data, racing the manual call to @close, which means the modal promise is resolved with undefined because the onDeactivate hook of focus-out runs before the modal close.

This happens because focus-out changed when it's onDeactivate hook is triggered, introducing a new onPostDeactivate which has the same timing as the old one.

@pichfl pichfl added the bug Something isn't working label Apr 11, 2022
This wasn't covered by our test suite before.
@pichfl pichfl merged commit 19b0085 into main Apr 12, 2022
@delete-merged-branch delete-merged-branch bot deleted the bugfix/results branch April 12, 2022 09:48
@pichfl pichfl changed the title Ensure calls to @close run before the focus-out deactivation Ensure calls to @close runs before the focus-out deactivation Apr 12, 2022
@pichfl pichfl changed the title Ensure calls to @close runs before the focus-out deactivation Ensure calls to @close run before the focus-out deactivation Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants