fix(overflow-menu): change menu div to button and update styles #4851
Conversation
ghost
requested review from
|
Deploy preview for the-carbon-components ready! Built with commit 497805f https://deploy-preview-4851--the-carbon-components.netlify.com |
|
Deploy preview for carbon-components-react ready! Built with commit 497805f https://deploy-preview-4851--carbon-components-react.netlify.com |
|
Deploy preview for carbon-elements ready! Built with commit 497805f |
| @@ -25,6 +25,10 @@ | |||
| @include button-reset; | |||
| } | |||
|
|
|||
| .#{$prefix}--overflow-menu { | |||
There was a problem hiding this comment.
Should we include this with the trigger block above?
| ( | ||
| menuBody.querySelector('[data-floating-menu-primary-focus]') || menuBody | ||
| ).focus(); | ||
| const primaryFocus = |
There was a problem hiding this comment.
What do you think the best name for this would be? Would it be something like focusTarget? Something else? curious what you feel like this is for and what name could communicate that clearly.
There was a problem hiding this comment.
I think primaryFocus works! In OverflowMenuItem, it seems like they use primaryFocus to mean a similar thing?
? { 'data-floating-menu-primary-focus': true }
: {};
Closes #2482
This was done during mob programming with @joshblack and @aledavila
This PR changes the OverflowMenu
divto abutton. In exploring the issue, we found focus is moved to the first item too quickly on a space keypress and FF is registering akeyUpevent that closes the menu. Changing the div to a button stops the focus from moving to the first element before the completion of the space keypress.Changelog
New
Changed
divtobuttonRemoved
Testing / Reviewing
Go to the OverflowMenu and make sure that space and enter open the menu and close the menu as expected in all browsers