fix: shift select

.changeset/strange-socks-beam.md

@@ -0,0 +1,5 @@
+---
+"@ultraviolet/ui": patch
+---
+
+`<List />` and `<Table />`: more intuitive behavior for shift+click

packages/ui/src/components/List/ListContext.tsx

@@ -12,13 +12,13 @@
   ComponentProps,
   Dispatch,
   ReactNode,
+  RefObject,
   SetStateAction,
 } from 'react'
 import type { Checkbox } from '../Checkbox'
 import type { ColumnProps } from './types'
 
 type RowState = Record<string | number, boolean>
-type MapCheckbox = Map<string | number, HTMLInputElement>
 
 export type ListContextValue = {
   // ============ Expandable logic ============
@@ -40,13 +40,14 @@
   subscribeHandler: () => void
   columns: ColumnProps[]
   inRange: Set<number | string>
-  mapCheckbox: MapCheckbox
   selectable: boolean
   selectAll: () => void
   selectedRowIds: RowState
   selectRow: (rowId: string) => void
   unselectAll: () => void
   unselectRow: (rowId: string) => void
+  refList: RefObject<HTMLInputElement[]>
+  handleOnChange: (value: string, checked: boolean) => void
 }
 
 const ListContext = createContext<ListContextValue | undefined>(undefined)
@@ -86,11 +87,9 @@
 }: ListProviderProps) => {
   const [expandedRowIds, setExpandedRowIds] = useState<RowState>({})
   const [selectedRowIds, setSelectedRowIds] = useState<RowState>({})
-  const [lastCheckedIndex, setLastCheckedIndex] = useState<
-    null | (number | string)
-  >(null)
+  const [lastCheckedCheckbox, setLastCheckedCheckbox] = useState<string>()
   const [inRange, setInRange] = useState<Set<number | string>>(new Set([]))
-  const refList = useRef<MapCheckbox>(new Map())
+  const refList = useRef<HTMLInputElement[]>([])
 
   const registerExpandableRow = useCallback(
     (rowId: string, expanded = false) => {
@@ -196,99 +195,91 @@
     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)
-        }
-      }
-
-      const handleClickRange = (checkbox: HTMLInputElement) => {
-        const shouldShiftEvent = inRange.size > 0
-        const isClickInsideRange = inRange.has(checkbox.value)
-
-        if (shouldShiftEvent && isClickInsideRange) {
-          let checkboxRows: RowState = {}
+      // Ensure that only existing checkboxes are in refList
+      refList.current = refList.current.filter(checkbox =>
+        document.contains(checkbox),
+      )
 
-          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) -
+              (Math.min(lastCheckedIndex, index) === index ? 1 : 0)
+            const end = Math.max(lastCheckedIndex, index)
+
+            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)
-          }
-          if ([false, 'indeterminate'].includes(state)) {
-            selectRows(checkboxIds, true)
+            selectRows(checkboxesInRange, currentCheckbox.checked) //  (un)selects the rows in the range
+            setLastCheckedCheckbox(currentCheckbox.value)
           }
-        }
+        } 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
          */
         setTimeout(() => {
           // clean up
           setInRange(new Set([]))
-          setLastCheckedIndex(checkbox.value)
+          setLastCheckedCheckbox(currentCheckbox.value)
         }, 1)
       }
-
-      const handleOnChange = (checkbox: HTMLInputElement) => {
-        const shouldHandleEvent = inRange.size === 0
-
-        if (shouldHandleEvent) {
-          selectRows([checkbox.value], !checkbox.checked)
-        }
-        setLastCheckedIndex(checkbox.value)
-      }
-
-      refList.current.forEach(checkbox => {
-        function clickHandler(this: HTMLInputElement) {
-          handleClickRange(this)
-        }
-
-        function hoverHandler(this: HTMLInputElement, event: MouseEvent) {
-          handleHover(this, event)
-        }
-
-        function changeHandler(this: HTMLInputElement) {
-          handleOnChange(this)
+      refList.current.forEach((checkbox, index) => {
+        function clickHandler(this: HTMLInputElement, event: MouseEvent) {
+          handleClickRange(this, index, event.shiftKey)
         }
 
-        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)
         })
       })
     }
 
     return () => {
       handlers.forEach(cleanup => cleanup())
     }
-  }, [inRange, lastCheckedIndex, selectRows])
+  }, [lastCheckedCheckbox, selectRows])
 
   useEffect(subscribeHandler, [subscribeHandler])
 
+  const handleOnChange = useCallback(
+    (value: string, checked: boolean) => {
+      selectRows([value], !checked)
+      setLastCheckedCheckbox(value)
+    },
+    [selectRows],
+  )
+
+  useEffect(() => {
+    // Re-arrange refList everytime it is modified in order to keep the write order of the elements in it.
+    const allCheckboxes = [
+      ...document.querySelectorAll('input[type="checkbox"]:not([value="all"])'),
+    ].map(element => element as HTMLInputElement)
+    refList.current = allCheckboxes.map(checkbox => checkbox)
+  }, [refList.current.length])
+
   const value = useMemo<ListContextValue>(
     () => ({
       allRowSelectValue,
@@ -300,7 +291,6 @@
       expandedRowIds,
       expandRow,
       inRange,
-      mapCheckbox: refList.current,
       registerExpandableRow,
       registerSelectableRow,
       selectable,
@@ -309,6 +299,8 @@
       selectRow,
       unselectAll,
       unselectRow,
+      refList,
+      handleOnChange,
     }),
     [
       allRowSelectValue,
@@ -329,6 +321,7 @@
       selectRow,
       unselectAll,
       unselectRow,
+      handleOnChange,
     ],
   )
 

packages/ui/src/components/List/Row.tsx

@@ -231,9 +231,10 @@ export const Row = forwardRef<HTMLTableRowElement, RowProps>(
       registerSelectableRow,
       selectedRowIds,
       expandButton,
-      mapCheckbox,
       inRange,
       columns,
+      refList,
+      handleOnChange,
     } = useListContext()
 
     const theme = useTheme()
@@ -277,16 +278,13 @@ export const Row = forwardRef<HTMLTableRowElement, RowProps>(
     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)
@@ -341,6 +339,7 @@ export const Row = forwardRef<HTMLTableRowElement, RowProps>(
                     ref={checkboxRef}
                     disabled={isSelectDisabled}
                     inRange={inRange?.has(id)}
+                    onChange={() => handleOnChange(id, selectedRowIds[id])}
                   />
                 </Tooltip>
               </StyledCheckboxContainer>

packages/ui/src/components/List/__stories__/Selectable.stories.tsx

@@ -1,68 +1,67 @@
+import type { StoryFn } from '@storybook/react'
+import { useState } from 'react'
 import { List } from '..'
 import { Button } from '../../Button'
 import { Stack } from '../../Stack'
 import { Text } from '../../Text'
-import { Template } from './Template.stories'
-import { data } from './resources'
+import { columns, data } from './resources'
 
-export const Selectable = Template.bind({})
+export const Selectable: StoryFn<typeof List> = args => {
+  const [clicked, setClick] = useState(true)
 
-Selectable.args = {
-  ...Template.args,
-  selectable: true,
-  children: (
+  return (
     <>
-      {data.map(planet => (
-        <List.Row
-          key={planet.id}
-          id={planet.id}
-          disabled={planet.id === 'mercury'}
-          selectDisabled={
-            planet.id === 'home-sweet-home'
-              ? "Earth isn't selectable"
-              : undefined
-          }
-          expandable={false}
-        >
-          <List.Cell>
-            {planet.name}
-            {planet.id === 'mercury'
-              ? ' (Not selectable because the row itself is disabled)'
-              : ''}
-            {planet.id === 'home-sweet-home'
-              ? ' (Not selectable because of prop `selectDisabled`)'
-              : ''}
-          </List.Cell>
-          <List.Cell>{planet.perihelion}AU</List.Cell>
-          <List.Cell>{planet.aphelion}AU</List.Cell>
-        </List.Row>
-      ))}
-      <List.SelectBar data={data} idKey="id">
-        {({ selectedItems, unselectAll }) => (
-          <Stack
-            direction="row"
-            alignItems="center"
-            justifyContent="space-between"
-            gap={2}
-          >
-            <Text variant="bodyStrong" as="p" sentiment="primary">
-              {selectedItems.length} item(s) selected
-            </Text>
-            <Button
-              size="small"
-              onClick={() => {
-                // oxlint-disable-next-line eslint/no-alert
-                alert('elements could be deleted')
-                unselectAll()
-              }}
+      <List {...args} columns={columns} selectable>
+        {data.map(planet =>
+          planet.id !== 'mars' || clicked ? (
+            <List.Row
+              key={planet.id}
+              id={planet.id}
+              expandable="Planet description"
             >
-              Delete
-            </Button>
-          </Stack>
+              <List.Cell>{planet.id}</List.Cell>
+              <List.Cell>{planet.name}</List.Cell>
+              <List.Cell>{planet.perihelion}AU</List.Cell>
+              <List.Cell>{planet.aphelion}AU</List.Cell>
+            </List.Row>
+          ) : null,
         )}
-      </List.SelectBar>
+
+        <List.SelectBar data={data} idKey="id">
+          {({ selectedItems, unselectAll }) => (
+            <Stack
+              direction="row"
+              alignItems="center"
+              justifyContent="space-between"
+              gap={2}
+            >
+              <Text variant="bodyStrong" as="p" sentiment="primary">
+                {selectedItems.length} item(s) selected (
+                {selectedItems.map(
+                  (item, index) => `${index > 0 ? ', ' : ''}${item.name}`,
+                )}
+                )
+              </Text>
+              <Button
+                size="small"
+                onClick={() => {
+                  // oxlint-disable-next-line eslint/no-alert
+                  alert('elements could be deleted')
+                  unselectAll()
+                }}
+              >
+                Delete
+              </Button>
+            </Stack>
+          )}
+        </List.SelectBar>
+      </List>
+
+      <button type="button" onClick={() => setClick(!clicked)}>
+        {clicked ? 'remove' : 'add'} mars as a planet
+      </button>
     </>
-  ),
+  )
 }
 
 Selectable.parameters = {