oui

oui copied to clipboard

Description

This PR removes the initial focusing state when opening up the theme selector popup.

Issues Resolved

Fixes #1163

Demo

Before

https://github.com/opensearch-project/oui/assets/65143821/c15db0c1-fd19-48d9-a458-cab4642d459d

After

https://github.com/opensearch-project/oui/assets/65143821/6326c9d9-db29-45a0-87ef-fbcc2703195d

Misc

Also fix a typo in guide_version_selector.tsx.

Check List

• [ ] New functionality includes testing.
• [ ] New functionality has been documented.
• [ ] All tests pass
  • [ ] yarn lint
  • [ ] yarn test-unit
• [ ] Update CHANGELOG.md
• [x] Commits are signed per the DCO using --signoff

Hi @kgcreative, could you please assist in determining the best UX for this scenario? Thank you!

This is a challenging one. When opening via mouse, we shouldn't have a focus selector, however, per WCAG accessibility standards, there is an expectation that when you open a control via keyboard interaction, that we move keyboard focus to the first item in the opened control. As such, if we can't differentiate between keyboard or mouse input, the existing behavior is the more correct one.

Additional context: https://www.w3.org/TR/UNDERSTANDING-WCAG20/navigation-mechanisms-focus-visible.html

Thanks for sharing your insights, @kgcreative! Based on what you've described, would you suggest implementing a mechanism where if the element is opened via keyboard (for example, pressing the Tab key), we highlight the selector, but if not, we skip the highlighting?

Thanks for sharing your insights, @kgcreative! Based on what you've described, would you suggest implementing a mechanism where if the element is opened via keyboard (for example, pressing the Tab key), we highlight the selector, but if not, we skip the highlighting?

This may be a larger issue. I would explore using :focus-visible instead of :focus, since that does use browser heuristics to determine whether an element should have a visible focus indicator or not.

cc: @BSFishy + @joshuarrrr

I feel like this is the wrong way to go about this. I think that changing the default value for focus would be breaking for UX. Additionally, OuiContextMenuPanelDescriptor offers initialFocusedItemIndex, which I think would be a better way to implement this. Just set that value to whatever value is currently selected. That way we don't even need to differentiate if the panel was opened by keyboard or mouse

Yeah, this needs to be a component level change, not changing the fous behavior in the implementation

@willie-hung are you interested in taking this forward or would you rather have it closed?

Hi @AMoo-Miki, I think it should be closed for now. However, if you decide to make changes at the component level, I would be happy to join the discussion and assist with the development.

Closing as recommended.