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

Added close modal on right click #961

Closed

Conversation

ZainGulbaz
Copy link

Fixes #837 .

Changes proposed:

  • Added feature to close modal on right click on overlay
  • Use props shouldCloseOnOverlayRightClick={true} to enable this feature
  • By default it is false
    -Its functionality is same as prop shouldCloseOnOverlayClick
    <Modal shouldCloseOnOverlayRightClick={true}>

Acceptance Checklist:

  • The commit message follows the guidelines in CONTRIBUTING.md.
  • Documentation (README.md) and examples have been updated as needed.
  • If this is a code change, a spec testing the functionality has been added.
  • If the commit message has [changed] or [removed], there is an upgrade path above.

@diasbruno
Copy link
Collaborator

@ZainGulbaz Looks good, but I don't think this is the right way to go...

Using a flag is nice, but later on someone will want to bind the context menu and we are going to have a flag and a callback to deal with.

In this case, it'll be better to let the user handle the event and decide want it's going to happen...

<Modal ... onOverlayContextMenu={closeModal} />

@ZainGulbaz
Copy link
Author

@diasbruno Ok, I will go with this approach.

@doeg
Copy link

doeg commented Feb 10, 2024

@diasbruno I think this PR can be closed out given that we now have #963, which implements onOverlayRightClick per your suggestion (but isn't quite merge-ready).

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.

How do I close Modal on right click in overlay?
3 participants