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

[Vue] Fix: Prevent modal from closing on Escape key when closeable is false #1510

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

ChazyTheBest
Copy link
Contributor

Summary:

This PR addresses an issue where the modal (dialog element) closes when the Escape key is pressed, even if the closeable prop is set to false.

Description of the Issue:

Currently, the modal closes when the Escape key is pressed regardless of the closeable prop. This behavior occurs because the native behavior of the <dialog> element allows it to be closed by the Escape key, which is not prevented in the existing implementation. As a result, even when closeable is false, the modal's internal event handling processes the Escape key event, leading to unintended closures.

From the MDN Web Docs on the <dialog> element:

By default, a dialog invoked by the showModal() method can be dismissed by pressing the Esc key.

Because of this default behavior, the modal closes even when it is not intended to be closeable, and the internal state is not updated correctly. This also leads to issues where the modal cannot be reopened because the show state remains true, causing the watch cleanup logic not to trigger properly.

Change Details:

Updated the closeOnEscape function to include e.preventDefault() to prevent the default action of the Escape key. This ensures that the modal does not close when closeable is set to false.

Why This Fix is Necessary:

By preventing the default action of the Escape key, we ensure that the modal remains open when closeable is set to false. This fix ensures that the native behavior of the <dialog> element is overridden when necessary, providing a more consistent user experience.

Technical Explanation:

  1. Preventing Default Action: The e.preventDefault() method is used to prevent the default behavior of the Escape key, which is to close the <dialog> element. This ensures that the modal does not close unexpectedly when the Escape key is pressed and closeable is false.
  2. Internal State Management: When the Escape key is pressed and closeable is false, the modal closes due to the native behavior, but the internal state (show) is not updated. This results in the modal being visually closed but logically still open (show remains true), leading to issues where the modal cannot be reopened because the state does not trigger the watch hook.
  3. Ensuring Cleanup: By correctly handling the Escape key event, we ensure that the watch cleanup logic is triggered properly, removing event listeners and resetting styles as expected.

Impact:

This change will prevent the modal from closing when the Escape key is pressed and closeable is false. It will also ensure proper state management and cleanup, preventing unexpected closures and making the modal behavior consistent with the closeable prop.

Testing:

  • Tested the modal with closeable set to false and verified that pressing the Escape key does not close the modal.
  • Tested the modal with closeable set to true and verified that pressing the Escape key closes the modal as expected.
  • Verified that the modal can be reopened after being closed correctly.

Please review the changes and let me know if there are any questions or further improvements needed.

@taylorotwell taylorotwell merged commit 37ea36c into laravel:5.x Jul 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants