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

Popover2 stopPropagation? #5898

Closed
synapse opened this issue Jan 29, 2023 · 3 comments
Closed

Popover2 stopPropagation? #5898

synapse opened this issue Jan 29, 2023 · 3 comments

Comments

@synapse
Copy link

synapse commented Jan 29, 2023

Environment

  • Package version(s): "@blueprintjs/popover2": "^1.10.1"
  • Browser and OS versions: Chrome Version 109.0.5414.119

Question

Is there a way to stop the event propagation when using the Popover2.

I need something like this where when I click the button would not trigger the parent click action:

<div onClick={doSomething}>
    <Popover2 content={<Something />}>
        <button>open a menu</button>
    </Popover2>
</div>

Something like this breaks the popover from opening

<div onClick={doSomething}>
    <Popover2 content={<Something />}>
        <button onClick={event => event.stopPropagation()}>open a menu</button>
    </Popover2>
</div>

This works but it's an extra step to add an extra tag everywhere

<div onClick={doSomething}>
    <span onClick={event => event.stopPropagation()}>
        <Popover2 content={<Something />}>
            <button>open a menu</button>
        </Popover2>
    </span>
</div>

Is there a better way to do something like this?

@SoulPancake
Copy link
Contributor

A work-around I had previously used was

<div onClick={event => {
  if (event.target.tagName !== 'BUTTON') {
    doSomething();
  }
}}>
    <Popover2 content={<Something />}>
        <button>open a menu</button>
    </Popover2>
</div>

@synapse
Copy link
Author

synapse commented Jan 30, 2023

Hey @SoulPancake thanks for the answer. That's what I'm using now but it's a dirty workaround and adds more useless DOM elements. Would be nice to have a prop on the popover either to stop the propagation or a callback of the click event so we can interact with it.

@adidahiya
Copy link
Contributor

@synapse I don't think @SoulPancake's solution is dirty, per se. I generally discourage our devs from using stopPropagation(), as it often has unintended consequences when interacting with other libraries on the page, and it can break interactions with Blueprint's Overlay components (dialog, popover, tooltip, context menu, etc.). So, I think it's good practice to be specific in your outer <div> click handler callback and encode logic there about exactly which events you care about (rather than relying on a more opaque stopPropagation() in a child element, which is a little harder to reason about IMO). For this reason, I don't think I'll be adding any kind of stopPropagation?: boolean prop to the popover component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants