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.

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

Add optional shouldFocusOnHover prop to aria combobox #7163

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

p1pah
Copy link

@p1pah p1pah commented Oct 9, 2024

No issue to close, but solves this discussion! #7045

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

@devongovett
Copy link
Member

Wouldn't this result in there being multiple highlighted items at once? e.g. one that you focused with the keyboard and one that you hovered over. By focusing the item on hover we ensure that there is only ever one highlighted item at a time. That matches the behavior of native <select>, menus, etc. Is the behavior you're looking to disable actually the selecting of an item on Tab rather than focusing on hover?

@p1pah
Copy link
Author

p1pah commented Oct 15, 2024

@devongovett I'm looking to stop a use case where the user opens the menu, hovers over a item and then they decide tab out. Some of our users have found it confusing that if they are just hovering over something it's automatically selecting it for them :/. With the two highlighted items question, I think we would approach that with two different color shades (one to indicate keyboard selection and one to show mouse hover). This optional prop fixes all our woes our users were having with the mouse hover 🤭! Please let me know what you think

@p1pah
Copy link
Author

p1pah commented Oct 21, 2024

@devongovett Something like this :) https://ariakit.org/components/combobox

@p1pah
Copy link
Author

p1pah commented Nov 6, 2024

Any chance this could be considered/merged into next release 😄 ?

@devongovett
Copy link
Member

devongovett commented Nov 18, 2024

I still feel like the better option would be to add a prop to prevent selecting the focused item on Tab, instead of tying this to hover only. As it is, this only prevents committing on tab if you previously hovered an item instead of using the keyboard. Seems like whether or not to commit on tab shouldn't depend on how you got to the item (hover or keyboard).

@p1pah
Copy link
Author

p1pah commented Nov 18, 2024

I hear you, but that is the exact thing I want haha "only preventing committing on tab if you previously hovered an item instead of using the keyboard.". With the approach of committing vs. not committing on tab, would one just pass the current focused element and detect whether it's focused/hovered via data attributes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants