feat(floating-menu): add close-on-blur behavior #219
Conversation
This change introduces track-blur mix-in which calls `handleBlur` method when component's root element loses focus. With this change, floating menu closes when it loses focus, and sets the focus back to the trigger button.
asudoh
force-pushed
the
floating-menu-close-on-blur
branch
from
31ef994 to
5f3d80a
Compare
| @@ -169,6 +183,7 @@ class FloatingMenu extends mixin(createComponent, eventedShowHideState) { | |||
| } | |||
| this._getContainer().appendChild(this.element); | |||
| this._place(); | |||
| (this.element.querySelector(this.options.selectorPrimaryFocus) || this.element).focus(); | |||
There was a problem hiding this comment.
If selectorPrimaryFocus is unspecified then focus should be placed on the first focusable element inside the menu.
There was a problem hiding this comment.
@iangfleming First focusable element is more complex topic than a human would imagine. A good interpretation of HTML spec can be found here.
Instead, this line does the following; If there is no element matching this.options.selectorPrimaryFocus, try to focus on the floating menu itself. If the floating menu is focusable, user clicking on tab key after floating menu gets open is automatically navigated to the first focusable element.
There was a problem hiding this comment.
@asudoh My concern is that if a dev is dynamically loading or changing the menu items and doesn't properly add selectorPrimaryFocus then the end user's experience will be extremely broken. When I was researching this I had come across that stackoverflow post you mentioned so I definitely understand that "first focusable element" is tricky. Maybe a better addition would be if selectorPrimaryFocus isn't found try and find the first a, button, input, select, or [tabindex]. That won't cover all focusable elements but it will cover the vast majority of use cases.
There was a problem hiding this comment.
My concern is that if a dev is dynamically loading or changing the menu items and doesn't properly add selectorPrimaryFocus then the end user's experience will be extremely broken
@iangfleming I can see the concern. There is a logic to alleviate that - I have made a change to the sample HTML to demonstrate how it works: 230d500
You can remove data-floating-menu-primary-focus attributes from the HTML to see that.
There was a problem hiding this comment.
Below shows how it works without data-floating-menu-primary-focus:
|
this is actually amazing! thank you! 👍 ✅ |
…on-design-system#219) * Update $productive-heading-05 to Light weight $expressive-heading-04 and -05 are using the light weight at 28px; $productive-heading-05 needs to be updated to match. * chore(type): sync to styles and snapshots
Overview
With this change, floating menu closes when it loses focus, and sets the focus back to the trigger button.
Added
handleBlurmethod when component's root element loses focusTesting / Reviewing
Testing should make sure dropdown, overflow-menu and tooltip are not broken.