Enable onAction with checkbox selection in ListView and TableView #3103

devongovett · 2022-05-04T23:06:43Z

Back in #2363, we added support for onAction in selectable collections such as ListView and TableView, along with support for highlight selection. However, we have use cases for onAction (i.e. folder navigation) with checkbox selection as well. This PR enables that.

In order to implement this we made the following behavior changes:

When onAction is defined, row actions become the primary click interaction instead of selection.
Users can click the checkbox to select rows. Once at least one row is selected, clicking anywhere on another row toggles its selection state (instead of performing onAction).
On touch devices, users can also long press on a row to select it, just like with highlight selection.
With keyboards, the Enter key always performs onAction (on key up) when defined, whereas Space toggles selection (on key down).
When drag and drop is enabled, it may conflict with the long press gesture used to enter selection mode. To avoid this, we disable dragging when nothing is selected on touch devices. So long pressing enters selection mode, and from there the user can drag. Not sure if this is always intuitive but it might be the only way. At least it only happens when both selection and dnd are enabled.

To do before release

Update docs

adobe-bot · 2022-05-04T23:13:24Z

Build successful! 🎉

adobe-bot · 2022-05-05T23:46:15Z

Build successful! 🎉

packages/@react-aria/selection/src/useSelectableItem.ts

snowystinger

With VO on iPhone, I noticed that I can start dragging in two ways, either the drag handle, or the new second long press to drag that all touch devices get.
In the second case, where I long press to start dragging, none of the announcements are read about how to perform the drag and drop or when it is completed.

Disabled Key f does not look very disabled? i had to look in the story to find out why I couldn't select or drag the item (ListView draggable-rows-selectionstyle-highlight-onaction)

In TableView selectionstyle-checkbox-onaction, if I use a mouse and click and long hold, it will not do action or selection when I stop. It also briefly flashes into active mode.

snowystinger · 2022-05-06T16:31:29Z

packages/@react-spectrum/table/test/Table.test.js

+        checkRowSelection(rows.slice(1), false);
+
+        let checkbox = within(rows[1]).getByRole('checkbox');
+        userEvent.click(checkbox);


stay in pointerType touch?

packages/@react-aria/selection/src/useSelectableItem.ts

snowystinger · 2022-05-06T17:20:56Z

packages/@react-aria/selection/src/useSelectableItem.ts

-          onSelect(e);
-        }
+        if (hasPrimaryAction || (hasSecondaryAction && e.pointerType !== 'mouse')) {
+          if (e.pointerType === 'keyboard' && !isActionKey()) {


how can we have a keyboard onPress that isn't an action key?

Action key = Enter, Selection Key = Space

Ah, yep, ok, weird thing then
https://reactspectrum.blob.core.windows.net/reactspectrum/b0e0b1d9f96ae8a0823015f9b74510194bb65205/storybook/index.html?path=/story/listview--selectionstyle-checkbox-onaction
If i select using space, then deselect using space. It triggers action when it deselects

+1 to what Rob saw. Something similar also happens if the user selects a row via clicking the checkbox and clicks on the row to deselect it

It should now only trigger onSelectionChange when de-selecting, not onAction. And when an onAction is defined, the Space key always selects, and the Enter key always only performs actions, not selection. Actions cannot be performed once in selection mode, matching mouse/touch.

devongovett · 2022-05-06T19:42:23Z

@snowystinger

With VO on iPhone, I noticed that I can start dragging in two ways, either the drag handle, or the new second long press to drag that all touch devices get.
In the second case, where I long press to start dragging, none of the announcements are read about how to perform the drag and drop or when it is completed.

This is the native drag and drop API - the same as you'd use without VO on. Long press is a passthrough gesture, so VoiceOver just lets it go through to the native dnd events.

LFDanLu · 2022-05-06T20:44:40Z

packages/@react-spectrum/list/test/ListView.test.js

+        fireEvent(el, pointerEvent('pointerdown', {pointerType: 'touch', pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
+        fireEvent(el, pointerEvent('pointerup', {pointerType: 'touch', pointerId: 1, width: 1, height: 1, pressure: 0, detail: 0}));
+        fireEvent.click(el, {pointerType: 'touch', width: 1, height: 1, detail: 1});


This test intentionally had a pointerType of mouse to mimic the Android Talkback specific case, mind if we change it back?

LFDanLu · 2022-05-06T20:52:11Z

packages/@react-aria/selection/src/useSelectableItem.ts

-          onSelect(e);
-        }
+        if (hasPrimaryAction || (hasSecondaryAction && e.pointerType !== 'mouse')) {
+          if (e.pointerType === 'keyboard' && !isActionKey()) {


+1 to what Rob saw. Something similar also happens if the user selects a row via clicking the checkbox and clicks on the row to deselect it

LFDanLu · 2022-05-06T20:55:38Z

Note: I'm happy to approve and merge for Monday testing, everything else seemed to work just fine on Android/desktop

…navigation # Conflicts: # packages/@react-spectrum/list/stories/ListView.stories.tsx

adobe-bot · 2022-05-09T16:03:56Z

Build successful! 🎉

LFDanLu

Still seeing onAction get fired when the user deselects the row via click in this story but that can be followup. Approving for testing

Repro:

Select the first row via clicking the checkbox
De-select the row by clicking on the row itself. Note onAction is fired

devongovett · 2022-05-09T16:43:19Z

Ughhhhh

devongovett added 2 commits May 4, 2022 15:55

Enable onAction with checkbox selection in ListView and TableView

17846f7

Fix test

00eb31c

Merge branch 'main' into listview-navigation

b0e0b1d

LFDanLu reviewed May 6, 2022

View reviewed changes

packages/@react-aria/selection/src/useSelectableItem.ts Show resolved Hide resolved

snowystinger reviewed May 6, 2022

View reviewed changes

LFDanLu reviewed May 6, 2022

View reviewed changes

devongovett added 3 commits May 9, 2022 08:21

Merge branch 'main' of github.com:adobe/react-spectrum into listview-…

693d9b4

…navigation # Conflicts: # packages/@react-spectrum/list/stories/ListView.stories.tsx

Prevent both selection and action occurring with keyboard

907c5fd

Test updates

a80569f

devongovett added needs testing needs docs labels May 9, 2022

dannify approved these changes May 9, 2022

View reviewed changes

LFDanLu approved these changes May 9, 2022

View reviewed changes

devongovett merged commit 7707ad0 into main May 9, 2022

devongovett deleted the listview-navigation branch May 9, 2022 16:43

devongovett mentioned this pull request May 9, 2022

Prevent onAction on mouse up when deselecting #3111

Merged

devongovett removed the needs testing label May 10, 2022

Enable onAction with checkbox selection in ListView and TableView #3103

Enable onAction with checkbox selection in ListView and TableView #3103

Uh oh!

Conversation

devongovett commented May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adobe-bot commented May 4, 2022

Uh oh!

adobe-bot commented May 5, 2022

Uh oh!

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devongovett commented May 6, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LFDanLu commented May 6, 2022

Uh oh!

adobe-bot commented May 9, 2022

Uh oh!

LFDanLu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devongovett commented May 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

devongovett commented May 4, 2022 •

edited

Loading

LFDanLu left a comment •

edited

Loading