Skip to content

[Popover] new captureDismiss=true prop for better uncontrolled click-to-close #2758

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Aug 3, 2018
Merged

[Popover] new captureDismiss=true prop for better uncontrolled click-to-close #2758

merged 7 commits into from
Aug 3, 2018

Conversation

giladgray
Copy link
Contributor

Fixes #2735, Fixes #2092, Fixes #2091

Changes proposed in this pull request:

  • improve Classes.POPOVER_DISMISS behavior:
    • walk up DOM tree, looking for first DISMISS or DISMISS_OVERRIDE. only dismiss if DISMISS found. this makes for very easy dismiss overriding (and I think fixes some deep issues with the old impl).
    • ignore DISMISS clicks if they come from inside a disabled element.
  • new Popover captureDismiss prop (default true) invokes preventDefault() on Classes.POPOVER_DISMISS clicks. Popover now ignores dismiss clicks if default is prevented, so Popovers around a Popover captureDismiss will not close when an inner popover is dismissed.
  • MenuItem disables captureDismiss to preserve existing (desired) behavior where clicking a nested submenu closes the entire hierarchy.
  • add nice tests for all this new behavior

@giladgray giladgray mentioned this pull request Aug 3, 2018
Closed
@blueprint-bot
Copy link

add nested popover & captureDismiss to example

Preview: documentation | landing | table

@blueprint-bot
Copy link

lint

Preview: documentation | landing | table

themadcreator
themadcreator approved these changes Aug 3, 2018
View reviewed changes
Copy link
Contributor

@themadcreator themadcreator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs and tests? yes please

@giladgray
Copy link
Contributor Author

we've got it all here!

@giladgray giladgray merged commit 95cb441 into develop Aug 3, 2018
@giladgray giladgray deleted the gg/popover-capture-dismiss branch August 3, 2018 21:32
@giladgray giladgray mentioned this pull request Aug 13, 2018
Closed
@adidahiya adidahiya mentioned this pull request Sep 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@invliD invliD Awaiting requested review from invliD

@cmslewis cmslewis Awaiting requested review from cmslewis

@adidahiya adidahiya Awaiting requested review from adidahiya

1 more reviewer

@themadcreator themadcreator themadcreator approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@giladgray @blueprint-bot @themadcreator