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

[select] fix(a11y): update active item on focus changes #5252

Merged
merged 1 commit into from
Apr 15, 2022

Conversation

jakejrichards
Copy link
Contributor

Pass handleFocus to itemRenderer to make select & query list components more accessible

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

Pass handleFocus to itemRenderer to make Select & QueryList components more accessible. Previously, tabbing through a query list would not update the activeItem, this caused unexpected behavior when selecting the actively focused item as it would actually select whatever was in activeItem.

These changes don't completely solve the problem as the up/down key driven state can still get out of sync with the focused item, but tabbing again will resume and we'll never select an unexpected item. I think the fully-baked solution here would go both ways focus <-> active, but this would be much greater lift.

Reviewers should focus on:

The focus here should be whether or not we consider this a viable intermediary state, and if it makes sense to update the active item "on focus" in this way. Another note is that this isn't relevant for Suggest/Multiselect components as the input is outside of the popover in those - tabbing in general is just completely broken there.

Screenshot

Before:
bp-bug-before

After:
bp-bug-after

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this out in the preview build; this feels like a strict improvement over the current UX, so I'm good with merging this. thanks for the PR 👍

@adidahiya adidahiya changed the title Pass handleFocus to itemRenderer to make Select & QueryList components more accessible [select] fix(a11y): update active item on focus changes Apr 15, 2022
@adidahiya adidahiya merged commit 1ab3636 into develop Apr 15, 2022
@adidahiya adidahiya deleted the jaker/select-focus branch April 15, 2022 16:08
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.

3 participants