Added managed keyboard nav and roles to NxDropdown - RSC-989 #845
Conversation
|
According to the pattern, when the dropdown menu is opened, it first focuses on the menu as shown here in the snapshot.
Will be reaching out to the designer about how it should look like when focused. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
gallery/src/components/NxDropdown/NxDropdownCloseHandlerExample.tsx
Outdated
Show resolved
Hide resolved
That's not what I'm seeing in the ARIA patterns docs and examples. Everything I'm seeing says to focus the first item in the dropdown, except if it was opened by the user pressing |
| <span className="nx-dropdown-button-content"> | ||
| Text Link - this link should trigger truncation | ||
| </span> | ||
| </a> | ||
| <NxButton onClick={() => alert('icon click')} | ||
| className="nx-dropdown-right-button" | ||
| variant="icon-only" | ||
| title="Delete Button Link2"> | ||
| title="Delete Button Link2" | ||
| role="menuitem"> |
There was a problem hiding this comment.
Check with design about how these buttons should be added into the recommended patterns. These are pretty nonstandard, so I think we're on our own in terms of how they behave. Having them just be part of the up/down key cycling doesn't necessarily seem right though.
| const page = getPage(); | ||
|
|
||
| await moveMouseAway(); | ||
| await page.keyboard.press('ArrowLeft'); |
There was a problem hiding this comment.
I don't see any behavior for the Left arrow key defined in the ARIA docs; what's this for?
| expect(button).toHaveProp('aria-controls', menuId); | ||
| expect(menu).toHaveProp('id', menuId); | ||
| }); | ||
|
|
There was a problem hiding this comment.
aria-expanded should be tested too
|
A key aspect of compound ARIA widgets like this is that the whole thing is one tabstop. That is, the user can tab to the dropdown, and regardless of whether it is open or not or how many menu items it has, they can tab away from it with a single press of the Tab key. Moving focus within the component should not be possible with Tab, but only with the arrow keys. This is typically accomplished by setting |
| } | ||
| }, [disableMenuKeyNav]); | ||
|
|
||
| function setFocusToMenuItem(direction: -1 | 1) { |
There was a problem hiding this comment.
Both NxSearchDropdown and NxComboxbox have also had to implement logic to do what this is doing, which they did with a function called adjustBtnFocus. As this is the third time we've needed similar logic, I think it probably makes sense to see if we can extract that out into a utility and reuse it here too.
There was a problem hiding this comment.
Both NxSearchDropdown and NxComboxbox have also had to implement logic to do what this is doing, which they did with a function called adjustBtnFocus. As this is the third time we've needed similar logic, I think it probably makes sense to see if we can extract that out into a utility and reuse it here too.
For that matter, both of those other components rely on this one. It'd be great if we could find a way to encapsulate this logic only in this component and remove it from the others.
| } | ||
| } | ||
|
|
||
| function activateMenuItem(event: React.KeyboardEvent<HTMLElement>) { |
There was a problem hiding this comment.
This seems like it's replicating built-in behavior; is it really necessary?
| @@ -68,12 +68,16 @@ function NxFilterDropdownRender<T extends string | number = string>(props: Props | |||
|
|
|||
| function onResetClick() { | |||
| onChange(new Set()); | |||
| if (menuRef.current) { | |||
| menuRef.current.focus(); | |||
There was a problem hiding this comment.
As I mentioned in another comment, I'm not seeing anything indicating that focusing the menu itself is truly a part of the pattern. If we get rid of that, what you could do here instead is focus the dropdown toggle.
Co-authored-by: Ross Pokorny <[email protected]>
https://issues.sonatype.org/browse/RSC-989
Curiously, in the example here, https://www.w3.org/WAI/ARIA/apg/example-index/menu-button/menu-button-links.html, it focuses on the menuitem when you hover over it with mouse.
Still working on this ticket, but would appreciate feedbacks!