refactor(tables): row selection as discriminated union (#4466)

* fix(tables): decouple master checkbox from cell-range, add allRowsSelected flag Master checkbox detached from gutter selection state when rows or columns changed after Cmd+A: the predicate matched normalizedSelection bounds exactly (endRow === rows.length-1, endCol === displayColumns.length-1), so any post-selection growth flipped it false while the cell-range overlay still painted every row checked. Replace the structural two-branch predicate with an explicit allRowsSelected flag plus a uniform set-membership check. handleSelectAllRows sets the flag in O(1); handleRowToggle materializes checkedRows when toggling out of "all" mode. Bulk-op read sites (delete, copy, cut, selectedRowCount) honor the flag. Decouple gutter checkbox from cell-range drag: dragging cells no longer fills gutter checkboxes — they reflect explicit row-selection intent only, matching Sheets/Airtable. Cell-range overlay still paints cells. * refactor(tables): row selection as discriminated union Collapse `checkedRows: Set<string>` + `allRowsSelected: boolean` into a single `RowSelection = { kind: 'none' | 'some' | 'all' }`. Impossible states (all + non-empty Set) become unrepresentable; predicates like `rowSelectionIncludes` and `rowSelectionIsEmpty` replace ad-hoc checks at every read site. * fix(tables): clear row selection after context-menu delete handleContextMenuDelete dispatched the delete but left rowSelection at its prior 'all' or 'some' state. After rows clear and a new row arrives (realtime, undo, append), rowSelectionIncludes returned true for it, rendering it checked and flipping the master checkbox back on. * chore(tables): address review nits on row selection refactor - guard selectedRowCount 'all' branch on contextRow membership in rows - restore blank line between row-selection helpers and constants * chore(tables): rename rowSelectionChanged to cellRangeRowChanged The helper compares NormalizedSelection (cell-range) state for a given row, not RowSelection. The old name collided with the new row-selection discriminated union and read ambiguously. * fix(tables): guard context-menu delete on stale rows, preserve selection on cancel - Guard the kind='all' branch on contextRow membership in currentRows (matches the same fix applied to selectedRowCount), so a context menu on a stale row no longer deletes the entire table. - Drop the eager rowSelection clear at modal-open time. The modal's onSuccess already calls handleClearSelection after the mutation resolves, so the post-delete invariant still holds; if the user cancels, the selection is now preserved.
2026-09-24 15:45:35 +08:00 · 2026-05-05 22:10:24 -07:00
parent 80eb5b9a6e
commit 1814105709
1 changed files with 93 additions and 72 deletions
@@ -85,7 +85,38 @@ import {

 const logger = createLogger('TableView')

-const EMPTY_CHECKED_ROWS = new Set<string>()
+type RowSelection = { kind: 'none' } | { kind: 'some'; ids: Set<string> } | { kind: 'all' }
+
+const ROW_SELECTION_NONE: RowSelection = { kind: 'none' }
+const ROW_SELECTION_ALL: RowSelection = { kind: 'all' }
+
+function rowSelectionIncludes(sel: RowSelection, id: string): boolean {
+  if (sel.kind === 'all') return true
+  if (sel.kind === 'some') return sel.ids.has(id)
+  return false
+}
+
+function rowSelectionIsEmpty(sel: RowSelection): boolean {
+  if (sel.kind === 'none') return true
+  if (sel.kind === 'some') return sel.ids.size === 0
+  return false
+}
+
+function rowSelectionMaterialize(sel: RowSelection, rows: TableRowType[]): Set<string> {
+  if (sel.kind === 'all') return new Set(rows.map((r) => r.id))
+  if (sel.kind === 'some') return new Set(sel.ids)
+  return new Set<string>()
+}
+
+function rowSelectionCoversAll(sel: RowSelection, rows: TableRowType[]): boolean {
+  if (rows.length === 0) return false
+  if (sel.kind === 'all') return true
+  if (sel.kind === 'none') return false
+  if (sel.ids.size < rows.length) return false
+  for (const r of rows) if (!sel.ids.has(r.id)) return false
+  return true
+}
+
 const COL_WIDTH_MIN = 80
 const COL_WIDTH_AUTO_FIT_MAX = 1000
 // Wide enough to host the row-number + per-row run button side by side.
@@ -143,7 +174,7 @@ export function Table({
  const [expandedCell, setExpandedCell] = useState<EditingCell | null>(null)
  const [selectionAnchor, setSelectionAnchor] = useState<CellCoord | null>(null)
  const [selectionFocus, setSelectionFocus] = useState<CellCoord | null>(null)
-  const [checkedRows, setCheckedRows] = useState(EMPTY_CHECKED_ROWS)
+  const [rowSelection, setRowSelection] = useState<RowSelection>(ROW_SELECTION_NONE)
  const [isColumnSelection, setIsColumnSelection] = useState(false)
  const lastCheckboxRowRef = useRef<string | null>(null)
  const isColumnSelectionRef = useRef(false)
@@ -379,22 +410,10 @@ export function Table({
    return null
  }, [dropTargetColumnName, dragColumnName, dropSide, displayColumns, columnWidths])

-  const isAllRowsSelected = useMemo(() => {
-    if (checkedRows.size > 0 && rows.length > 0 && checkedRows.size >= rows.length) {
-      for (const row of rows) {
-        if (!checkedRows.has(row.id)) return false
-      }
-      return true
-    }
-    return (
-      normalizedSelection !== null &&
-      rows.length > 0 &&
-      normalizedSelection.startRow === 0 &&
-      normalizedSelection.endRow === rows.length - 1 &&
-      normalizedSelection.startCol === 0 &&
-      normalizedSelection.endCol === displayColumns.length - 1
-    )
-  }, [checkedRows, normalizedSelection, displayColumns.length, rows])
+  const isAllRowsSelected = useMemo(
+    () => rowSelectionCoversAll(rowSelection, rows),
+    [rowSelection, rows]
+  )

  const isAllRowsSelectedRef = useRef(isAllRowsSelected)
  isAllRowsSelectedRef.current = isAllRowsSelected
@@ -408,8 +427,8 @@ export function Table({
  const anchorRowIdRef = useRef<string | null>(null)
  const focusRowIdRef = useRef<string | null>(null)

-  const checkedRowsRef = useRef(checkedRows)
-  checkedRowsRef.current = checkedRows
+  const rowSelectionRef = useRef(rowSelection)
+  rowSelectionRef.current = rowSelection

  columnsRef.current = displayColumns
  schemaColumnsRef.current = columns
@@ -498,12 +517,16 @@ export function Table({
      return
    }

-    const checked = checkedRowsRef.current
+    const rowSel = rowSelectionRef.current
    const currentRows = rowsRef.current
    let snapshots: DeletedRowSnapshot[] = []

-    if (checked.size > 0 && checked.has(contextRow.id)) {
-      snapshots = collectRowSnapshots(currentRows.filter((r) => checked.has(r.id)))
+    const contextRowInRows = currentRows.some((r) => r.id === contextRow.id)
+
+    if (rowSel.kind === 'all' && contextRowInRows) {
+      snapshots = collectRowSnapshots(currentRows)
+    } else if (rowSel.kind === 'some' && rowSel.ids.has(contextRow.id)) {
+      snapshots = collectRowSnapshots(currentRows.filter((r) => rowSel.ids.has(r.id)))
    } else {
      const sel = computeNormalizedSelection(selectionAnchorRef.current, selectionFocusRef.current)
      const contextRowArrayIndex = currentRows.findIndex((r) => r.id === contextRow.id)
@@ -677,7 +700,7 @@ export function Table({

  const handleCellMouseDown = useCallback(
    (rowIndex: number, colIndex: number, shiftKey: boolean) => {
-      setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+      setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
      setIsColumnSelection(false)
      lastCheckboxRowRef.current = null
      if (shiftKey && selectionAnchorRef.current) {
@@ -713,25 +736,22 @@ export function Table({
        ? currentRows.findIndex((r) => r.id === lastCheckboxRowRef.current)
        : -1

-    if (lastIdx !== -1) {
-      const from = Math.min(lastIdx, rowIndex)
-      const to = Math.max(lastIdx, rowIndex)
-      setCheckedRows((prev) => {
-        const next = new Set(prev)
+    setRowSelection((prev) => {
+      const next = rowSelectionMaterialize(prev, currentRows)
+      if (lastIdx !== -1) {
+        const from = Math.min(lastIdx, rowIndex)
+        const to = Math.max(lastIdx, rowIndex)
        for (let i = from; i <= to; i++) {
          const r = currentRows[i]
          if (r) next.add(r.id)
        }
-        return next
-      })
-    } else {
-      setCheckedRows((prev) => {
-        const next = new Set(prev)
-        if (next.has(targetId)) next.delete(targetId)
-        else next.add(targetId)
-        return next
-      })
-    }
+      } else if (next.has(targetId)) {
+        next.delete(targetId)
+      } else {
+        next.add(targetId)
+      }
+      return next.size === 0 ? ROW_SELECTION_NONE : { kind: 'some', ids: next }
+    })
    lastCheckboxRowRef.current = targetId
    scrollRef.current?.focus({ preventScroll: true })
  }, [])
@@ -739,7 +759,7 @@ export function Table({
  const handleClearSelection = useCallback(() => {
    setSelectionAnchor(null)
    setSelectionFocus(null)
-    setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+    setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
    setIsColumnSelection(false)
    lastCheckboxRowRef.current = null
  }, [])
@@ -749,7 +769,7 @@ export function Table({
    if (lastRow < 0) return

    setEditingCell(null)
-    setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+    setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
    lastCheckboxRowRef.current = null

    if (shiftKey && isColumnSelectionRef.current && selectionAnchorRef.current) {
@@ -768,7 +788,7 @@ export function Table({
    if (lastRow < 0) return

    setEditingCell(null)
-    setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+    setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
    lastCheckboxRowRef.current = null

    setSelectionAnchor({ rowIndex: 0, colIndex: startColIndex })
@@ -783,7 +803,7 @@ export function Table({
    const currentCols = columnsRef.current
    if (rws.length === 0 || currentCols.length === 0) return
    setEditingCell(null)
-    setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+    setRowSelection(ROW_SELECTION_ALL)
    lastCheckboxRowRef.current = null
    suppressFocusScrollRef.current = true
    setSelectionAnchor({ rowIndex: 0, colIndex: 0 })
@@ -875,7 +895,7 @@ export function Table({
    setDragColumnName(columnName)
    setSelectionAnchor(null)
    setSelectionFocus(null)
-    setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+    setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
    setIsColumnSelection(false)
  }, [])

@@ -1339,7 +1359,7 @@ export function Table({
        }
        setSelectionAnchor(null)
        setSelectionFocus(null)
-        setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+        setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
        setIsColumnSelection(false)
        lastCheckboxRowRef.current = null
        return
@@ -1352,7 +1372,7 @@ export function Table({
        if (rws.length > 0 && currentCols.length > 0) {
          suppressFocusScrollRef.current = true
          setEditingCell(null)
-          setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+          setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
          lastCheckboxRowRef.current = null
          setSelectionAnchor({ rowIndex: 0, colIndex: 0 })
          setSelectionFocus({
@@ -1370,7 +1390,7 @@ export function Table({
        const lastRow = rowsRef.current.length - 1
        if (lastRow < 0) return
        e.preventDefault()
-        setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+        setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
        lastCheckboxRowRef.current = null
        setSelectionAnchor({ rowIndex: 0, colIndex: a.colIndex })
        setSelectionFocus({ rowIndex: lastRow, colIndex: a.colIndex })
@@ -1384,7 +1404,7 @@ export function Table({
        const currentCols = columnsRef.current
        if (currentCols.length === 0) return
        e.preventDefault()
-        setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+        setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
        lastCheckboxRowRef.current = null
        setIsColumnSelection(false)
        setSelectionAnchor({ rowIndex: a.rowIndex, colIndex: 0 })
@@ -1392,17 +1412,20 @@ export function Table({
        return
      }

-      if ((e.key === 'Delete' || e.key === 'Backspace') && checkedRowsRef.current.size > 0) {
+      if (
+        (e.key === 'Delete' || e.key === 'Backspace') &&
+        !rowSelectionIsEmpty(rowSelectionRef.current)
+      ) {
        if (editingCellRef.current) return
        if (!canEditRef.current) return
        e.preventDefault()
-        const checked = checkedRowsRef.current
+        const rowSel = rowSelectionRef.current
        const currentRows = rowsRef.current
        const currentCols = columnsRef.current
        const undoCells: Array<{ rowId: string; data: Record<string, unknown> }> = []
        const batchUpdates: Array<{ rowId: string; data: Record<string, unknown> }> = []
        for (const row of currentRows) {
-          if (!checked.has(row.id)) continue
+          if (!rowSelectionIncludes(rowSel, row.id)) continue
          const updates: Record<string, unknown> = {}
          const previousData: Record<string, unknown> = {}
          for (const col of currentCols) {
@@ -1481,7 +1504,7 @@ export function Table({

      if (e.key === 'Tab') {
        e.preventDefault()
-        setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+        setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
        setIsColumnSelection(false)
        lastCheckboxRowRef.current = null
        setSelectionAnchor(moveCell(anchor, cols.length, totalRows, e.shiftKey ? -1 : 1))
@@ -1491,7 +1514,7 @@ export function Table({

      if (['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight'].includes(e.key)) {
        e.preventDefault()
-        setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
+        setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE))
        setIsColumnSelection(false)
        lastCheckboxRowRef.current = null
        const focus = selectionFocusRef.current ?? anchor
@@ -1669,15 +1692,15 @@ export function Table({
      if (tag === 'INPUT' || tag === 'TEXTAREA') return
      if (editingCellRef.current) return

-      const checked = checkedRowsRef.current
+      const rowSel = rowSelectionRef.current
      const cols = columnsRef.current
      const currentRows = rowsRef.current

-      if (checked.size > 0) {
+      if (!rowSelectionIsEmpty(rowSel)) {
        e.preventDefault()
        const lines: string[] = []
        for (const row of currentRows) {
-          if (!checked.has(row.id)) continue
+          if (!rowSelectionIncludes(rowSel, row.id)) continue
          const cells: string[] = cols.map((col) => {
            const value: unknown = row.data[col.name]
            if (value === null || value === undefined) return ''
@@ -1720,17 +1743,17 @@ export function Table({
      if (editingCellRef.current) return
      if (!canEditRef.current) return

-      const checked = checkedRowsRef.current
+      const rowSel = rowSelectionRef.current
      const cols = columnsRef.current
      const currentRows = rowsRef.current
      const undoCells: Array<{ rowId: string; data: Record<string, unknown> }> = []
      const batchUpdates: Array<{ rowId: string; data: Record<string, unknown> }> = []

-      if (checked.size > 0) {
+      if (!rowSelectionIsEmpty(rowSel)) {
        e.preventDefault()
        const lines: string[] = []
        for (const row of currentRows) {
-          if (!checked.has(row.id)) continue
+          if (!rowSelectionIncludes(rowSel, row.id)) continue
          const cells: string[] = cols.map((col) => {
            const value: unknown = row.data[col.name]
            if (value === null || value === undefined) return ''
@@ -2425,10 +2448,14 @@ export function Table({
    const contextRow = contextMenu.isOpen ? contextMenu.row : null
    if (!contextRow) return 1

-    if (checkedRows.size > 0 && checkedRows.has(contextRow.id)) {
+    if (rowSelection.kind === 'all') {
+      return rows.some((r) => r.id === contextRow.id) ? Math.max(rows.length, 1) : 1
+    }
+
+    if (rowSelection.kind === 'some' && rowSelection.ids.has(contextRow.id)) {
      let count = 0
      for (const row of rows) {
-        if (checkedRows.has(row.id)) count++
+        if (rowSelection.ids.has(row.id)) count++
      }
      return Math.max(count, 1)
    }
@@ -2442,7 +2469,7 @@ export function Table({
    const start = Math.max(0, sel.startRow)
    const end = Math.min(rows.length - 1, sel.endRow)
    return Math.max(end - start + 1, 1)
-  }, [contextMenu.isOpen, contextMenu.row, checkedRows, normalizedSelection, rows])
+  }, [contextMenu.isOpen, contextMenu.row, rowSelection, normalizedSelection, rows])

  const pendingUpdate = updateRowMutation.isPending ? updateRowMutation.variables : null

@@ -2756,7 +2783,7 @@ export function Table({
                        onContextMenu={handleRowContextMenu}
                        onCellMouseDown={handleCellMouseDown}
                        onCellMouseEnter={handleCellMouseEnter}
-                        isRowChecked={checkedRows.has(row.id)}
+                        isRowChecked={rowSelectionIncludes(rowSelection, row.id)}
                        onRowToggle={handleRowToggle}
                        runningCount={runningByRowId.get(row.id) ?? 0}
                        hasWorkflowColumns={hasWorkflowColumns}
@@ -3012,7 +3039,7 @@ interface DataRowProps {
  workflowNameById: Record<string, string>
 }

-function rowSelectionChanged(
+function cellRangeRowChanged(
  rowIndex: number,
  colCount: number,
  prev: NormalizedSelection | null,
@@ -3075,7 +3102,7 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean {
    return false
  }

-  return !rowSelectionChanged(
+  return !cellRangeRowChanged(
    prev.rowIndex,
    prev.columns.length,
    prev.normalizedSelection,
@@ -3109,13 +3136,7 @@ const DataRow = React.memo(function DataRow({
 }: DataRowProps) {
  const sel = normalizedSelection
  const isMultiCell = sel !== null && (sel.startRow !== sel.endRow || sel.startCol !== sel.endCol)
-  const isRowSelectedByRange =
-    sel !== null &&
-    rowIndex >= sel.startRow &&
-    rowIndex <= sel.endRow &&
-    sel.startCol === 0 &&
-    sel.endCol === columns.length - 1
-  const isRowSelected = isRowChecked || isRowSelectedByRange
+  const isRowSelected = isRowChecked

  return (
    <tr onContextMenu={(e) => onContextMenu(e, row)}>