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 @@ import type {
   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,13 @@ export type ListContextValue = {
   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[]>
 }
 
 const ListContext = createContext<ListContextValue | undefined>(undefined)
@@ -86,11 +86,9 @@ export const ListProvider = ({
 }: 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,55 +194,54 @@ 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
          */
         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,20 +265,18 @@ 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)
         })
       })
     }
 
     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,

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

@@ -231,9 +231,9 @@ export const Row = forwardRef<HTMLTableRowElement, RowProps>(
       registerSelectableRow,
       selectedRowIds,
       expandButton,
-      mapCheckbox,
       inRange,
       columns,
+      refList,
     } = useListContext()
 
     const theme = useTheme()
@@ -277,16 +277,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)

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
           </tr>
           <tr
             class="emotion-45 emotion-46"
-            data-highlight="false"
+            data-highlight="true"
             tabindex="-1"
           >
             <td
@@ -14624,11 +14624,11 @@ exports[`List > Should render correctly with selectable with shift click for mul
                 <div
                   aria-disabled="false"
                   class="emotion-52 emotion-53 emotion-12"
-                  data-checked="false"
+                  data-checked="true"
                   data-error="false"
                 >
                   <input
-                    aria-checked="false"
+                    aria-checked="true"
                     aria-invalid="false"
                     aria-label="select"
                     class="emotion-13 emotion-14"

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

@@ -132,9 +132,9 @@ export const Row = ({
     registerSelectableRow,
     selectedRowIds,
     expandButton,
-    mapCheckbox,
     inRange,
     columns,
+    refList,
   } = useTableContext()
 
   const checkboxRowRef = useRef<HTMLInputElement>(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 (
     <>
       <StyledTr