Skip to content

Commit 1287623

Browse files
waleedlatif1claude
andcommitted
refactor(tables): rowId-stable selection across sort, position math via reduce
- 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 <noreply@anthropic.com>
1 parent 11fc89d commit 1287623

1 file changed

Lines changed: 88 additions & 67 deletions

File tree

  • apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx

Lines changed: 88 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,8 @@ export function Table({
239239
setColumnOrder(order)
240240
}
241241

242-
// Width keys are either the logical name or `${name}::${path}` (fanned-out
243-
// workflow columns). Rename rewrites every key whose prefix matches.
242+
// Width keys are either the logical name or `${name}::${path}` for fanned-out
243+
// workflow columns; rename must rewrite every key whose prefix matches.
244244
function handleColumnRename(oldName: string, newName: string) {
245245
let updatedWidths = columnWidthsRef.current
246246
let widthsChanged = false
@@ -291,9 +291,6 @@ export function Table({
291291
const pushUndoRef = useRef(pushUndo)
292292
pushUndoRef.current = pushUndo
293293

294-
// `columns`, `tableWorkflowGroups`, `workflowStates`, `columnSourceInfo`,
295-
// and `workflowNameById` come from `useTable` above.
296-
297294
const displayColumns = useMemo<DisplayColumn[]>(() => {
298295
let ordered: ColumnDefinition[]
299296
if (!columnOrder || columnOrder.length === 0) {
@@ -410,6 +407,8 @@ export function Table({
410407
const rowsRef = useRef(rows)
411408
const selectionAnchorRef = useRef(selectionAnchor)
412409
const selectionFocusRef = useRef(selectionFocus)
410+
const anchorRowIdRef = useRef<string | null>(null)
411+
const focusRowIdRef = useRef<string | null>(null)
413412

414413
const checkedRowsRef = useRef(checkedRows)
415414
checkedRowsRef.current = checkedRows
@@ -495,7 +494,8 @@ export function Table({
495494
}, [contextMenu.row, contextMenu.columnName, closeContextMenu])
496495

497496
const handleContextMenuDelete = useCallback(() => {
498-
if (!contextMenu.row) {
497+
const contextRow = contextMenu.row
498+
if (!contextRow) {
499499
closeContextMenu()
500500
return
501501
}
@@ -504,24 +504,19 @@ export function Table({
504504
const currentRows = rowsRef.current
505505
let snapshots: DeletedRowSnapshot[] = []
506506

507-
const contextRowArrayIndex = currentRows.findIndex((r) => r.id === contextMenu.row!.id)
508-
509-
if (checked.size > 0 && checked.has(contextMenu.row.id)) {
507+
if (checked.size > 0 && checked.has(contextRow.id)) {
510508
snapshots = collectRowSnapshots(currentRows.filter((r) => checked.has(r.id)))
511509
} else {
512510
const sel = computeNormalizedSelection(selectionAnchorRef.current, selectionFocusRef.current)
511+
const contextRowArrayIndex = currentRows.findIndex((r) => r.id === contextRow.id)
513512
const isInSelection =
514513
sel !== null && contextRowArrayIndex >= sel.startRow && contextRowArrayIndex <= sel.endRow
515514

516515
if (isInSelection && sel) {
517516
snapshots = collectRowSnapshots(currentRows.slice(sel.startRow, sel.endRow + 1))
518517
} else {
519518
snapshots = [
520-
{
521-
rowId: contextMenu.row.id,
522-
data: { ...contextMenu.row.data },
523-
position: contextMenu.row.position,
524-
},
519+
{ rowId: contextRow.id, data: { ...contextRow.data }, position: contextRow.position },
525520
]
526521
}
527522
}
@@ -582,10 +577,11 @@ export function Table({
582577
}, [contextMenuExecutionId, closeContextMenu])
583578

584579
const handleDuplicateRow = useCallback(() => {
585-
if (!contextMenu.row) return
586-
const rowData = { ...contextMenu.row.data }
587-
const position = contextMenu.row.position + 1
588-
const sourceArrayIndex = rowsRef.current.findIndex((r) => r.id === contextMenu.row!.id)
580+
const contextRow = contextMenu.row
581+
if (!contextRow) return
582+
const rowData = { ...contextRow.data }
583+
const position = contextRow.position + 1
584+
const sourceArrayIndex = rowsRef.current.findIndex((r) => r.id === contextRow.id)
589585
closeContextMenu()
590586
createRef.current(
591587
{ data: rowData, position },
@@ -631,11 +627,11 @@ export function Table({
631627
onSuccess: (response: Record<string, unknown>) => {
632628
const newRowId = extractCreatedRowId(response)
633629
if (newRowId) {
634-
const lastRow = rowsRef.current[rowsRef.current.length - 1]
630+
const maxPosition = rowsRef.current.reduce((max, r) => Math.max(max, r.position), -1)
635631
pushUndoRef.current({
636632
type: 'create-row',
637633
rowId: newRowId,
638-
position: (lastRow?.position ?? -1) + 1,
634+
position: maxPosition + 1,
639635
})
640636
}
641637
},
@@ -714,37 +710,29 @@ export function Table({
714710
if (!targetRow) return
715711
const targetId = targetRow.id
716712

717-
if (shiftKey && lastCheckboxRowRef.current !== null) {
718-
const lastIdx = currentRows.findIndex((r) => r.id === lastCheckboxRowRef.current)
719-
if (lastIdx !== -1) {
720-
const from = Math.min(lastIdx, rowIndex)
721-
const to = Math.max(lastIdx, rowIndex)
722-
setCheckedRows((prev) => {
723-
const next = new Set(prev)
724-
for (let i = from; i <= to; i++) {
725-
const r = currentRows[i]
726-
if (r) next.add(r.id)
727-
}
728-
return next
729-
})
730-
} else {
731-
setCheckedRows((prev) => {
732-
const next = new Set(prev)
733-
if (next.has(targetId)) next.delete(targetId)
734-
else next.add(targetId)
735-
return next
736-
})
737-
}
738-
} else {
713+
const lastIdx =
714+
shiftKey && lastCheckboxRowRef.current !== null
715+
? currentRows.findIndex((r) => r.id === lastCheckboxRowRef.current)
716+
: -1
717+
718+
if (lastIdx !== -1) {
719+
const from = Math.min(lastIdx, rowIndex)
720+
const to = Math.max(lastIdx, rowIndex)
739721
setCheckedRows((prev) => {
740722
const next = new Set(prev)
741-
if (next.has(targetId)) {
742-
next.delete(targetId)
743-
} else {
744-
next.add(targetId)
723+
for (let i = from; i <= to; i++) {
724+
const r = currentRows[i]
725+
if (r) next.add(r.id)
745726
}
746727
return next
747728
})
729+
} else {
730+
setCheckedRows((prev) => {
731+
const next = new Set(prev)
732+
if (next.has(targetId)) next.delete(targetId)
733+
else next.add(targetId)
734+
return next
735+
})
748736
}
749737
lastCheckboxRowRef.current = targetId
750738
scrollRef.current?.focus({ preventScroll: true })
@@ -798,6 +786,7 @@ export function Table({
798786
if (rws.length === 0 || currentCols.length === 0) return
799787
setEditingCell(null)
800788
setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
789+
lastCheckboxRowRef.current = null
801790
suppressFocusScrollRef.current = true
802791
setSelectionAnchor({ rowIndex: 0, colIndex: 0 })
803792
setSelectionFocus({
@@ -1159,14 +1148,49 @@ export function Table({
11591148
}, [])
11601149

11611150
useEffect(() => {
1162-
const len = rows.length
1163-
if (selectionAnchorRef.current && selectionAnchorRef.current.rowIndex >= len) {
1164-
setSelectionAnchor(null)
1151+
anchorRowIdRef.current = selectionAnchor
1152+
? (rowsRef.current[selectionAnchor.rowIndex]?.id ?? null)
1153+
: null
1154+
}, [selectionAnchor])
1155+
1156+
useEffect(() => {
1157+
focusRowIdRef.current = selectionFocus
1158+
? (rowsRef.current[selectionFocus.rowIndex]?.id ?? null)
1159+
: null
1160+
}, [selectionFocus])
1161+
1162+
useEffect(() => {
1163+
const anchor = selectionAnchorRef.current
1164+
if (anchor) {
1165+
const expectedId = anchorRowIdRef.current
1166+
const actualId = rows[anchor.rowIndex]?.id ?? null
1167+
if (expectedId && expectedId !== actualId) {
1168+
const newIndex = rows.findIndex((r) => r.id === expectedId)
1169+
if (newIndex >= 0) {
1170+
setSelectionAnchor({ rowIndex: newIndex, colIndex: anchor.colIndex })
1171+
} else {
1172+
setSelectionAnchor(null)
1173+
}
1174+
} else if (anchor.rowIndex >= rows.length) {
1175+
setSelectionAnchor(null)
1176+
}
11651177
}
1166-
if (selectionFocusRef.current && selectionFocusRef.current.rowIndex >= len) {
1167-
setSelectionFocus(null)
1178+
const focus = selectionFocusRef.current
1179+
if (focus) {
1180+
const expectedId = focusRowIdRef.current
1181+
const actualId = rows[focus.rowIndex]?.id ?? null
1182+
if (expectedId && expectedId !== actualId) {
1183+
const newIndex = rows.findIndex((r) => r.id === expectedId)
1184+
if (newIndex >= 0) {
1185+
setSelectionFocus({ rowIndex: newIndex, colIndex: focus.colIndex })
1186+
} else {
1187+
setSelectionFocus(null)
1188+
}
1189+
} else if (focus.rowIndex >= rows.length) {
1190+
setSelectionFocus(null)
1191+
}
11681192
}
1169-
}, [rows.length])
1193+
}, [rows])
11701194

11711195
useEffect(() => {
11721196
if (isColumnSelection) return
@@ -1203,12 +1227,10 @@ export function Table({
12031227
setInitialCharacter(null)
12041228
}, [])
12051229

1206-
// Double-click highlights the cell's text and, only if the text is actually
1207-
// truncated, opens the expanded popover. The cell has `select-none` which
1208-
// suppresses the highlight even for programmatic selections, so we override
1209-
// `user-select` on the inner element until the next click. Workflow cells nest
1210-
// their text inside a span with its own `overflow-clip`, so we measure the leaf
1211-
// element's scroll dimensions, not just the wrapper div's.
1230+
// The cell has `select-none` which suppresses programmatic selection, so we
1231+
// override `user-select` on the inner element until the next click. The popover
1232+
// only opens when the leaf's scroll dimensions exceed its client dimensions
1233+
// (workflow cells nest text inside a span with its own `overflow-clip`).
12121234
const handleCellDoubleClick = useCallback(
12131235
(rowId: string, columnName: string, columnKey: string) => {
12141236
setSelectionFocus(null)
@@ -1319,6 +1341,7 @@ export function Table({
13191341
suppressFocusScrollRef.current = true
13201342
setEditingCell(null)
13211343
setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
1344+
lastCheckboxRowRef.current = null
13221345
setSelectionAnchor({ rowIndex: 0, colIndex: 0 })
13231346
setSelectionFocus({
13241347
rowIndex: rws.length - 1,
@@ -1408,7 +1431,7 @@ export function Table({
14081431
if (newRowId) {
14091432
pushUndoRef.current({ type: 'create-row', rowId: newRowId, position })
14101433
}
1411-
setSelectionAnchor({ rowIndex: position, colIndex })
1434+
setSelectionAnchor({ rowIndex: anchor.rowIndex + 1, colIndex })
14121435
setSelectionFocus(null)
14131436
},
14141437
}
@@ -1776,8 +1799,7 @@ export function Table({
17761799

17771800
const currentCols = columnsRef.current
17781801
const currentRows = rowsRef.current
1779-
const lastRowPosition =
1780-
currentRows.length > 0 ? currentRows[currentRows.length - 1].position : -1
1802+
const lastRowPosition = currentRows.reduce((max, r) => Math.max(max, r.position), -1)
17811803

17821804
const undoCells: Array<{ rowId: string; data: Record<string, unknown> }> = []
17831805
const updateBatch: Array<{ rowId: string; data: Record<string, unknown> }> = []
@@ -2386,9 +2408,10 @@ export function Table({
23862408
)
23872409

23882410
const selectedRowCount = useMemo(() => {
2389-
if (!contextMenu.isOpen || !contextMenu.row) return 1
2411+
const contextRow = contextMenu.isOpen ? contextMenu.row : null
2412+
if (!contextRow) return 1
23902413

2391-
if (checkedRows.size > 0 && checkedRows.has(contextMenu.row.id)) {
2414+
if (checkedRows.size > 0 && checkedRows.has(contextRow.id)) {
23922415
let count = 0
23932416
for (const row of rows) {
23942417
if (checkedRows.has(row.id)) count++
@@ -2399,10 +2422,8 @@ export function Table({
23992422
const sel = normalizedSelection
24002423
if (!sel) return 1
24012424

2402-
const contextRowArrayIndex = rows.findIndex((r) => r.id === contextMenu.row!.id)
2403-
const isInSelection = contextRowArrayIndex >= sel.startRow && contextRowArrayIndex <= sel.endRow
2404-
2405-
if (!isInSelection) return 1
2425+
const contextRowArrayIndex = rows.findIndex((r) => r.id === contextRow.id)
2426+
if (contextRowArrayIndex < sel.startRow || contextRowArrayIndex > sel.endRow) return 1
24062427

24072428
const start = Math.max(0, sel.startRow)
24082429
const end = Math.min(rows.length - 1, sel.endRow)

0 commit comments

Comments
 (0)