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.

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

OverflowMenu not working inside Modal (v10) #3665

Closed
2 tasks done
sally-c-milson opened this issue Aug 6, 2019 · 6 comments
Closed
2 tasks done

OverflowMenu not working inside Modal (v10) #3665

sally-c-milson opened this issue Aug 6, 2019 · 6 comments
Labels
type: bug 🐛

Comments

@sally-c-milson
Copy link

sally-c-milson commented Aug 6, 2019 •
edited
Loading

What package(s) are you using?

  • carbon-components
  • carbon-components-react

Detailed description

Describe in detail the issue you're having.
Trying to upgrade to v10. We are using OverflowMenu inside Modal as seen in the screenshot below. (Open in top left corner of picture). The click event is being blocked by something. I compared OverflowMenuComponent inside and outside the modal. Looking at the event listeners it seems like the event is being blocked by something called focusTrap.

Is this issue related to a specific component?
Interaction between OverflowMenuItem and Modal

What did you expect to happen? What happened instead? What would you like to
see changed?
I expect the click event to get to the handler. It does not.

What browser are you working in?
Chrome

What version of the Carbon Design System are you using?
"carbon-components": "^10.0.0",
"carbon-components-react": "^7.0.0",

What offering/product do you work on? Any pressing ship or release dates we
should be aware of?
I work on Voice Agent, and we planned on finishing the upgrade to v10 for our August release.

Steps to reproduce the issue

  1. Step one
    Create a Modal with an OverflowMenu inside it.
  2. Step two
    Have an OverflowMenuItem item with an onClick handler.
  3. Step three
    Click on the menu item to see it doesn't work.

Additional information

  • Screenshots or code
    model-overflow
@sally-c-milson sally-c-milson changed the title OverflowMenu not working inside Model (v10) OverflowMenu not working inside Modal (v10) Aug 6, 2019
@asudoh
Copy link
Contributor

asudoh commented Aug 6, 2019

Hi 👋 thank you for reporting! Would you create a reduced case based on https://codesandbox.io/s/github/carbon-design-system/carbon/tree/master/packages/react/examples/codesandbox?

@sally-c-milson
Copy link
Author

@asudoh Here it is: https://codesandbox.io/s/codesandbox-c46yh

@asudoh
Copy link
Contributor

asudoh commented Aug 8, 2019

Thanks - Would you try adding focusTrap={false} to <Modal>? Your CodeSandbox seems to be working this way.

@sally-c-milson
Copy link
Author

Thanks! I wasn't aware of that property.

Could there be any negative effects of just turning off focusTrap? (as opposed to moving the open menu to be inside the focusTrap.) I tried tabing out of the modal and everything seemed to be fine, but I was wondering if there were any other possible effects I should be aware of.

@asudoh
Copy link
Contributor

asudoh commented Aug 8, 2019

The only difference I'm aware of is focusTrap={true} supports reverse-focus-wrap (the behavior with shift-tab).

@abbeyhrt
Copy link
Contributor

abbeyhrt commented Sep 9, 2019

I'm going to close this since it seems like the issue was addressed, feel free to comment if you feel like it hasn’t been addressed yet!

@abbeyhrt abbeyhrt closed this as completed Sep 9, 2019
asudoh added a commit to asudoh/carbon-components that referenced this issue Feb 4, 2020
@asudoh
feat(components): support automatic focus sentinel
1bbe349 
This change eliminates the need for application to put focus sentinel
by having `<Modal>`, `<ComposedModal>` and `<FloatingMenu>`
automatically put the focus sentinels.

This change also add support for reverse-focus-wrap feature to
`<Modal>` and `<ComposedModal>`, without needing using 3rd-party
`focus-trap-react` library. This helps applications hitting adverse
side-effects that `focus-trap-react` library causes (e.g. carbon-design-system#3021, carbon-design-system#3665
and carbon-design-system#4600).

Fixes carbon-design-system#3817.
Fixes carbon-design-system#4036.
Fixes carbon-design-system#4600.
@asudoh asudoh mentioned this issue Feb 4, 2020
Merged
asudoh added a commit that referenced this issue Feb 14, 2020
@asudoh
feat(components): support automatic focus sentinel (#5260)
0e9c6d5 
This change eliminates the need for application to put focus sentinel
by having `<Modal>`, `<ComposedModal>` and `<FloatingMenu>`
automatically put the focus sentinels.

This change also add support for reverse-focus-wrap feature to
`<Modal>` and `<ComposedModal>`, without needing using 3rd-party
`focus-trap-react` library. This helps applications hitting adverse
side-effects that `focus-trap-react` library causes (e.g. #3021, #3665
and #4600).

Fixes #3817.
Fixes #4036.
Fixes #4600.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: bug 🐛
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@asudoh @abbeyhrt @sally-c-milson