From 9ad8b12e671e171678fbb8b554469eeb7cdee4c3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 4 May 2026 22:46:34 -0700 Subject: [PATCH 1/4] =?UTF-8?q?refactor(tables):=20phase=201=20=E2=80=94?= =?UTF-8?q?=20gutter=20numbering=20from=20array=20index?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete the PositionGapRows component, the gap-fill loop, and the GAP_CHECKBOX_CLASS / GAP_ROW_LIMIT / PositionGapRowsProps surface area. The server's recompactPositions() guarantees positions are 0..N-1 contiguous in the unfiltered view, so the phantom-row machinery has been defending against a state that essentially never happens. DataRow now receives an arrayIndex prop and renders {arrayIndex + 1} in the gutter. Selection coordinates still flow through row.position; that switches in phase 2. Co-Authored-By: Claude Opus 4.7 --- .../[tableId]/components/table/table.tsx | 246 +++--------------- 1 file changed, 38 insertions(+), 208 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx index 08ba169d1e..eefa619ff7 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx @@ -2690,62 +2690,40 @@ export function Table({ ) : ( <> - {rows.map((row, index) => { - const prevPosition = index > 0 ? rows[index - 1].position : -1 - const gapCount = - queryOptions.filter || queryOptions.sort - ? 0 - : row.position - prevPosition - 1 - return ( - - {gapCount > 0 && ( - - )} - - - ) - })} + {rows.map((row, index) => ( + + ))} )} @@ -2947,157 +2925,6 @@ export function Table({ ) } -const GAP_ROW_LIMIT = 200 -const GAP_CHECKBOX_CLASS = cn(CELL_CHECKBOX, 'cursor-pointer') - -interface PositionGapRowsProps { - count: number - startPosition: number - columns: DisplayColumn[] - normalizedSelection: NormalizedSelection | null - checkedRows: Set - firstRowUnderHeader?: boolean - onCellMouseDown: (rowIndex: number, colIndex: number, shiftKey: boolean) => void - onCellMouseEnter: (rowIndex: number, colIndex: number) => void - onRowToggle: (rowIndex: number, shiftKey: boolean) => void -} - -const PositionGapRows = React.memo( - function PositionGapRows({ - count, - startPosition, - columns, - normalizedSelection, - checkedRows, - firstRowUnderHeader = false, - onCellMouseDown, - onCellMouseEnter, - onRowToggle, - }: PositionGapRowsProps) { - const capped = Math.min(count, GAP_ROW_LIMIT) - const sel = normalizedSelection - const isMultiCell = sel !== null && (sel.startRow !== sel.endRow || sel.startCol !== sel.endCol) - - return ( - <> - {Array.from({ length: capped }).map((_, i) => { - const position = startPosition + i - const isGapChecked = checkedRows.has(position) - return ( - - -
-
{ - if (e.button !== 0) return - onRowToggle(position, e.shiftKey) - }} - > - - {position + 1} - -
- -
-
-
- - {columns.map((col, colIndex) => { - const inRange = - sel !== null && - position >= sel.startRow && - position <= sel.endRow && - colIndex >= sel.startCol && - colIndex <= sel.endCol - const isAnchor = - sel !== null && position === sel.anchorRow && colIndex === sel.anchorCol - const isHighlighted = inRange || isGapChecked - - const isTopEdge = inRange ? position === sel!.startRow : isGapChecked - const isBottomEdge = inRange ? position === sel!.endRow : isGapChecked - const isLeftEdge = inRange ? colIndex === sel!.startCol : colIndex === 0 - const isRightEdge = inRange - ? colIndex === sel!.endCol - : colIndex === columns.length - 1 - const belowHeader = firstRowUnderHeader && i === 0 - - return ( - { - if (e.button !== 0) return - onCellMouseDown(position, colIndex, e.shiftKey) - }} - onMouseEnter={() => onCellMouseEnter(position, colIndex)} - > - {isHighlighted && (isMultiCell || isGapChecked) && ( -
- )} - {isAnchor &&
} -
- - ) - })} - - ) - })} - {count > GAP_ROW_LIMIT && ( - - - - )} - - ) - }, - (prev, next) => { - if ( - prev.count !== next.count || - prev.startPosition !== next.startPosition || - prev.columns !== next.columns || - prev.normalizedSelection !== next.normalizedSelection || - prev.firstRowUnderHeader !== next.firstRowUnderHeader || - prev.onCellMouseDown !== next.onCellMouseDown || - prev.onCellMouseEnter !== next.onCellMouseEnter || - prev.onRowToggle !== next.onRowToggle - ) { - return false - } - const end = prev.startPosition + Math.min(prev.count, GAP_ROW_LIMIT) - for (let p = prev.startPosition; p < end; p++) { - if (prev.checkedRows.has(p) !== next.checkedRows.has(p)) return false - } - return true - } -) - const TableColGroup = React.memo(function TableColGroup({ columns, columnWidths, @@ -3120,6 +2947,7 @@ interface DataRowProps { row: TableRowType columns: DisplayColumn[] rowIndex: number + arrayIndex: number isFirstRow: boolean editingColumnName: string | null initialCharacter: string | null @@ -3180,6 +3008,7 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean { prev.row !== next.row || prev.columns !== next.columns || prev.rowIndex !== next.rowIndex || + prev.arrayIndex !== next.arrayIndex || prev.isFirstRow !== next.isFirstRow || prev.editingColumnName !== next.editingColumnName || prev.pendingCellValue !== next.pendingCellValue || @@ -3219,6 +3048,7 @@ const DataRow = React.memo(function DataRow({ row, columns, rowIndex, + arrayIndex, isFirstRow, editingColumnName, initialCharacter, @@ -3266,7 +3096,7 @@ const DataRow = React.memo(function DataRow({ isRowSelected ? 'hidden' : 'block group-hover/checkbox:hidden' )} > - {row.position + 1} + {arrayIndex + 1}
Date: Mon, 4 May 2026 22:58:59 -0700 Subject: [PATCH 2/4] =?UTF-8?q?refactor(tables):=20phase=202-5=20=E2=80=94?= =?UTF-8?q?=20selection=20by=20array=20index,=20checks=20by=20row=20id?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decouple Tables UI selection coordinates from DB position: - rowIndex semantics shift from row.position to array index across selection state, mouse handlers, keyboard nav, paste, scrollIntoView - checkedRows: Set (position) → Set (rowId), survives sort/filter and realtime row inserts - lastCheckboxRowRef stores rowId; shift-click range resolves to current array indices for visual-order ranges - Drop positionMap/maxPosition derived state in favor of direct rowsRef reads - ExpandedCellPopover anchors via data-row-id (row-id-stable) instead of data-row (array index) - collectRowSnapshots accepts Iterable directly - Add bounds-validation effect to clamp anchor/focus when rows.length shrinks (sort change, pagination, realtime delete) - Drop redundant arrayIndex prop on DataRow (rowIndex now equals it) Server-side position math stays at API boundary only: insertRow, duplicateRow, shift-Enter append, paste create-batch, undo snapshots. Co-Authored-By: Claude Opus 4.7 --- .../table/cells/expanded-cell-popover.tsx | 2 +- .../[tableId]/components/table/table.tsx | 222 +++++++++--------- .../[tableId]/components/table/utils.ts | 12 +- 3 files changed, 117 insertions(+), 119 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/cells/expanded-cell-popover.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/cells/expanded-cell-popover.tsx index 388622c82b..cc71632069 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/cells/expanded-cell-popover.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/cells/expanded-cell-popover.tsx @@ -78,7 +78,7 @@ export function ExpandedCellPopover({ return } setDraftValue(isEditable ? formatValueForInput(target.value, target.column.type) : '') - const selector = `[data-table-scroll] [data-row="${target.row.position}"][data-col="${target.colIndex}"]` + const selector = `[data-table-scroll] [data-row-id="${target.row.id}"][data-col="${target.colIndex}"]` const el = document.querySelector(selector) if (!el) { setRect(null) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx index eefa619ff7..7628431295 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx @@ -85,7 +85,7 @@ import { const logger = createLogger('TableView') -const EMPTY_CHECKED_ROWS = new Set() +const EMPTY_CHECKED_ROWS = new Set() 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. @@ -147,7 +147,7 @@ export function Table({ const [selectionFocus, setSelectionFocus] = useState(null) const [checkedRows, setCheckedRows] = useState(EMPTY_CHECKED_ROWS) const [isColumnSelection, setIsColumnSelection] = useState(false) - const lastCheckboxRowRef = useRef(null) + const lastCheckboxRowRef = useRef(null) const isColumnSelectionRef = useRef(false) const [showDeleteTableConfirm, setShowDeleteTableConfirm] = useState(false) const [deletingColumns, setDeletingColumns] = useState(null) @@ -321,20 +321,6 @@ export function Table({ ) const hasWorkflowGroup = headerGroups.some((g) => g.kind === 'workflow') - const maxPosition = useMemo(() => (rows.length > 0 ? rows[rows.length - 1].position : -1), [rows]) - const maxPositionRef = useRef(maxPosition) - maxPositionRef.current = maxPosition - - const positionMap = useMemo(() => { - const map = new Map() - for (const row of rows) { - map.set(row.position, row) - } - return map - }, [rows]) - const positionMapRef = useRef(positionMap) - positionMapRef.current = positionMap - const normalizedSelection = useMemo( () => computeNormalizedSelection(selectionAnchor, selectionFocus), [selectionAnchor, selectionFocus] @@ -401,19 +387,19 @@ export function Table({ const isAllRowsSelected = useMemo(() => { if (checkedRows.size > 0 && rows.length > 0 && checkedRows.size >= rows.length) { for (const row of rows) { - if (!checkedRows.has(row.position)) return false + if (!checkedRows.has(row.id)) return false } return true } return ( normalizedSelection !== null && - maxPosition >= 0 && + rows.length > 0 && normalizedSelection.startRow === 0 && - normalizedSelection.endRow === maxPosition && + normalizedSelection.endRow === rows.length - 1 && normalizedSelection.startCol === 0 && normalizedSelection.endCol === displayColumns.length - 1 ) - }, [checkedRows, normalizedSelection, maxPosition, displayColumns.length, rows]) + }, [checkedRows, normalizedSelection, displayColumns.length, rows]) const isAllRowsSelectedRef = useRef(isAllRowsSelected) isAllRowsSelectedRef.current = isAllRowsSelected @@ -515,24 +501,20 @@ export function Table({ } const checked = checkedRowsRef.current - const pMap = positionMapRef.current + const currentRows = rowsRef.current let snapshots: DeletedRowSnapshot[] = [] - if (checked.size > 0 && checked.has(contextMenu.row.position)) { - snapshots = collectRowSnapshots(checked, pMap) + const contextRowArrayIndex = currentRows.findIndex((r) => r.id === contextMenu.row!.id) + + if (checked.size > 0 && checked.has(contextMenu.row.id)) { + snapshots = collectRowSnapshots(currentRows.filter((r) => checked.has(r.id))) } else { const sel = computeNormalizedSelection(selectionAnchorRef.current, selectionFocusRef.current) const isInSelection = - sel !== null && - contextMenu.row.position >= sel.startRow && - contextMenu.row.position <= sel.endRow + sel !== null && contextRowArrayIndex >= sel.startRow && contextRowArrayIndex <= sel.endRow if (isInSelection && sel) { - const positions = Array.from( - { length: sel.endRow - sel.startRow + 1 }, - (_, i) => sel.startRow + i - ) - snapshots = collectRowSnapshots(positions, pMap) + snapshots = collectRowSnapshots(currentRows.slice(sel.startRow, sel.endRow + 1)) } else { snapshots = [ { @@ -603,6 +585,7 @@ export function Table({ if (!contextMenu.row) return const rowData = { ...contextMenu.row.data } const position = contextMenu.row.position + 1 + const sourceArrayIndex = rowsRef.current.findIndex((r) => r.id === contextMenu.row!.id) closeContextMenu() createRef.current( { data: rowData, position }, @@ -618,8 +601,10 @@ export function Table({ }) } const colIndex = selectionAnchorRef.current?.colIndex ?? 0 - setSelectionAnchor({ rowIndex: position, colIndex }) - setSelectionFocus(null) + if (sourceArrayIndex !== -1) { + setSelectionAnchor({ rowIndex: sourceArrayIndex + 1, colIndex }) + setSelectionFocus(null) + } }, } ) @@ -646,10 +631,11 @@ export function Table({ onSuccess: (response: Record) => { const newRowId = extractCreatedRowId(response) if (newRowId) { + const lastRow = rowsRef.current[rowsRef.current.length - 1] pushUndoRef.current({ type: 'create-row', rowId: newRowId, - position: maxPositionRef.current + 1, + position: (lastRow?.position ?? -1) + 1, }) } }, @@ -723,29 +709,44 @@ export function Table({ setSelectionFocus(null) setIsColumnSelection(false) + const currentRows = rowsRef.current + const targetRow = currentRows[rowIndex] + if (!targetRow) return + const targetId = targetRow.id + if (shiftKey && lastCheckboxRowRef.current !== null) { - const from = Math.min(lastCheckboxRowRef.current, rowIndex) - const to = Math.max(lastCheckboxRowRef.current, rowIndex) - const pMap = positionMapRef.current - setCheckedRows((prev) => { - const next = new Set(prev) - for (const [pos] of pMap) { - if (pos >= from && pos <= to) next.add(pos) - } - return next - }) + const lastIdx = currentRows.findIndex((r) => r.id === lastCheckboxRowRef.current) + if (lastIdx !== -1) { + const from = Math.min(lastIdx, rowIndex) + const to = Math.max(lastIdx, rowIndex) + setCheckedRows((prev) => { + const next = new Set(prev) + 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 { setCheckedRows((prev) => { const next = new Set(prev) - if (next.has(rowIndex)) { - next.delete(rowIndex) + if (next.has(targetId)) { + next.delete(targetId) } else { - next.add(rowIndex) + next.add(targetId) } return next }) } - lastCheckboxRowRef.current = rowIndex + lastCheckboxRowRef.current = targetId scrollRef.current?.focus({ preventScroll: true }) }, []) @@ -758,7 +759,7 @@ export function Table({ }, []) const handleColumnSelect = useCallback((colIndex: number, shiftKey: boolean) => { - const lastRow = maxPositionRef.current + const lastRow = rowsRef.current.length - 1 if (lastRow < 0) return setEditingCell(null) @@ -777,7 +778,7 @@ export function Table({ }, []) const handleGroupSelect = useCallback((startColIndex: number, size: number) => { - const lastRow = maxPositionRef.current + const lastRow = rowsRef.current.length - 1 if (lastRow < 0) return setEditingCell(null) @@ -800,7 +801,7 @@ export function Table({ suppressFocusScrollRef.current = true setSelectionAnchor({ rowIndex: 0, colIndex: 0 }) setSelectionFocus({ - rowIndex: maxPositionRef.current, + rowIndex: rws.length - 1, colIndex: currentCols.length - 1, }) setIsColumnSelection(false) @@ -1139,13 +1140,15 @@ export function Table({ useEffect(() => { if (!isColumnSelection || !selectionAnchor) return + const lastRow = rows.length - 1 + if (lastRow < 0) return setSelectionFocus((prev) => { - if (!prev || prev.rowIndex !== maxPosition) { - return { rowIndex: maxPosition, colIndex: prev?.colIndex ?? selectionAnchor.colIndex } + if (!prev || prev.rowIndex !== lastRow) { + return { rowIndex: lastRow, colIndex: prev?.colIndex ?? selectionAnchor.colIndex } } return prev }) - }, [isColumnSelection, maxPosition, selectionAnchor]) + }, [isColumnSelection, rows.length, selectionAnchor]) useEffect(() => { const handleMouseUp = () => { @@ -1155,6 +1158,16 @@ export function Table({ return () => document.removeEventListener('mouseup', handleMouseUp) }, []) + useEffect(() => { + const len = rows.length + if (selectionAnchorRef.current && selectionAnchorRef.current.rowIndex >= len) { + setSelectionAnchor(null) + } + if (selectionFocusRef.current && selectionFocusRef.current.rowIndex >= len) { + setSelectionFocus(null) + } + }, [rows.length]) + useEffect(() => { if (isColumnSelection) return if (suppressFocusScrollRef.current) { @@ -1201,12 +1214,13 @@ export function Table({ setSelectionFocus(null) setIsColumnSelection(false) - const row = rowsRef.current.find((r) => r.id === rowId) + const rowArrayIndex = rowsRef.current.findIndex((r) => r.id === rowId) + const row = rowArrayIndex !== -1 ? rowsRef.current[rowArrayIndex] : null const colIndex = columnsRef.current.findIndex((c) => c.key === columnKey) let overflows = true if (row && colIndex !== -1) { const td = document.querySelector( - `[data-table-scroll] [data-row="${row.position}"][data-col="${colIndex}"]` + `[data-table-scroll] [data-row="${rowArrayIndex}"][data-col="${colIndex}"]` ) const inner = td?.querySelector(':scope > div:last-child') if (inner) { @@ -1307,7 +1321,7 @@ export function Table({ setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS)) setSelectionAnchor({ rowIndex: 0, colIndex: 0 }) setSelectionFocus({ - rowIndex: maxPositionRef.current, + rowIndex: rws.length - 1, colIndex: currentCols.length - 1, }) setIsColumnSelection(false) @@ -1318,7 +1332,7 @@ export function Table({ if ((e.metaKey || e.ctrlKey) && e.key === ' ') { const a = selectionAnchorRef.current if (!a || editingCellRef.current) return - const lastRow = maxPositionRef.current + const lastRow = rowsRef.current.length - 1 if (lastRow < 0) return e.preventDefault() setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS)) @@ -1348,13 +1362,12 @@ export function Table({ if (!canEditRef.current) return e.preventDefault() const checked = checkedRowsRef.current - const pMap = positionMapRef.current + const currentRows = rowsRef.current const currentCols = columnsRef.current const undoCells: Array<{ rowId: string; data: Record }> = [] const batchUpdates: Array<{ rowId: string; data: Record }> = [] - for (const pos of checked) { - const row = pMap.get(pos) - if (!row) continue + for (const row of currentRows) { + if (!checked.has(row.id)) continue const updates: Record = {} const previousData: Record = {} for (const col of currentCols) { @@ -1377,12 +1390,12 @@ export function Table({ if (!anchor || editingCellRef.current) return const cols = columnsRef.current - const mp = maxPositionRef.current - const totalRows = mp + 1 + const currentRows = rowsRef.current + const totalRows = currentRows.length if (e.shiftKey && e.key === 'Enter') { if (!canEditRef.current) return - const row = positionMapRef.current.get(anchor.rowIndex) + const row = currentRows[anchor.rowIndex] if (!row) return e.preventDefault() const position = row.position + 1 @@ -1409,7 +1422,7 @@ export function Table({ const col = cols[anchor.colIndex] if (!col) return - const row = positionMapRef.current.get(anchor.rowIndex) + const row = currentRows[anchor.rowIndex] if (!row) return if (col.type === 'boolean') { @@ -1424,7 +1437,7 @@ export function Table({ if (e.key === ' ' && !e.shiftKey) { if (!canEditRef.current) return e.preventDefault() - const row = positionMapRef.current.get(anchor.rowIndex) + const row = currentRows[anchor.rowIndex] if (row) { setEditingRow(row) } @@ -1536,8 +1549,7 @@ export function Table({ if (!canEditRef.current) return const sel = computeNormalizedSelection(anchor, selectionFocusRef.current) if (!sel || sel.startRow === sel.endRow) return - const pMap = positionMapRef.current - const sourceRow = pMap.get(sel.startRow) + const sourceRow = currentRows[sel.startRow] if (!sourceRow) return const undoCells: Array<{ rowId: string @@ -1545,7 +1557,7 @@ export function Table({ newData: Record }> = [] for (let r = sel.startRow + 1; r <= sel.endRow; r++) { - const row = pMap.get(r) + const row = currentRows[r] if (!row) continue const oldData: Record = {} const newData: Record = {} @@ -1572,11 +1584,10 @@ export function Table({ e.preventDefault() const sel = computeNormalizedSelection(anchor, selectionFocusRef.current) if (!sel) return - const pMap = positionMapRef.current const undoCells: Array<{ rowId: string; data: Record }> = [] const batchUpdates: Array<{ rowId: string; data: Record }> = [] for (let r = sel.startRow; r <= sel.endRow; r++) { - const row = pMap.get(r) + const row = currentRows[r] if (!row) continue const updates: Record = {} const previousData: Record = {} @@ -1610,7 +1621,7 @@ export function Table({ if (col.type === 'date' && !/[\d\-/]/.test(e.key)) return e.preventDefault() - const row = positionMapRef.current.get(anchor.rowIndex) + const row = currentRows[anchor.rowIndex] if (!row) return setEditingCell({ rowId: row.id, columnName: col.name }) setInitialCharacter(e.key) @@ -1625,15 +1636,13 @@ export function Table({ const checked = checkedRowsRef.current const cols = columnsRef.current - const pMap = positionMapRef.current + const currentRows = rowsRef.current if (checked.size > 0) { e.preventDefault() - const sorted = Array.from(checked).sort((a, b) => a - b) const lines: string[] = [] - for (const pos of sorted) { - const row = pMap.get(pos) - if (!row) continue + for (const row of currentRows) { + if (!checked.has(row.id)) continue const cells: string[] = cols.map((col) => { const value: unknown = row.data[col.name] if (value === null || value === undefined) return '' @@ -1657,7 +1666,7 @@ export function Table({ const cells: string[] = [] for (let c = sel.startCol; c <= sel.endCol; c++) { if (c >= cols.length) break - const row = pMap.get(r) + const row = currentRows[r] const value: unknown = row ? row.data[cols[c].name] : null if (value === null || value === undefined) { cells.push('') @@ -1678,17 +1687,15 @@ export function Table({ const checked = checkedRowsRef.current const cols = columnsRef.current - const pMap = positionMapRef.current + const currentRows = rowsRef.current const undoCells: Array<{ rowId: string; data: Record }> = [] const batchUpdates: Array<{ rowId: string; data: Record }> = [] if (checked.size > 0) { e.preventDefault() - const sorted = Array.from(checked).sort((a, b) => a - b) const lines: string[] = [] - for (const pos of sorted) { - const row = pMap.get(pos) - if (!row) continue + for (const row of currentRows) { + if (!checked.has(row.id)) continue const cells: string[] = cols.map((col) => { const value: unknown = row.data[col.name] if (value === null || value === undefined) return '' @@ -1715,7 +1722,7 @@ export function Table({ e.preventDefault() const lines: string[] = [] for (let r = sel.startRow; r <= sel.endRow; r++) { - const row = pMap.get(r) + const row = currentRows[r] if (!row) continue const cells: string[] = [] const updates: Record = {} @@ -1768,7 +1775,9 @@ export function Table({ if (pasteRows.length === 0) return const currentCols = columnsRef.current - const pMap = positionMapRef.current + const currentRows = rowsRef.current + const lastRowPosition = + currentRows.length > 0 ? currentRows[currentRows.length - 1].position : -1 const undoCells: Array<{ rowId: string; data: Record }> = [] const updateBatch: Array<{ rowId: string; data: Record }> = [] @@ -1776,7 +1785,7 @@ export function Table({ const createBatchPositions: number[] = [] for (let r = 0; r < pasteRows.length; r++) { - const targetRow = currentAnchor.rowIndex + r + const targetArrayIndex = currentAnchor.rowIndex + r const rowData: Record = {} for (let c = 0; c < pasteRows[r].length; c++) { @@ -1794,7 +1803,7 @@ export function Table({ if (Object.keys(rowData).length === 0) continue - const existingRow = pMap.get(targetRow) + const existingRow = currentRows[targetArrayIndex] if (existingRow) { const previousData: Record = {} for (const key of Object.keys(rowData)) { @@ -1804,7 +1813,7 @@ export function Table({ updateBatch.push({ rowId: existingRow.id, data: rowData }) } else { createBatchRows.push(rowData) - createBatchPositions.push(targetRow) + createBatchPositions.push(lastRowPosition + 1 + (targetArrayIndex - currentRows.length)) } } @@ -1871,7 +1880,7 @@ export function Table({ const anchor = selectionAnchorRef.current if (!anchor) return const cols = columnsRef.current - const totalRows = maxPositionRef.current + 1 + const totalRows = rowsRef.current.length if (reason === 'enter') { setSelectionAnchor({ @@ -2379,10 +2388,10 @@ export function Table({ const selectedRowCount = useMemo(() => { if (!contextMenu.isOpen || !contextMenu.row) return 1 - if (checkedRows.size > 0 && checkedRows.has(contextMenu.row.position)) { + if (checkedRows.size > 0 && checkedRows.has(contextMenu.row.id)) { let count = 0 - for (const pos of checkedRows) { - if (positionMap.has(pos)) count++ + for (const row of rows) { + if (checkedRows.has(row.id)) count++ } return Math.max(count, 1) } @@ -2390,17 +2399,15 @@ export function Table({ const sel = normalizedSelection if (!sel) return 1 - const isInSelection = - contextMenu.row.position >= sel.startRow && contextMenu.row.position <= sel.endRow + const contextRowArrayIndex = rows.findIndex((r) => r.id === contextMenu.row!.id) + const isInSelection = contextRowArrayIndex >= sel.startRow && contextRowArrayIndex <= sel.endRow if (!isInSelection) return 1 - let count = 0 - for (let r = sel.startRow; r <= sel.endRow; r++) { - if (positionMap.has(r)) count++ - } - return Math.max(count, 1) - }, [contextMenu.isOpen, contextMenu.row, checkedRows, normalizedSelection, positionMap]) + 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]) const pendingUpdate = updateRowMutation.isPending ? updateRowMutation.variables : null @@ -2695,9 +2702,8 @@ export function Table({ key={row.id} row={row} columns={displayColumns} - rowIndex={row.position} - arrayIndex={index} - isFirstRow={row.position === 0} + rowIndex={index} + isFirstRow={index === 0} editingColumnName={ editingCell?.rowId === row.id ? editingCell.columnName : null } @@ -2715,7 +2721,7 @@ export function Table({ onContextMenu={handleRowContextMenu} onCellMouseDown={handleCellMouseDown} onCellMouseEnter={handleCellMouseEnter} - isRowChecked={checkedRows.has(row.position)} + isRowChecked={checkedRows.has(row.id)} onRowToggle={handleRowToggle} runningCount={runningByRowId.get(row.id) ?? 0} hasWorkflowColumns={hasWorkflowColumns} @@ -2947,7 +2953,6 @@ interface DataRowProps { row: TableRowType columns: DisplayColumn[] rowIndex: number - arrayIndex: number isFirstRow: boolean editingColumnName: string | null initialCharacter: string | null @@ -3008,7 +3013,6 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean { prev.row !== next.row || prev.columns !== next.columns || prev.rowIndex !== next.rowIndex || - prev.arrayIndex !== next.arrayIndex || prev.isFirstRow !== next.isFirstRow || prev.editingColumnName !== next.editingColumnName || prev.pendingCellValue !== next.pendingCellValue || @@ -3048,7 +3052,6 @@ const DataRow = React.memo(function DataRow({ row, columns, rowIndex, - arrayIndex, isFirstRow, editingColumnName, initialCharacter, @@ -3096,7 +3099,7 @@ const DataRow = React.memo(function DataRow({ isRowSelected ? 'hidden' : 'block group-hover/checkbox:hidden' )} > - {arrayIndex + 1} + {rowIndex + 1}
{ diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/utils.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/utils.ts index 9223391c7b..6721e6e60f 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/utils.ts @@ -177,16 +177,10 @@ export function computeNormalizedSelection( } } -export function collectRowSnapshots( - positions: Iterable, - positionMap: Map -): DeletedRowSnapshot[] { +export function collectRowSnapshots(rows: Iterable): DeletedRowSnapshot[] { const snapshots: DeletedRowSnapshot[] = [] - for (const pos of positions) { - const row = positionMap.get(pos) - if (row) { - snapshots.push({ rowId: row.id, data: { ...row.data }, position: row.position }) - } + for (const row of rows) { + snapshots.push({ rowId: row.id, data: { ...row.data }, position: row.position }) } return snapshots } From 12876231cd6930e994c6691414ca9c0adb860ec5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 5 May 2026 10:35:28 -0700 Subject: [PATCH 3/4] refactor(tables): rowId-stable selection across sort, position math via reduce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Track anchor/focus by rowId so same-length sort changes remap selection to the new visual index instead of leaving it on a different row. - Replace last-row position lookups with Math.max reduce in paste's create-batch and append-row's undo snapshot — under non-position sorts, the last visual row's position is not the largest. - Trim a navigation-noise comment and tighten two over-explanatory ones. Co-Authored-By: Claude Opus 4.7 --- .../[tableId]/components/table/table.tsx | 155 ++++++++++-------- 1 file changed, 88 insertions(+), 67 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx index 7628431295..aa51fa3134 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx @@ -239,8 +239,8 @@ export function Table({ setColumnOrder(order) } - // Width keys are either the logical name or `${name}::${path}` (fanned-out - // workflow columns). Rename rewrites every key whose prefix matches. + // Width keys are either the logical name or `${name}::${path}` for fanned-out + // workflow columns; rename must rewrite every key whose prefix matches. function handleColumnRename(oldName: string, newName: string) { let updatedWidths = columnWidthsRef.current let widthsChanged = false @@ -291,9 +291,6 @@ export function Table({ const pushUndoRef = useRef(pushUndo) pushUndoRef.current = pushUndo - // `columns`, `tableWorkflowGroups`, `workflowStates`, `columnSourceInfo`, - // and `workflowNameById` come from `useTable` above. - const displayColumns = useMemo(() => { let ordered: ColumnDefinition[] if (!columnOrder || columnOrder.length === 0) { @@ -410,6 +407,8 @@ export function Table({ const rowsRef = useRef(rows) const selectionAnchorRef = useRef(selectionAnchor) const selectionFocusRef = useRef(selectionFocus) + const anchorRowIdRef = useRef(null) + const focusRowIdRef = useRef(null) const checkedRowsRef = useRef(checkedRows) checkedRowsRef.current = checkedRows @@ -495,7 +494,8 @@ export function Table({ }, [contextMenu.row, contextMenu.columnName, closeContextMenu]) const handleContextMenuDelete = useCallback(() => { - if (!contextMenu.row) { + const contextRow = contextMenu.row + if (!contextRow) { closeContextMenu() return } @@ -504,12 +504,11 @@ export function Table({ const currentRows = rowsRef.current let snapshots: DeletedRowSnapshot[] = [] - const contextRowArrayIndex = currentRows.findIndex((r) => r.id === contextMenu.row!.id) - - if (checked.size > 0 && checked.has(contextMenu.row.id)) { + if (checked.size > 0 && checked.has(contextRow.id)) { snapshots = collectRowSnapshots(currentRows.filter((r) => checked.has(r.id))) } else { const sel = computeNormalizedSelection(selectionAnchorRef.current, selectionFocusRef.current) + const contextRowArrayIndex = currentRows.findIndex((r) => r.id === contextRow.id) const isInSelection = sel !== null && contextRowArrayIndex >= sel.startRow && contextRowArrayIndex <= sel.endRow @@ -517,11 +516,7 @@ export function Table({ snapshots = collectRowSnapshots(currentRows.slice(sel.startRow, sel.endRow + 1)) } else { snapshots = [ - { - rowId: contextMenu.row.id, - data: { ...contextMenu.row.data }, - position: contextMenu.row.position, - }, + { rowId: contextRow.id, data: { ...contextRow.data }, position: contextRow.position }, ] } } @@ -582,10 +577,11 @@ export function Table({ }, [contextMenuExecutionId, closeContextMenu]) const handleDuplicateRow = useCallback(() => { - if (!contextMenu.row) return - const rowData = { ...contextMenu.row.data } - const position = contextMenu.row.position + 1 - const sourceArrayIndex = rowsRef.current.findIndex((r) => r.id === contextMenu.row!.id) + const contextRow = contextMenu.row + if (!contextRow) return + const rowData = { ...contextRow.data } + const position = contextRow.position + 1 + const sourceArrayIndex = rowsRef.current.findIndex((r) => r.id === contextRow.id) closeContextMenu() createRef.current( { data: rowData, position }, @@ -631,11 +627,11 @@ export function Table({ onSuccess: (response: Record) => { const newRowId = extractCreatedRowId(response) if (newRowId) { - const lastRow = rowsRef.current[rowsRef.current.length - 1] + const maxPosition = rowsRef.current.reduce((max, r) => Math.max(max, r.position), -1) pushUndoRef.current({ type: 'create-row', rowId: newRowId, - position: (lastRow?.position ?? -1) + 1, + position: maxPosition + 1, }) } }, @@ -714,37 +710,29 @@ export function Table({ if (!targetRow) return const targetId = targetRow.id - if (shiftKey && lastCheckboxRowRef.current !== null) { - const lastIdx = currentRows.findIndex((r) => r.id === lastCheckboxRowRef.current) - if (lastIdx !== -1) { - const from = Math.min(lastIdx, rowIndex) - const to = Math.max(lastIdx, rowIndex) - setCheckedRows((prev) => { - const next = new Set(prev) - 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 { + const lastIdx = + shiftKey && lastCheckboxRowRef.current !== null + ? 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) - if (next.has(targetId)) { - next.delete(targetId) - } else { - next.add(targetId) + 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 + }) } lastCheckboxRowRef.current = targetId scrollRef.current?.focus({ preventScroll: true }) @@ -798,6 +786,7 @@ export function Table({ if (rws.length === 0 || currentCols.length === 0) return setEditingCell(null) setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS)) + lastCheckboxRowRef.current = null suppressFocusScrollRef.current = true setSelectionAnchor({ rowIndex: 0, colIndex: 0 }) setSelectionFocus({ @@ -1159,14 +1148,49 @@ export function Table({ }, []) useEffect(() => { - const len = rows.length - if (selectionAnchorRef.current && selectionAnchorRef.current.rowIndex >= len) { - setSelectionAnchor(null) + anchorRowIdRef.current = selectionAnchor + ? (rowsRef.current[selectionAnchor.rowIndex]?.id ?? null) + : null + }, [selectionAnchor]) + + useEffect(() => { + focusRowIdRef.current = selectionFocus + ? (rowsRef.current[selectionFocus.rowIndex]?.id ?? null) + : null + }, [selectionFocus]) + + useEffect(() => { + const anchor = selectionAnchorRef.current + if (anchor) { + const expectedId = anchorRowIdRef.current + const actualId = rows[anchor.rowIndex]?.id ?? null + if (expectedId && expectedId !== actualId) { + const newIndex = rows.findIndex((r) => r.id === expectedId) + if (newIndex >= 0) { + setSelectionAnchor({ rowIndex: newIndex, colIndex: anchor.colIndex }) + } else { + setSelectionAnchor(null) + } + } else if (anchor.rowIndex >= rows.length) { + setSelectionAnchor(null) + } } - if (selectionFocusRef.current && selectionFocusRef.current.rowIndex >= len) { - setSelectionFocus(null) + const focus = selectionFocusRef.current + if (focus) { + const expectedId = focusRowIdRef.current + const actualId = rows[focus.rowIndex]?.id ?? null + if (expectedId && expectedId !== actualId) { + const newIndex = rows.findIndex((r) => r.id === expectedId) + if (newIndex >= 0) { + setSelectionFocus({ rowIndex: newIndex, colIndex: focus.colIndex }) + } else { + setSelectionFocus(null) + } + } else if (focus.rowIndex >= rows.length) { + setSelectionFocus(null) + } } - }, [rows.length]) + }, [rows]) useEffect(() => { if (isColumnSelection) return @@ -1203,12 +1227,10 @@ export function Table({ setInitialCharacter(null) }, []) - // Double-click highlights the cell's text and, only if the text is actually - // truncated, opens the expanded popover. The cell has `select-none` which - // suppresses the highlight even for programmatic selections, so we override - // `user-select` on the inner element until the next click. Workflow cells nest - // their text inside a span with its own `overflow-clip`, so we measure the leaf - // element's scroll dimensions, not just the wrapper div's. + // The cell has `select-none` which suppresses programmatic selection, so we + // override `user-select` on the inner element until the next click. The popover + // only opens when the leaf's scroll dimensions exceed its client dimensions + // (workflow cells nest text inside a span with its own `overflow-clip`). const handleCellDoubleClick = useCallback( (rowId: string, columnName: string, columnKey: string) => { setSelectionFocus(null) @@ -1319,6 +1341,7 @@ export function Table({ suppressFocusScrollRef.current = true setEditingCell(null) setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS)) + lastCheckboxRowRef.current = null setSelectionAnchor({ rowIndex: 0, colIndex: 0 }) setSelectionFocus({ rowIndex: rws.length - 1, @@ -1408,7 +1431,7 @@ export function Table({ if (newRowId) { pushUndoRef.current({ type: 'create-row', rowId: newRowId, position }) } - setSelectionAnchor({ rowIndex: position, colIndex }) + setSelectionAnchor({ rowIndex: anchor.rowIndex + 1, colIndex }) setSelectionFocus(null) }, } @@ -1776,8 +1799,7 @@ export function Table({ const currentCols = columnsRef.current const currentRows = rowsRef.current - const lastRowPosition = - currentRows.length > 0 ? currentRows[currentRows.length - 1].position : -1 + const lastRowPosition = currentRows.reduce((max, r) => Math.max(max, r.position), -1) const undoCells: Array<{ rowId: string; data: Record }> = [] const updateBatch: Array<{ rowId: string; data: Record }> = [] @@ -2386,9 +2408,10 @@ export function Table({ ) const selectedRowCount = useMemo(() => { - if (!contextMenu.isOpen || !contextMenu.row) return 1 + const contextRow = contextMenu.isOpen ? contextMenu.row : null + if (!contextRow) return 1 - if (checkedRows.size > 0 && checkedRows.has(contextMenu.row.id)) { + if (checkedRows.size > 0 && checkedRows.has(contextRow.id)) { let count = 0 for (const row of rows) { if (checkedRows.has(row.id)) count++ @@ -2399,10 +2422,8 @@ export function Table({ const sel = normalizedSelection if (!sel) return 1 - const contextRowArrayIndex = rows.findIndex((r) => r.id === contextMenu.row!.id) - const isInSelection = contextRowArrayIndex >= sel.startRow && contextRowArrayIndex <= sel.endRow - - if (!isInSelection) return 1 + const contextRowArrayIndex = rows.findIndex((r) => r.id === contextRow.id) + if (contextRowArrayIndex < sel.startRow || contextRowArrayIndex > sel.endRow) return 1 const start = Math.max(0, sel.startRow) const end = Math.min(rows.length - 1, sel.endRow) From 633ddba241fd840f0148e8179743e7f4296fc5de Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 5 May 2026 10:47:10 -0700 Subject: [PATCH 4/4] fix(tables): guard rowId remap effect and document paste batch position - Skip the validation effect when rows is empty (transient state during initial load of a new sort/filter before keepPreviousData populates) so selection survives uncached query changes. - Skip when isColumnSelection is true; the column-selection pinning effect owns focus.rowIndex for those, and remapping would shrink a full-column range to wherever the captured endpoints happened to land after reorder. - Comment lastRowPosition's hoist invariant so a future refactor doesn't move it inside the loop and produce colliding positions. Co-Authored-By: Claude Opus 4.7 --- .../tables/[tableId]/components/table/table.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx index aa51fa3134..35b9c6a9fb 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx @@ -1160,6 +1160,14 @@ export function Table({ }, [selectionFocus]) useEffect(() => { + // Skip during transient empty-rows state (initial load of a new sort/filter + // before keepPreviousData kicks in) — clearing here would lose the user's + // selection across every uncached query change. + if (rows.length === 0) return + // Column selections pin focus to the last row via the effect above; remapping + // by row id would shrink a full-column range to whichever rows happened to be + // at the endpoints when the selection was captured. + if (isColumnSelectionRef.current) return const anchor = selectionAnchorRef.current if (anchor) { const expectedId = anchorRowIdRef.current @@ -1799,6 +1807,8 @@ export function Table({ const currentCols = columnsRef.current const currentRows = rowsRef.current + // Captured once before the loop so each new row in the batch gets a unique, + // sequential position via `+ (newRowIndex - currentRows.length)` below. const lastRowPosition = currentRows.reduce((max, r) => Math.max(max, r.position), -1) const undoCells: Array<{ rowId: string; data: Record }> = []