add selectOnFocus prop to allow disabling selection on focus#9415
add selectOnFocus prop to allow disabling selection on focus#9415romansndlr wants to merge 2 commits intoadobe:mainfrom
Conversation
|
Would love your review on this one please 🙏🏻 |
|
Thanks for the PR, the team is currently on holiday. We'll review when we are back in the new year. |
snowystinger
left a comment
There was a problem hiding this comment.
Would just using 'Option + Arrow Down/Up' to navigate via keyboard without selecting on focus work for you without needing these changes? Selection is pretty complex, so it'd be better if we didn't need to add another option.
| linkBehavior?: 'action' | 'selection' | 'override', | ||
| /** | ||
| * Whether selection should occur automatically on focus. | ||
| * @default true (when selectionBehavior is 'replace') |
There was a problem hiding this comment.
should this be disregarded if selectionBehaviour is not replace?
| it('should not select on focus when selectOnFocus={false} with multiple selection', async () => { | ||
| let onSelectionChange = jest.fn(); | ||
| let {getAllByRole} = render( | ||
| <GridList aria-label="Test" selectionMode="multiple" selectionBehavior="replace" selectOnFocus={false} onSelectionChange={onSelectionChange}> |
There was a problem hiding this comment.
how does one select multiple in replace? does shift+arrow down work?
devongovett
left a comment
There was a problem hiding this comment.
Why can't you use selectionBehavior="toggle"? That mode already has the behavior you want.
Motivation
We want to allow a user to browse the list using their keyboard without "persisting" the selection.
This is consistent with many implementations out in the wild:
✅ Pull Request Checklist:
📝 Test Instructions:
selectionBehavior="replace"andselectionMode="multiple"selectOnFocus={false}to the GridList