From 010a4a5c27453fea9467a32a2cebdb22d00bebaa Mon Sep 17 00:00:00 2001 From: lisa Date: Fri, 3 Jan 2025 16:37:52 +0100 Subject: [PATCH] fix: shift select --- .changeset/strange-socks-beam.md | 5 + .../ui/src/components/List/ListContext.tsx | 103 +++++------ packages/ui/src/components/List/Row.tsx | 13 +- .../__snapshots__/index.test.tsx.snap | 6 +- packages/ui/src/components/Table/Row.tsx | 23 ++- .../__snapshots__/index.test.tsx.snap | 165 +----------------- .../components/Table/__tests__/index.test.tsx | 7 +- 7 files changed, 78 insertions(+), 244 deletions(-) create mode 100644 .changeset/strange-socks-beam.md diff --git a/.changeset/strange-socks-beam.md b/.changeset/strange-socks-beam.md new file mode 100644 index 0000000000..1138e407b1 --- /dev/null +++ b/.changeset/strange-socks-beam.md @@ -0,0 +1,5 @@ +--- +"@ultraviolet/ui": patch +--- + +`` and ``: more intuitive behavior for shift+click diff --git a/packages/ui/src/components/List/ListContext.tsx b/packages/ui/src/components/List/ListContext.tsx index be0021e5f9..97827f0e2e 100644 --- a/packages/ui/src/components/List/ListContext.tsx +++ b/packages/ui/src/components/List/ListContext.tsx @@ -12,13 +12,13 @@ import type { ComponentProps, Dispatch, ReactNode, + RefObject, SetStateAction, } from 'react' import type { Checkbox } from '../Checkbox' import type { ColumnProps } from './types' type RowState = Record -type MapCheckbox = Map export type ListContextValue = { // ============ Expandable logic ============ @@ -40,13 +40,13 @@ export type ListContextValue = { subscribeHandler: () => void columns: ColumnProps[] inRange: Set - mapCheckbox: MapCheckbox selectable: boolean selectAll: () => void selectedRowIds: RowState selectRow: (rowId: string) => void unselectAll: () => void unselectRow: (rowId: string) => void + refList: RefObject } const ListContext = createContext(undefined) @@ -86,11 +86,9 @@ export const ListProvider = ({ }: ListProviderProps) => { const [expandedRowIds, setExpandedRowIds] = useState({}) const [selectedRowIds, setSelectedRowIds] = useState({}) - const [lastCheckedIndex, setLastCheckedIndex] = useState< - null | (number | string) - >(null) + const [lastCheckedCheckbox, setLastCheckedCheckbox] = useState() const [inRange, setInRange] = useState>(new Set([])) - const refList = useRef(new Map()) + const refList = useRef([]) const registerExpandableRow = useCallback( (rowId: string, expanded = false) => { @@ -196,47 +194,46 @@ export const ListProvider = ({ const handlers: (() => void)[] = [] if (refList.current) { - const handleHover = (checkbox: HTMLInputElement, event: MouseEvent) => { - const isShiftPressed = event.shiftKey - - const isHoverActive = - isShiftPressed && lastCheckedIndex !== null && !checkbox.disabled - - if (isHoverActive) { - setInRange(prev => new Set([...prev, checkbox.value])) - } - - if (!lastCheckedIndex && !checkbox.disabled) { - setLastCheckedIndex(checkbox.value) - } + // Ensure that only existing checkboxes are in refList + if (refList.current) { + refList.current = refList.current.filter(checkbox => + document.contains(checkbox), + ) } - - const handleClickRange = (checkbox: HTMLInputElement) => { - const shouldShiftEvent = inRange.size > 0 - const isClickInsideRange = inRange.has(checkbox.value) - - if (shouldShiftEvent && isClickInsideRange) { - let checkboxRows: RowState = {} - - refList.current.forEach((value, key) => { - if (inRange.has(key)) { - checkboxRows = { - ...checkboxRows, - // handle the conflict event ( click and onChange in the same time on the last checkbox click) - [key]: key === checkbox.value ? !value.checked : value.checked, + const handleClickRange = ( + currentCheckbox: HTMLInputElement, + index: number, + isShiftPressed: boolean, + ) => { + if (isShiftPressed) { + const checkboxesInRange: string[] = [] + + // Get the index of the lastCheckedCheckbox + const targetCheckbox = refList.current.find( + checkbox => checkbox.value === lastCheckedCheckbox, + ) + const lastCheckedIndex = targetCheckbox + ? refList.current.indexOf(targetCheckbox) + : undefined + + if (lastCheckedIndex !== undefined) { + const start = Math.min(lastCheckedIndex, index) + const end = + Math.max(lastCheckedIndex, index) + + (Math.max(lastCheckedIndex, index) === index ? 0 : 1) + + refList.current.forEach((checkbox, key) => { + if (start < key && key < end) { + if (!checkbox.disabled) { + checkboxesInRange.push(checkbox.value) + } } - } - }) - const state = checkStateOfCheckboxs(checkboxRows) - const checkboxIds = Object.keys(checkboxRows) + }) - if (state === true) { - selectRows(checkboxIds, false) + selectRows(checkboxesInRange, currentCheckbox.checked) // (un)selects the rows in the range + setLastCheckedCheckbox(currentCheckbox.value) } - if ([false, 'indeterminate'].includes(state)) { - selectRows(checkboxIds, true) - } - } + } else if (index === 0) setLastCheckedCheckbox(undefined) /** * Handle the case when there is multiple selected value during a time, and the user click without shift event @@ -244,7 +241,7 @@ export const ListProvider = ({ setTimeout(() => { // clean up setInRange(new Set([])) - setLastCheckedIndex(checkbox.value) + setLastCheckedCheckbox(currentCheckbox.value) }, 1) } @@ -254,16 +251,12 @@ export const ListProvider = ({ if (shouldHandleEvent) { selectRows([checkbox.value], !checkbox.checked) } - setLastCheckedIndex(checkbox.value) + setLastCheckedCheckbox(checkbox.value) } - refList.current.forEach(checkbox => { - function clickHandler(this: HTMLInputElement) { - handleClickRange(this) - } - - function hoverHandler(this: HTMLInputElement, event: MouseEvent) { - handleHover(this, event) + refList.current.forEach((checkbox, index) => { + function clickHandler(this: HTMLInputElement, event: MouseEvent) { + handleClickRange(this, index, event.shiftKey) } function changeHandler(this: HTMLInputElement) { @@ -272,12 +265,10 @@ export const ListProvider = ({ checkbox.addEventListener('change', changeHandler) checkbox.addEventListener('click', clickHandler) - checkbox.addEventListener('mouseover', hoverHandler) handlers.push(() => { checkbox.removeEventListener('change', changeHandler) checkbox.removeEventListener('click', clickHandler) - checkbox.removeEventListener('mouseover', hoverHandler) }) }) } @@ -285,7 +276,7 @@ export const ListProvider = ({ return () => { handlers.forEach(cleanup => cleanup()) } - }, [inRange, lastCheckedIndex, selectRows]) + }, [inRange.size, lastCheckedCheckbox, selectRows]) useEffect(subscribeHandler, [subscribeHandler]) @@ -300,7 +291,6 @@ export const ListProvider = ({ expandedRowIds, expandRow, inRange, - mapCheckbox: refList.current, registerExpandableRow, registerSelectableRow, selectable, @@ -309,6 +299,7 @@ export const ListProvider = ({ selectRow, unselectAll, unselectRow, + refList, }), [ allRowSelectValue, diff --git a/packages/ui/src/components/List/Row.tsx b/packages/ui/src/components/List/Row.tsx index c82edd3138..47e7edb151 100644 --- a/packages/ui/src/components/List/Row.tsx +++ b/packages/ui/src/components/List/Row.tsx @@ -231,9 +231,9 @@ export const Row = forwardRef( registerSelectableRow, selectedRowIds, expandButton, - mapCheckbox, inRange, columns, + refList, } = useListContext() const theme = useTheme() @@ -277,16 +277,13 @@ export const Row = forwardRef( const canClickRowToExpand = !disabled && !!expandable && !expandButton useEffect(() => { + const refAtEffectStart = refList.current const { current } = checkboxRef - if (current) { - mapCheckbox.set(id, current) + if (refAtEffectStart && current && !refAtEffectStart.includes(current)) { + refList.current.push(current) } - - return () => { - mapCheckbox.delete(id) - } - }, [mapCheckbox, id]) + }, [refList]) const childrenLength = Children.count(children) + (selectable ? 1 : 0) + (expandButton ? 1 : 0) diff --git a/packages/ui/src/components/List/__tests__/__snapshots__/index.test.tsx.snap b/packages/ui/src/components/List/__tests__/__snapshots__/index.test.tsx.snap index 1e2a3cd9ed..fa1c427585 100644 --- a/packages/ui/src/components/List/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/ui/src/components/List/__tests__/__snapshots__/index.test.tsx.snap @@ -14612,7 +14612,7 @@ exports[`List > Should render correctly with selectable with shift click for mul
Should render correctly with selectable with shift click for mul
(null) @@ -170,23 +170,20 @@ export const Row = ({ const canClickRowToExpand = hasExpandable && !expandButton - useEffect(() => { - const { current } = checkboxRowRef - - if (current) { - mapCheckbox.set(id, current) - } - - return () => { - mapCheckbox.delete(id) - } - }, [mapCheckbox, id]) - const theme = useTheme() const childrenLength = Children.count(children) + (selectable ? 1 : 0) + (expandButton ? 1 : 0) + useEffect(() => { + const refAtEffectStart = refList.current + const { current } = checkboxRowRef + + if (refAtEffectStart && current && !refAtEffectStart.includes(current)) { + refList.current.push(current) + } + }, [refList]) + return ( <> Should render correctly with selectable and shift click 1`] = ` fill: #ffebf2; } -.emotion-58 svg { - padding: 0.125rem; - outline: 1px inset #8c40ef; - box-shadow: 0px 0px 0px 3px #8c40ef40; - -webkit-transition: box-shadow 250ms ease,outline 250ms ease,padding 250ms ease; - transition: box-shadow 250ms ease,outline 250ms ease,padding 250ms ease; -} - -.emotion-58 svg rect { - fill: #e5dbfd; - stroke: #792dd4; -} - .emotion-70 { display: table-cell; vertical-align: middle; @@ -4162,144 +4149,6 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` text-align: left; } -.emotion-148 { - position: relative; - display: -webkit-inline-box; - display: -webkit-inline-flex; - display: -ms-inline-flexbox; - display: inline-flex; - -webkit-align-items: start; - -webkit-box-align: start; - -ms-flex-align: start; - align-items: start; - gap: 0.5rem; -} - -.emotion-148 .eqr7bqq4 { - cursor: pointer; -} - -.emotion-148[aria-disabled='true'] { - cursor: not-allowed; - color: #b5b7bd; -} - -.emotion-148[aria-disabled='true'] .eqr7bqq4 { - cursor: not-allowed; -} - -.emotion-148[aria-disabled='true'] .emotion-16 { - fill: #e9eaeb; -} - -.emotion-148[aria-disabled='true'] .emotion-16 .emotion-18 { - stroke: #d9dadd; - fill: #f3f3f4; -} - -.emotion-148[aria-disabled='true'] .emotion-14[aria-invalid="true"]:checked+.emotion-16 { - fill: #ffd3e3; -} - -.emotion-148[aria-disabled='true'] .emotion-14[aria-invalid="true"]:checked+.emotion-16 .emotion-18 { - stroke: #ffd3e3; - fill: #ffd3e3; -} - -.emotion-148[aria-disabled='true'] .emotion-14[aria-invalid="true"]+.emotion-16 { - fill: #ffebf2; -} - -.emotion-148[aria-disabled='true'] .emotion-14[aria-invalid="true"]+.emotion-16 .emotion-18 { - stroke: #ffbbd3; - fill: #ffebf2; -} - -.emotion-148[aria-disabled='true'] .emotion-14:checked+.emotion-16 { - fill: #e5dbfd; -} - -.emotion-148[aria-disabled='true'] .emotion-14:checked+.emotion-16 .emotion-18 { - stroke: #d8c5fc; - fill: #d8c5fc; -} - -.emotion-148[aria-disabled='true'] .emotion-14[aria-checked="mixed"]+.emotion-16 { - fill: #e5dbfd; -} - -.emotion-148[aria-disabled='true'] .emotion-14[aria-checked="mixed"]+.emotion-16 .emotion-18 { - stroke: #e5dbfd; - fill: #e5dbfd; -} - -.emotion-148 .emotion-14:checked+.emotion-16 path { - transform-origin: center; - -webkit-transition: 200ms -webkit-transform ease-in-out; - transition: 200ms transform ease-in-out; - -webkit-transform: scale(1); - -moz-transform: scale(1); - -ms-transform: scale(1); - transform: scale(1); - -webkit-transform: translate(2px, 2px); - -moz-transform: translate(2px, 2px); - -ms-transform: translate(2px, 2px); - transform: translate(2px, 2px); -} - -.emotion-148 .emotion-14:checked+.emotion-16 .emotion-18 { - fill: #8c40ef; - stroke: #8c40ef; -} - -.emotion-148 .emotion-14[aria-invalid="true"]:checked+.emotion-16 .emotion-18 { - fill: #e51963; - stroke: #e51963; -} - -.emotion-148 .emotion-14[aria-checked="mixed"]+.emotion-16 .emotion-20 { - fill: #ffffff; -} - -.emotion-148 .emotion-14[aria-checked="mixed"]+.emotion-16 .emotion-18 { - fill: #8c40ef; - stroke: #8c40ef; -} - -.emotion-148:hover[aria-disabled='false'] .emotion-14[aria-invalid='false'][aria-checked='false']+.emotion-16 .emotion-18 { - stroke: #792dd4; - fill: #e5dbfd; -} - -.emotion-148:hover[aria-disabled='false'] .emotion-14[aria-invalid='false'][aria-checked='true']+.emotion-16 .emotion-18 { - stroke: #792dd4; - fill: #792dd4; -} - -.emotion-148:hover[aria-disabled='false'] .emotion-14[aria-invalid='false'][aria-checked='mixed']+.emotion-16 .emotion-18 { - stroke: #792dd4; - fill: #792dd4; -} - -.emotion-148:hover[aria-disabled='false'] .emotion-14[aria-invalid='true'][aria-checked='false']+.emotion-16 .emotion-18 { - stroke: #92103f; - fill: #ffd3e3; -} - -.emotion-148:hover[aria-disabled='false'] .emotion-14[aria-invalid='true'][aria-checked='true']+.emotion-16 .emotion-18 { - stroke: #d6175c; - fill: #d6175c; -} - -.emotion-148 .emotion-14[aria-invalid="true"]+.emotion-16 { - fill: #e51963; -} - -.emotion-148 .emotion-14[aria-invalid="true"]+.emotion-16 .emotion-18 { - stroke: #e51963; - fill: #ffebf2; -} -
@@ -4705,7 +4554,7 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` >
@@ -4794,7 +4643,7 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` >
@@ -4883,7 +4732,7 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` >
@@ -4972,7 +4821,7 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` >
@@ -5061,7 +4910,7 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` >
@@ -5150,7 +4999,7 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` >
@@ -5239,7 +5088,7 @@ exports[`Table > Should render correctly with selectable and shift click 1`] = ` >
diff --git a/packages/ui/src/components/Table/__tests__/index.test.tsx b/packages/ui/src/components/Table/__tests__/index.test.tsx index 8ccdaafabd..d820815986 100644 --- a/packages/ui/src/components/Table/__tests__/index.test.tsx +++ b/packages/ui/src/components/Table/__tests__/index.test.tsx @@ -423,14 +423,9 @@ describe('Table', () => { fireEvent.keyDown(document, { key: 'Shift', code: 'ShiftLeft' }) - // Test hovering - fireEvent.mouseOver(firstRowCheckbox, { shiftKey: true }) - fireEvent.mouseOver(secondRowCheckbox, { shiftKey: true }) - fireEvent.mouseOver(thirdRowCheckbox, { shiftKey: true }) - fireEvent.keyUp(document, { key: 'Shift', code: 'ShiftLeft' }) - fireEvent.click(thirdRowCheckbox) + fireEvent.click(thirdRowCheckbox, { shiftKey: true }) expect(firstRowCheckbox).toBeChecked() expect(secondRowCheckbox).toBeChecked()