Skip to content

fix: Support range selection with grid/table checkboxes #7839

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.

Sign up for GitHub

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

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

fix: Support range selection with grid/table checkboxes #7839

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b451fa3
fix: Support range selection with grid/table checkboxes
levrik Feb 27, 2025
f842abe
cr
levrik Mar 3, 2025
7ff30b8
Merge branch 'main' into fix-collection-checkbox-range-selection
snowystinger Mar 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion packages/@react-aria/grid/src/useGridSelectionCheckbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {GridState} from '@react-stately/grid';
// @ts-ignore
import intlMessages from '../intl/*.json';
import {Key} from '@react-types/shared';
import {useEffect, useRef} from 'react';
import {useId} from '@react-aria/utils';
import {useLocalizedStringFormatter} from '@react-aria/i18n';

Expand Down Expand Up @@ -31,8 +32,30 @@ export function useGridSelectionCheckbox<T, C extends GridCollection<T>>(props:
let isDisabled = !state.selectionManager.canSelectItem(key);
let isSelected = state.selectionManager.isSelected(key);

let isShiftDown = useRef(false);
useEffect(() => {
let trackKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Shift') {
isShiftDown.current = true;
}
};
let trackKeyUp = (e: KeyboardEvent) => {
if (e.key === 'Shift') {
isShiftDown.current = false;
}
};

document.addEventListener('keydown', trackKeyDown);
Copy link
Member

Choose a reason for hiding this comment

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

these should be capturing listeners, don't want to miss it should something call stopPropagation

Copy link
Author

Choose a reason for hiding this comment

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

makes sense, added

document.addEventListener('keyup', trackKeyUp);

return () => {
document.removeEventListener('keydown', trackKeyDown);
document.removeEventListener('keyup', trackKeyUp);
};
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
});
}, []);

We don't want to re-run this every time we render, only on mount/unmount

Copy link
Author

@levrik levrik Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

oh my... no idea how I missed that. added.


// Checkbox should always toggle selection, regardless of selectionBehavior.
Copy link
Member

Choose a reason for hiding this comment

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

Will need to discuss with the team if the proposed change conflicts with this intent

Copy link
Author

@levrik levrik Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Just to add some background here: I have a table with href on each row so people use the checkboxes to do the initial selection. While the table then switches into another mode that allows selection also on the rest of each row, people tend to keep clicking on the checkboxes. Not having Shift+Click for range selection available there while it is on the rest of the table is kinda confusing and led to complaints from customers and even the CEO 😅

let onChange = () => manager.toggleSelection(key);
let onChange = () => isShiftDown.current ? manager.extendSelection(key) : manager.toggleSelection(key);

const stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/grid');

Expand Down