Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

feat(value-list, pick-list): support ↑↓ arrow keys for navigation and value selection when multiple mode is disabled #887

Merged
merged 8 commits into from
Mar 27, 2020

Conversation

driskull
Copy link
Member

@driskull driskull commented Mar 20, 2020

Related Issue: #873

Summary

This updates the pick/value-list components to support item navigation with the up/down arrow keys. When multiple is disabled, moving an item will also affect item selection (akin to radio groups).

Notable changes

  • Added setFocus to lists and items.
  • The wrapped "calciteListItemChange" event used by the value-list is modified before it continues bubbling to adjust the item instance accordingly since it was always a pick-list-item instance.

@driskull driskull self-assigned this Mar 20, 2020
@driskull
Copy link
Member Author

@jcfranco this is working but needs the following still:

  • Tests
  • Only the selected item should be able to be tabbed to when multiple is disabled.
    • If no item is selected, the first item should be focusable.
    • This is how native radio elements work. Only the selected radio can be focused and using the arrow keys goes to the next item and selects it.

@driskull driskull added this to the KOO (King of Ooo) milestone Mar 27, 2020
@jcfranco jcfranco changed the title feat(value-list, pick-list): Support arrow keys for values selection feat(value-list, pick-list): support ↑↓ arrow keys for navigation and value selection when multiple mode is disabled Mar 27, 2020
@jcfranco jcfranco added a11y enhancement New feature request for an existing component labels Mar 27, 2020
@jcfranco jcfranco marked this pull request as ready for review March 27, 2020 21:00
@jcfranco jcfranco requested a review from a team as a code owner March 27, 2020 21:00
@jcfranco jcfranco assigned driskull and unassigned driskull Mar 27, 2020
@jcfranco
Copy link
Member

@driskull Can you review? 🙇

Copy link
Member Author

@driskull driskull left a comment

Choose a reason for hiding this comment

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

Code looks good! 👍

@jcfranco the only concern I have is that maybe the control slot should receive focus for pick list?

For example, I can't keyboard to the "X" icon without selecting that item first.

@jcfranco
Copy link
Member

the only concern I have is that maybe the control slot should receive focus for pick list?
For example, I can't keyboard to the "X" icon without selecting that item first.

Could you walk me through that? When you tab into a list with removable items, you'd want the X to be focused and then ↑ ↓ would move between other item's Xs?

@driskull
Copy link
Member Author

When you tab into a list with removable items, you'd want the X to be focused and then ↑ ↓ would move between other item's Xs?

Yeah, I'm not sure what would be ideal. Maybe we could proceed with how you have it and then we can consult with Arjav?

@jcfranco
Copy link
Member

Sounds good. 👌

@jcfranco
Copy link
Member

Approving on behalf of @driskull. 🤜:boom: 🤛

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Mother-tested, self-approved!

@jcfranco jcfranco merged commit 28cc424 into master Mar 27, 2020
@jcfranco jcfranco deleted the dris0000/lists-arrow-keys branch March 27, 2020 23:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y enhancement New feature request for an existing component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants