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

the submenu items onclick are not triggered after click the submenu itself #3010

Closed
gasolin opened this issue Oct 9, 2018 · 15 comments · Fixed by #3716
Closed

the submenu items onclick are not triggered after click the submenu itself #3010

gasolin opened this issue Oct 9, 2018 · 15 comments · Fixed by #3716

Comments

@gasolin
Copy link

gasolin commented Oct 9, 2018

Environment

  • Package version(s): 3.6.1
  • Browser and OS versions: Chromium 69

Steps to reproduce

submenu > sub item 1
          sub item 2

The normal usage is hover on submenu and click sub item 1 to trigger sub item 1 onClick as above.

  1. when submenu popup is shown as above, click the submenu item itself. the onClick of submenu itself is triggered
  2. then click the sub item 1 or sub item 2

Actual behavior

sub item 1/sub item 2 onclick is not triggered after click the submenu itself

Expected behavior

sub item 1/sub item 2 onclick still triggered normally after click the submenu itself

might related #2796

@gasolin
Copy link
Author

gasolin commented Oct 11, 2018

Here's the workaround

<MenuItem
  popoverProps={{
    hoverCloseDelay: 400,
    captureDismiss: true,
  }}
>
  {submenu}
</MenuItem>

a proper fix might be to append the above popoverProps when found this MenuItem is a submenu

@karlb
Copy link

karlb commented Oct 16, 2018

I have this problem with @blueprintjs/core 3.7.0. gasolin's workaround makes the submenu work, but the parent menu does not close when clicking an entry in the submenu.

@giladgray
Copy link
Contributor

giladgray commented Oct 17, 2018

@karlb yes that's exactly what captureDismiss: true does -- prevents parents from closing due to child clicks.
this issue needs deeper investigation.

@gasolin
Copy link
Author

gasolin commented Oct 17, 2018

another way to make submenu click work is use click mode instead of hover mode

<MenuItem
  popoverProps={{
    interactionKind: PopoverInteractionKind.CLICK,
  }}
>
  {submenu}
</MenuItem>

The menu/sub menu won't close automatically either with above setting (no captureDismiss: true)

@giladgray Is there a way to trigger an event to dismiss those popover in react onClick function?

@giladgray
Copy link
Contributor

@gasolin have you read this docs section carefully? https://blueprintjs.com/docs/#core/components/popover.closing-on-click

@gasolin
Copy link
Author

gasolin commented Oct 18, 2018

@giladgray yes, I inspected the sub menuitems and all items already equip with bp3-popover-dismiss class like above doc mentioned, but still not dismiss correctly. So maybe css is not enough and wonder if there's a js solution or the fix

@giladgray
Copy link
Contributor

@gasolin try adding autoFocus={false} to your topmost Popover. menu item submenus disable that prop because of this exact issue. further, it makes sense for a dropdown menu to leave focus on the original target rather than pulling it into the menu that was just opened.

i'm considering changing the Popover autoFocus default to false, but will not be able to do so until 4.0

@pcdv
Copy link

pcdv commented Dec 12, 2018

Hitting the same issue.
See https://codesandbox.io/s/6r9yvlmpz

Expected:
All menu entries should trigger an alert.

Actual:
The second sub-menu has no problem.
The first sub-menu closes when an entry is clicked.

I tried all the workarounds in this issue but all leave the menu open when clicked.

@giladgray
Copy link
Contributor

@pcdv dude. slap an autoFocus={false} on that Popover and you're golden. tested in your sandbox, works like a dream.

@pcdv
Copy link

pcdv commented Dec 13, 2018

@giladgray thanks, I had forgotten to remove an experimental e.stopPropagation() that caused the menu to remain open.

@jaamison
Copy link
Contributor

jaamison commented Jan 8, 2019

I've dug a little deeper into this, and I think @gasolin is actually onto something bigger. What I've found is that when a Popover with interactionKind HOVER has its target clicked or otherwise focused, the next mousedown (not click) anywhere on the document causes the popover's overlay to close itself. (the nested submenu popover in this case, not the outermost one)

Setting autoFocus={false} may appear to fix things on the surface, but all it's doing is helping to avoid creating the conditions that trigger the bug, but those conditions can still be encountered in different ways, and when they are, the buggy behavior is still exhibited.

This behavior is not immediately visible, because 1) in the majority of cases that the target gets focus, it is also being hovered, so the popover's open condition is still being met, reopening it without delay; and (more interestingly) 2) if a popover's target is clicked, causing the document's next mousedown to close it, and that mousedown event happens to be on an element inside the popover's contents (e.g. a MenuItem), the popover's close transition lets the element linger on the screen just long enough for it to catch the mouseup event of a typical click (although not a long slow one) and therefore have its onClick handler called, making it appear that the menu item was clicked normally and that the click was what caused the popover to be dismissed (which, because the outermost/parent popover is not dismissed - it has no reason to be - makes it appear to be an issue with shouldDismissPopover, Classes.POPOVER_DISMISS, etc when really it has nothing to do with those).

This bug is actually reproducible in the docs examples, although it's hard to even notice unless you're looking for it.

  1. Go to the "Interactions" section of the popover docs: https://blueprintjs.com/docs/#core/components/popover.interactions

  2. Hover the "HOVER" button and wait for the popover menu to pop up

  3. With the cursor still hovering over the HOVER button, click on the button

  4. Move the cursor onto a menu item in the popover

  5. Click the menu item and don't release the mouse button. You'll notice the popover will close, even though all the menu items are shouldDismissPopover={false}

  6. You'll notice that without clicking the target button first, the menu items behave as expected (the popover stays open). Also, if you click quickly enough, you can visually see the menu item's styling change to its active state indicating that it's catching the entire click event before the popover is finished closing.

I also think this might be behind #2796 (onClick will fire if the user clicks quickly enough, but if too much time elapses between the mousedown and the mouseup of the click, then the event won't fire, resulting in behavior that might appear to be nondeterministic)

I'm screwing around with some possible fixes.

@alecf
Copy link
Contributor

alecf commented Mar 1, 2019

I'm experiencing this too, and it makes my app feel really flakey. @giladgray I feel like the "Status: needs more info" is no longer applicable, as you can see this behavior in the docs today:
https://blueprintjs.com/docs/#core/components/menu
image

If you click on "Settings" before you click on "remove application" then "remove application"'s click does not dismiss the menu - this is indication that the actual "remove application" menu item is not firing at all!

@alecf
Copy link
Contributor

alecf commented Mar 1, 2019

and the autoFocus={false} does NOT fix the original problem reported in this bug

@alecf
Copy link
Contributor

alecf commented Mar 1, 2019

actually, pinging @adidahiya instead..

@tnrich
Copy link
Contributor

tnrich commented Mar 11, 2019

Also seeing this issue as well. Let us know if we can help in any way @adidahiya 👍

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

Successfully merging a pull request may close this issue.

8 participants