From f734c442f444f05b0cf3e5ce9794af5dad284c29 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 5 Aug 2026 21:15:06 -0700 Subject: [PATCH 1/3] fix(tables): stop remote cell selections painting over the row gutter --- .../components/table-grid/constants.ts | 2 + .../components/table-grid/data-row.tsx | 3 + .../table-grid/remote-selection-overlay.tsx | 160 +++++++++++------- .../components/table-grid/table-grid.tsx | 1 + 4 files changed, 109 insertions(+), 57 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts index 7ccc298fca9..f369e93d363 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts @@ -14,6 +14,8 @@ export const COLUMN_SIDEBAR_WIDTH = 400 export const CELL = 'border-[var(--border)] border-r border-b px-2 py-[7px] align-middle select-none' +/** `z-[6]` is load-bearing: the remote-selection overlay splits its layers around it so a + * peer's selection scrolled behind this cell is hidden by paint order. */ export const CELL_CHECKBOX = 'sticky left-0 z-[6] border-[var(--border)] border-r border-b bg-[var(--bg)] px-0 py-[7px] align-middle select-none' export const CELL_HEADER_CHECKBOX = diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx index b73bf68dc49..ccb2e727c19 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx @@ -315,6 +315,9 @@ export const DataRow = React.memo(function DataRow({ data-row={rowIndex} data-row-id={row.id} data-col={colIndex} + // Read by overlays measured off these cells (see remote-selection-overlay.tsx) + // to tell a cell frozen in the sticky zone from one scrolled behind it. + data-pinned={isPinnedCell ? '' : undefined} className={cn( CELL, (isHighlighted || isAnchor || isEditing) && 'relative', diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx index 5461082f3de..68a5e90b5b8 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx @@ -19,7 +19,11 @@ interface SelectionBox { left: number width: number height: number - /** Viewport-space top/left of the selection, for the body-portaled name label. */ + /** Whether every cell of the selection is pinned, i.e. it belongs to the frozen left zone + * and so renders above it rather than behind it. */ + pinned: boolean + /** Viewport-space top/left of the selection, for the body-portaled name label. `left` is + * clamped to the frozen zone so the label never floats over the gutter. */ viewportTop: number viewportLeft: number /** Resolved anchor/focus cell indices (undefined when off-window). Coverage by the local @@ -39,23 +43,25 @@ interface RemoteSelectionOverlayProps { rowIndexById: Map /** The local user's own normalized selection, so a co-selected remote cell defers to it. */ localSelection: NormalizedSelection | null + /** Width of the frozen left zone (row gutter + pinned columns). Paint order hides the boxes + * behind it; this is what the JS hover hit-test and the name label test against. */ + stickyLeftWidth: number /** The grid's scroll container (`data-table-scroll`), queried for cell rects. */ scrollElement: HTMLElement | null } -/** The cell `` for a (rowId, columnIndex), or undefined when virtualized off-window. */ -function cellRect( +/** The cell `` for a (rowId, columnIndex), or null when virtualized off-window. */ +function cellElement( scrollEl: HTMLElement, rowId: string, columnIndex: number | undefined -): DOMRect | undefined { - if (columnIndex === undefined) return undefined +): HTMLElement | null { + if (columnIndex === undefined) return null // `rowId` is a remote peer's value — escape it so a hostile id can't break the // selector and throw (`columnIndex` is a local numeric index, already safe). - const cell = scrollEl.querySelector( + return scrollEl.querySelector( `[data-row-id="${CSS.escape(rowId)}"][data-col="${columnIndex}"]` ) - return cell?.getBoundingClientRect() } /** @@ -76,6 +82,30 @@ function isSelectionCovered( ) } +/** + * One peer's selection rectangle. The border is an inset box-shadow (no layout width, so it + * never stacks with an adjacent cell's border) plus a subtle fill, darker while they edit. + */ +interface SelectionRectProps { + box: SelectionBox +} + +function SelectionRect({ box }: SelectionRectProps) { + return ( +
+ ) +} + /** * Renders remote collaborators' cell selections over the table grid — a colored * border per user (Google-Sheets style), a darker fill while they are editing, and @@ -93,6 +123,7 @@ export function RemoteSelectionOverlay({ columnIndexById, rowIndexById, localSelection, + stickyLeftWidth, scrollElement, }: RemoteSelectionOverlayProps) { const rootRef = useRef(null) @@ -112,16 +143,28 @@ export function RemoteSelectionOverlay({ // Read only by the pointer hit-test (never in render) to skip a locally-covered box. const localSelectionRef = useRef(localSelection) localSelectionRef.current = localSelection + // Read via ref so a column resize or a pin/unpin never re-subscribes the listeners. + const stickyLeftWidthRef = useRef(stickyLeftWidth) + stickyLeftWidthRef.current = stickyLeftWidth // Cached content-wrapper origin, refreshed on each measure (scroll/resize/data change), // so the pointer hit-test never forces a layout read per mouse move. const originRef = useRef({ top: 0, left: 0 }) + // Content-space x of the frozen zone's right edge. Paint order hides a box behind the zone + // (see the layers in render), but the hover hit-test is plain JS and has to exclude it by + // hand — so this is refreshed on every scroll event, not just on the rAF-throttled measure, + // and can never trail the pointer. + const frozenEdgeXRef = useRef(0) const measure = useCallback(() => { const scrollEl = scrollElement const root = rootRef.current if (!scrollEl || !root) return + frozenEdgeXRef.current = scrollEl.scrollLeft + stickyLeftWidthRef.current const origin = root.getBoundingClientRect() originRef.current = { top: origin.top, left: origin.left } + // The wrapper is the scroller's only child, so its origin already encodes the scroll + // offset — no second `getBoundingClientRect()` for the frozen zone's viewport x. + const stickyViewportX = origin.left + frozenEdgeXRef.current const next: SelectionBox[] = [] for (const selection of remoteSelectionsRef.current) { const { anchor, focus, editing } = selection.cell @@ -129,11 +172,15 @@ export function RemoteSelectionOverlay({ const focusCol = columnIndexByIdRef.current.get(focus.columnId) const anchorRow = rowIndexByIdRef.current.get(anchor.rowId) const focusRow = rowIndexByIdRef.current.get(focus.rowId) - const rects = [ - cellRect(scrollEl, anchor.rowId, anchorCol), - cellRect(scrollEl, focus.rowId, focusCol), - ].filter((rect): rect is DOMRect => rect !== undefined) - if (rects.length === 0) continue + const cells = [ + cellElement(scrollEl, anchor.rowId, anchorCol), + cellElement(scrollEl, focus.rowId, focusCol), + ].filter((cell): cell is HTMLElement => cell !== null) + if (cells.length === 0) continue + const rects = cells.map((cell) => cell.getBoundingClientRect()) + // A range straddling the boundary defers to the frozen zone, so its scrolled-away half + // can't bleed over the gutter — the conservative half of the trade, and the rarer case. + const pinned = cells.every((cell) => cell.hasAttribute('data-pinned')) const viewportTop = Math.min(...rects.map((r) => r.top)) const viewportLeft = Math.min(...rects.map((r) => r.left)) @@ -150,8 +197,9 @@ export function RemoteSelectionOverlay({ left, width: right - left, height: bottom - top, + pinned, viewportTop, - viewportLeft, + viewportLeft: pinned ? viewportLeft : Math.max(viewportLeft, stickyViewportX), anchorRow, anchorCol, focusRow, @@ -169,6 +217,9 @@ export function RemoteSelectionOverlay({ let raf = 0 const schedule = () => { + // Plain number, no DOM write: the hit-test needs the frozen edge on every event, but + // the boxes' own occlusion is paint-order and needs nothing from JS. + frozenEdgeXRef.current = scrollEl.scrollLeft + stickyLeftWidthRef.current if (!raf) raf = requestAnimationFrame(() => { raf = 0 @@ -181,6 +232,9 @@ export function RemoteSelectionOverlay({ const y = event.clientY - top const hit = boxesRef.current.find( (b) => + // Only the part of the box that clears the frozen zone is painted — hovering the + // row gutter it hides behind must not pop the peer's name tag. + (b.pinned || x >= frozenEdgeXRef.current) && x >= b.left && x <= b.left + b.width && y >= b.top && @@ -233,56 +287,48 @@ export function RemoteSelectionOverlay({ // subscribed). Layout effect so positions update before paint — no one-frame lag as a // peer moves. NOT keyed on `localSelection`: moving the local caret changes only which // boxes are `covered`, which the cheap in-memory pass below handles without a reflow. + // `stickyLeftWidth` is a dep too: pinning a column moves the frozen zone's edge without + // resizing the content, so nothing else would refresh the hit-test's boundary. useLayoutEffect(() => { measure() - }, [remoteSelections, columnIndexById, measure]) + }, [remoteSelections, columnIndexById, stickyLeftWidth, measure]) - // Re-derived in render so it reacts to `localSelection`: when the local selection grows to - // cover the hovered box (its outline is no longer drawn) without another pointer move, the - // floating name tag must drop rather than linger over cells with no visible remote selection. - const hoveredBox = hoveredSocketId - ? boxes.find( - (box) => - box.socketId === hoveredSocketId && - !isSelectionCovered( - box.anchorRow, - box.anchorCol, - box.focusRow, - box.focusCol, - localSelection - ) - ) - : undefined + // Partitioned in render so it reacts to `localSelection`: a cell the local user also has + // selected shows only the local selection — the remote box isn't drawn (its `boxes` entry + // still drives the hover name). Resolving the hovered box in the same pass means that when + // the local selection grows to cover it without another pointer move, the floating name tag + // drops rather than lingering over cells with no visible remote selection. + const scrollingBoxes: SelectionBox[] = [] + const frozenBoxes: SelectionBox[] = [] + let hoveredBox: SelectionBox | undefined + for (const box of boxes) { + if ( + isSelectionCovered(box.anchorRow, box.anchorCol, box.focusRow, box.focusCol, localSelection) + ) { + continue + } + ;(box.pinned ? frozenBoxes : scrollingBoxes).push(box) + if (box.socketId === hoveredSocketId) hoveredBox = box + } return ( <> -
- {boxes.map((box) => - // A cell the local user also has selected shows only the local selection — the - // remote box isn't drawn (its `boxes` entry still drives the hover name). The - // border is an inset box-shadow (no layout width, so it never stacks with an - // adjacent cell's border) plus a subtle fill, darker while the peer is editing. - isSelectionCovered( - box.anchorRow, - box.anchorCol, - box.focusRow, - box.focusCol, - localSelection - ) ? null : ( -
- ) - )} +
+ {/* Split by the frozen left zone (row gutter + pinned columns, both opaque at `z-[6]`). + Ordinary-column selections sit BELOW at `z-[5]`, so scrolling one behind the gutter + hides it by paint order with nothing to sync per frame; pinned ones must sit above + at `z-[8]` or that cell's own opaque background swallows them. Both still clear + ordinary cells, which carry no background and no z-index. */} +
+ {scrollingBoxes.map((box) => ( + + ))} +
+
+ {frozenBoxes.map((box) => ( + + ))} +
{/* The name label portals to the body so it floats on top of the grid (and its sticky header) instead of being clipped by the overlay's overflow-hidden; it's diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index c37bf812c6e..07611167a1a 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -4528,6 +4528,7 @@ export function TableGrid({ columnIndexById={columnIndexById} rowIndexById={rowIndexById} localSelection={normalizedSelection} + stickyLeftWidth={pinnedStickyLeftEdge} scrollElement={scrollRef.current} /> )} From 97470bdedfa544beca77fb21ff915160cf9fc6c1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 5 Aug 2026 21:20:30 -0700 Subject: [PATCH 2/3] fix(tables): classify a remote selection as pinned only when both endpoints resolve --- .../components/table-grid/data-row.tsx | 7 +++++-- .../table-grid/remote-selection-overlay.tsx | 19 ++++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx index ccb2e727c19..acf192e3002 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx @@ -306,6 +306,11 @@ export const DataRow = React.memo(function DataRow({ const isRightEdge = inRange ? colIndex === sel!.endCol : colIndex === columns.length - 1 const pinnedLeft = pinnedOffsets?.get(column.key) + /** + * Whether this cell is frozen in the sticky left zone. Drives the sticky offset and, + * via the `data-pinned` attribute below, tells overlays measured off these cells + * (see `remote-selection-overlay.tsx`) a frozen cell from one scrolled behind the zone. + */ const isPinnedCell = pinnedLeft !== undefined const isPinnedSeparator = column.key === lastPinnedColKey @@ -315,8 +320,6 @@ export const DataRow = React.memo(function DataRow({ data-row={rowIndex} data-row-id={row.id} data-col={colIndex} - // Read by overlays measured off these cells (see remote-selection-overlay.tsx) - // to tell a cell frozen in the sticky zone from one scrolled behind it. data-pinned={isPinnedCell ? '' : undefined} className={cn( CELL, diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx index 68a5e90b5b8..425e30206d4 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx @@ -172,15 +172,20 @@ export function RemoteSelectionOverlay({ const focusCol = columnIndexByIdRef.current.get(focus.columnId) const anchorRow = rowIndexByIdRef.current.get(anchor.rowId) const focusRow = rowIndexByIdRef.current.get(focus.rowId) - const cells = [ - cellElement(scrollEl, anchor.rowId, anchorCol), - cellElement(scrollEl, focus.rowId, focusCol), - ].filter((cell): cell is HTMLElement => cell !== null) + const anchorCell = cellElement(scrollEl, anchor.rowId, anchorCol) + const focusCell = cellElement(scrollEl, focus.rowId, focusCol) + const cells = [anchorCell, focusCell].filter((cell): cell is HTMLElement => cell !== null) if (cells.length === 0) continue const rects = cells.map((cell) => cell.getBoundingClientRect()) - // A range straddling the boundary defers to the frozen zone, so its scrolled-away half - // can't bleed over the gutter — the conservative half of the trade, and the rarer case. - const pinned = cells.every((cell) => cell.hasAttribute('data-pinned')) + // Both endpoints must be resolved AND pinned. Anything else — a range straddling the + // boundary, or one whose other endpoint is virtualized off-window and so can't be + // classified — defers to the frozen zone, where the worst case is a selection the zone + // hides rather than one painting over the gutter. + const pinned = + anchorCell !== null && + focusCell !== null && + anchorCell.hasAttribute('data-pinned') && + focusCell.hasAttribute('data-pinned') const viewportTop = Math.min(...rects.map((r) => r.top)) const viewportLeft = Math.min(...rects.map((r) => r.left)) From 1bfc501c82c453ed72d87d86b1bd58bf94961019 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 5 Aug 2026 21:40:06 -0700 Subject: [PATCH 3/3] fix(tables): classify an off-window selection endpoint by its column --- .../table-grid/remote-selection-overlay.tsx | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx index 425e30206d4..25cff333674 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx @@ -50,6 +50,23 @@ interface RemoteSelectionOverlayProps { scrollElement: HTMLElement | null } +/** + * Whether a selection endpoint lands in the frozen left zone. Rows are virtualized, so an + * endpoint's own cell may not exist; fall back to any rendered row's cell in that column, + * since pinning is a per-column property. An endpoint whose column is gone entirely (hidden + * or deleted locally) can't be classified and is treated as unpinned — the safe direction, + * since the frozen zone then occludes it rather than being painted over. + */ +function endpointIsPinned( + scrollEl: HTMLElement, + cell: HTMLElement | null, + columnIndex: number | undefined +): boolean { + if (cell !== null) return cell.hasAttribute('data-pinned') + if (columnIndex === undefined) return false + return scrollEl.querySelector(`[data-col="${columnIndex}"][data-pinned]`) !== null +} + /** The cell `` for a (rowId, columnIndex), or null when virtualized off-window. */ function cellElement( scrollEl: HTMLElement, @@ -177,15 +194,11 @@ export function RemoteSelectionOverlay({ const cells = [anchorCell, focusCell].filter((cell): cell is HTMLElement => cell !== null) if (cells.length === 0) continue const rects = cells.map((cell) => cell.getBoundingClientRect()) - // Both endpoints must be resolved AND pinned. Anything else — a range straddling the - // boundary, or one whose other endpoint is virtualized off-window and so can't be - // classified — defers to the frozen zone, where the worst case is a selection the zone - // hides rather than one painting over the gutter. + // Only a selection pinned at BOTH ends renders above the frozen zone. One that straddles + // the boundary goes below it, so its unpinned half can't paint over the gutter. const pinned = - anchorCell !== null && - focusCell !== null && - anchorCell.hasAttribute('data-pinned') && - focusCell.hasAttribute('data-pinned') + endpointIsPinned(scrollEl, anchorCell, anchorCol) && + endpointIsPinned(scrollEl, focusCell, focusCol) const viewportTop = Math.min(...rects.map((r) => r.top)) const viewportLeft = Math.min(...rects.map((r) => r.left))