Skip to content

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

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

Merged
waleedlatif1 merged 6 commits into from
May 6, 2026
+93 −72
Merged

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

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22b555e
fix(tables): decouple master checkbox from cell-range, add allRowsSel…
waleedlatif1 May 6, 2026
d1c0eed
refactor(tables): row selection as discriminated union
waleedlatif1 May 6, 2026
3fa050d
fix(tables): clear row selection after context-menu delete
waleedlatif1 May 6, 2026
0f986cf
chore(tables): address review nits on row selection refactor
waleedlatif1 May 6, 2026
fa8733c
chore(tables): rename rowSelectionChanged to cellRangeRowChanged
waleedlatif1 May 6, 2026
e64787e
fix(tables): guard context-menu delete on stale rows, preserve select…
waleedlatif1 May 6, 2026
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
165 changes: 93 additions & 72 deletions apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
waleedlatif1 marked this conversation as resolved.
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
Comment thread
waleedlatif1 marked this conversation as resolved.
const COL_WIDTH_AUTO_FIT_MAX = 1000
// Wide enough to host the row-number + per-row run button side by side.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Comment thread
waleedlatif1 marked this conversation as resolved.
} 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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -713,33 +736,30 @@ 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 })
}, [])

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
}, [])
Expand All @@ -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) {
Expand All @@ -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 })
Expand All @@ -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 })
Expand Down Expand Up @@ -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)
}, [])

Expand Down Expand Up @@ -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
Expand All @@ -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({
Expand All @@ -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 })
Expand All @@ -1384,25 +1404,28 @@ 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 })
setSelectionFocus({ rowIndex: a.rowIndex, colIndex: currentCols.length - 1 })
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) {
Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand Down Expand Up @@ -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 ''
Expand Down Expand Up @@ -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 ''
Expand Down Expand Up @@ -2425,10 +2448,14 @@ export function Table({
const contextRow = contextMenu.isOpen ? contextMenu.row : null
Comment thread
waleedlatif1 marked this conversation as resolved.
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)
}
Expand All @@ -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

Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -3012,7 +3039,7 @@ interface DataRowProps {
workflowNameById: Record<string, string>
}

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

return !rowSelectionChanged(
return !cellRangeRowChanged(
prev.rowIndex,
prev.columns.length,
prev.normalizedSelection,
Expand Down Expand Up @@ -3109,13 +3136,7 @@ const DataRow = React.memo(function DataRow({
}: DataRowProps) {
Comment thread
waleedlatif1 marked this conversation as resolved.
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)}>
Expand Down
Loading