Skip to content

Commit 3510b0c

Browse files
authored
fix(tables): stop remote cell selections painting over the row gutter (#6310)
* fix(tables): stop remote cell selections painting over the row gutter * fix(tables): classify a remote selection as pinned only when both endpoints resolve * fix(tables): classify an off-window selection endpoint by its column
1 parent 0ebbcc7 commit 3510b0c

4 files changed

Lines changed: 130 additions & 57 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ export const COLUMN_SIDEBAR_WIDTH = 400
1414

1515
export const CELL =
1616
'border-[var(--border)] border-r border-b px-2 py-[7px] align-middle select-none'
17+
/** `z-[6]` is load-bearing: the remote-selection overlay splits its layers around it so a
18+
* peer's selection scrolled behind this cell is hidden by paint order. */
1719
export const CELL_CHECKBOX =
1820
'sticky left-0 z-[6] border-[var(--border)] border-r border-b bg-[var(--bg)] px-0 py-[7px] align-middle select-none'
1921
export const CELL_HEADER_CHECKBOX =

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,11 @@ export const DataRow = React.memo(function DataRow({
306306
const isRightEdge = inRange ? colIndex === sel!.endCol : colIndex === columns.length - 1
307307

308308
const pinnedLeft = pinnedOffsets?.get(column.key)
309+
/**
310+
* Whether this cell is frozen in the sticky left zone. Drives the sticky offset and,
311+
* via the `data-pinned` attribute below, tells overlays measured off these cells
312+
* (see `remote-selection-overlay.tsx`) a frozen cell from one scrolled behind the zone.
313+
*/
309314
const isPinnedCell = pinnedLeft !== undefined
310315
const isPinnedSeparator = column.key === lastPinnedColKey
311316

@@ -315,6 +320,7 @@ export const DataRow = React.memo(function DataRow({
315320
data-row={rowIndex}
316321
data-row-id={row.id}
317322
data-col={colIndex}
323+
data-pinned={isPinnedCell ? '' : undefined}
318324
className={cn(
319325
CELL,
320326
(isHighlighted || isAnchor || isEditing) && 'relative',

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx

Lines changed: 121 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ interface SelectionBox {
1919
left: number
2020
width: number
2121
height: number
22-
/** Viewport-space top/left of the selection, for the body-portaled name label. */
22+
/** Whether every cell of the selection is pinned, i.e. it belongs to the frozen left zone
23+
* and so renders above it rather than behind it. */
24+
pinned: boolean
25+
/** Viewport-space top/left of the selection, for the body-portaled name label. `left` is
26+
* clamped to the frozen zone so the label never floats over the gutter. */
2327
viewportTop: number
2428
viewportLeft: number
2529
/** Resolved anchor/focus cell indices (undefined when off-window). Coverage by the local
@@ -39,23 +43,42 @@ interface RemoteSelectionOverlayProps {
3943
rowIndexById: Map<string, number>
4044
/** The local user's own normalized selection, so a co-selected remote cell defers to it. */
4145
localSelection: NormalizedSelection | null
46+
/** Width of the frozen left zone (row gutter + pinned columns). Paint order hides the boxes
47+
* behind it; this is what the JS hover hit-test and the name label test against. */
48+
stickyLeftWidth: number
4249
/** The grid's scroll container (`data-table-scroll`), queried for cell rects. */
4350
scrollElement: HTMLElement | null
4451
}
4552

46-
/** The cell `<td>` for a (rowId, columnIndex), or undefined when virtualized off-window. */
47-
function cellRect(
53+
/**
54+
* Whether a selection endpoint lands in the frozen left zone. Rows are virtualized, so an
55+
* endpoint's own cell may not exist; fall back to any rendered row's cell in that column,
56+
* since pinning is a per-column property. An endpoint whose column is gone entirely (hidden
57+
* or deleted locally) can't be classified and is treated as unpinned — the safe direction,
58+
* since the frozen zone then occludes it rather than being painted over.
59+
*/
60+
function endpointIsPinned(
61+
scrollEl: HTMLElement,
62+
cell: HTMLElement | null,
63+
columnIndex: number | undefined
64+
): boolean {
65+
if (cell !== null) return cell.hasAttribute('data-pinned')
66+
if (columnIndex === undefined) return false
67+
return scrollEl.querySelector(`[data-col="${columnIndex}"][data-pinned]`) !== null
68+
}
69+
70+
/** The cell `<td>` for a (rowId, columnIndex), or null when virtualized off-window. */
71+
function cellElement(
4872
scrollEl: HTMLElement,
4973
rowId: string,
5074
columnIndex: number | undefined
51-
): DOMRect | undefined {
52-
if (columnIndex === undefined) return undefined
75+
): HTMLElement | null {
76+
if (columnIndex === undefined) return null
5377
// `rowId` is a remote peer's value — escape it so a hostile id can't break the
5478
// selector and throw (`columnIndex` is a local numeric index, already safe).
55-
const cell = scrollEl.querySelector(
79+
return scrollEl.querySelector<HTMLElement>(
5680
`[data-row-id="${CSS.escape(rowId)}"][data-col="${columnIndex}"]`
5781
)
58-
return cell?.getBoundingClientRect()
5982
}
6083

6184
/**
@@ -76,6 +99,30 @@ function isSelectionCovered(
7699
)
77100
}
78101

102+
/**
103+
* One peer's selection rectangle. The border is an inset box-shadow (no layout width, so it
104+
* never stacks with an adjacent cell's border) plus a subtle fill, darker while they edit.
105+
*/
106+
interface SelectionRectProps {
107+
box: SelectionBox
108+
}
109+
110+
function SelectionRect({ box }: SelectionRectProps) {
111+
return (
112+
<div
113+
className='absolute rounded-xs'
114+
style={{
115+
top: box.top,
116+
left: box.left,
117+
width: box.width,
118+
height: box.height,
119+
boxShadow: `inset 0 0 0 2px ${box.color}`,
120+
backgroundColor: withAlpha(box.color, box.editing ? 0.22 : 0.08),
121+
}}
122+
/>
123+
)
124+
}
125+
79126
/**
80127
* Renders remote collaborators' cell selections over the table grid — a colored
81128
* border per user (Google-Sheets style), a darker fill while they are editing, and
@@ -93,6 +140,7 @@ export function RemoteSelectionOverlay({
93140
columnIndexById,
94141
rowIndexById,
95142
localSelection,
143+
stickyLeftWidth,
96144
scrollElement,
97145
}: RemoteSelectionOverlayProps) {
98146
const rootRef = useRef<HTMLDivElement>(null)
@@ -112,28 +160,45 @@ export function RemoteSelectionOverlay({
112160
// Read only by the pointer hit-test (never in render) to skip a locally-covered box.
113161
const localSelectionRef = useRef(localSelection)
114162
localSelectionRef.current = localSelection
163+
// Read via ref so a column resize or a pin/unpin never re-subscribes the listeners.
164+
const stickyLeftWidthRef = useRef(stickyLeftWidth)
165+
stickyLeftWidthRef.current = stickyLeftWidth
115166
// Cached content-wrapper origin, refreshed on each measure (scroll/resize/data change),
116167
// so the pointer hit-test never forces a layout read per mouse move.
117168
const originRef = useRef({ top: 0, left: 0 })
169+
// Content-space x of the frozen zone's right edge. Paint order hides a box behind the zone
170+
// (see the layers in render), but the hover hit-test is plain JS and has to exclude it by
171+
// hand — so this is refreshed on every scroll event, not just on the rAF-throttled measure,
172+
// and can never trail the pointer.
173+
const frozenEdgeXRef = useRef(0)
118174

119175
const measure = useCallback(() => {
120176
const scrollEl = scrollElement
121177
const root = rootRef.current
122178
if (!scrollEl || !root) return
179+
frozenEdgeXRef.current = scrollEl.scrollLeft + stickyLeftWidthRef.current
123180
const origin = root.getBoundingClientRect()
124181
originRef.current = { top: origin.top, left: origin.left }
182+
// The wrapper is the scroller's only child, so its origin already encodes the scroll
183+
// offset — no second `getBoundingClientRect()` for the frozen zone's viewport x.
184+
const stickyViewportX = origin.left + frozenEdgeXRef.current
125185
const next: SelectionBox[] = []
126186
for (const selection of remoteSelectionsRef.current) {
127187
const { anchor, focus, editing } = selection.cell
128188
const anchorCol = columnIndexByIdRef.current.get(anchor.columnId)
129189
const focusCol = columnIndexByIdRef.current.get(focus.columnId)
130190
const anchorRow = rowIndexByIdRef.current.get(anchor.rowId)
131191
const focusRow = rowIndexByIdRef.current.get(focus.rowId)
132-
const rects = [
133-
cellRect(scrollEl, anchor.rowId, anchorCol),
134-
cellRect(scrollEl, focus.rowId, focusCol),
135-
].filter((rect): rect is DOMRect => rect !== undefined)
136-
if (rects.length === 0) continue
192+
const anchorCell = cellElement(scrollEl, anchor.rowId, anchorCol)
193+
const focusCell = cellElement(scrollEl, focus.rowId, focusCol)
194+
const cells = [anchorCell, focusCell].filter((cell): cell is HTMLElement => cell !== null)
195+
if (cells.length === 0) continue
196+
const rects = cells.map((cell) => cell.getBoundingClientRect())
197+
// Only a selection pinned at BOTH ends renders above the frozen zone. One that straddles
198+
// the boundary goes below it, so its unpinned half can't paint over the gutter.
199+
const pinned =
200+
endpointIsPinned(scrollEl, anchorCell, anchorCol) &&
201+
endpointIsPinned(scrollEl, focusCell, focusCol)
137202

138203
const viewportTop = Math.min(...rects.map((r) => r.top))
139204
const viewportLeft = Math.min(...rects.map((r) => r.left))
@@ -150,8 +215,9 @@ export function RemoteSelectionOverlay({
150215
left,
151216
width: right - left,
152217
height: bottom - top,
218+
pinned,
153219
viewportTop,
154-
viewportLeft,
220+
viewportLeft: pinned ? viewportLeft : Math.max(viewportLeft, stickyViewportX),
155221
anchorRow,
156222
anchorCol,
157223
focusRow,
@@ -169,6 +235,9 @@ export function RemoteSelectionOverlay({
169235

170236
let raf = 0
171237
const schedule = () => {
238+
// Plain number, no DOM write: the hit-test needs the frozen edge on every event, but
239+
// the boxes' own occlusion is paint-order and needs nothing from JS.
240+
frozenEdgeXRef.current = scrollEl.scrollLeft + stickyLeftWidthRef.current
172241
if (!raf)
173242
raf = requestAnimationFrame(() => {
174243
raf = 0
@@ -181,6 +250,9 @@ export function RemoteSelectionOverlay({
181250
const y = event.clientY - top
182251
const hit = boxesRef.current.find(
183252
(b) =>
253+
// Only the part of the box that clears the frozen zone is painted — hovering the
254+
// row gutter it hides behind must not pop the peer's name tag.
255+
(b.pinned || x >= frozenEdgeXRef.current) &&
184256
x >= b.left &&
185257
x <= b.left + b.width &&
186258
y >= b.top &&
@@ -233,56 +305,48 @@ export function RemoteSelectionOverlay({
233305
// subscribed). Layout effect so positions update before paint — no one-frame lag as a
234306
// peer moves. NOT keyed on `localSelection`: moving the local caret changes only which
235307
// boxes are `covered`, which the cheap in-memory pass below handles without a reflow.
308+
// `stickyLeftWidth` is a dep too: pinning a column moves the frozen zone's edge without
309+
// resizing the content, so nothing else would refresh the hit-test's boundary.
236310
useLayoutEffect(() => {
237311
measure()
238-
}, [remoteSelections, columnIndexById, measure])
312+
}, [remoteSelections, columnIndexById, stickyLeftWidth, measure])
239313

240-
// Re-derived in render so it reacts to `localSelection`: when the local selection grows to
241-
// cover the hovered box (its outline is no longer drawn) without another pointer move, the
242-
// floating name tag must drop rather than linger over cells with no visible remote selection.
243-
const hoveredBox = hoveredSocketId
244-
? boxes.find(
245-
(box) =>
246-
box.socketId === hoveredSocketId &&
247-
!isSelectionCovered(
248-
box.anchorRow,
249-
box.anchorCol,
250-
box.focusRow,
251-
box.focusCol,
252-
localSelection
253-
)
254-
)
255-
: undefined
314+
// Partitioned in render so it reacts to `localSelection`: a cell the local user also has
315+
// selected shows only the local selection — the remote box isn't drawn (its `boxes` entry
316+
// still drives the hover name). Resolving the hovered box in the same pass means that when
317+
// the local selection grows to cover it without another pointer move, the floating name tag
318+
// drops rather than lingering over cells with no visible remote selection.
319+
const scrollingBoxes: SelectionBox[] = []
320+
const frozenBoxes: SelectionBox[] = []
321+
let hoveredBox: SelectionBox | undefined
322+
for (const box of boxes) {
323+
if (
324+
isSelectionCovered(box.anchorRow, box.anchorCol, box.focusRow, box.focusCol, localSelection)
325+
) {
326+
continue
327+
}
328+
;(box.pinned ? frozenBoxes : scrollingBoxes).push(box)
329+
if (box.socketId === hoveredSocketId) hoveredBox = box
330+
}
256331

257332
return (
258333
<>
259-
<div ref={rootRef} className='pointer-events-none absolute inset-0 z-[8] overflow-hidden'>
260-
{boxes.map((box) =>
261-
// A cell the local user also has selected shows only the local selection — the
262-
// remote box isn't drawn (its `boxes` entry still drives the hover name). The
263-
// border is an inset box-shadow (no layout width, so it never stacks with an
264-
// adjacent cell's border) plus a subtle fill, darker while the peer is editing.
265-
isSelectionCovered(
266-
box.anchorRow,
267-
box.anchorCol,
268-
box.focusRow,
269-
box.focusCol,
270-
localSelection
271-
) ? null : (
272-
<div
273-
key={box.socketId}
274-
className='absolute rounded-xs'
275-
style={{
276-
top: box.top,
277-
left: box.left,
278-
width: box.width,
279-
height: box.height,
280-
boxShadow: `inset 0 0 0 2px ${box.color}`,
281-
backgroundColor: withAlpha(box.color, box.editing ? 0.22 : 0.08),
282-
}}
283-
/>
284-
)
285-
)}
334+
<div ref={rootRef} className='pointer-events-none absolute inset-0 overflow-hidden'>
335+
{/* Split by the frozen left zone (row gutter + pinned columns, both opaque at `z-[6]`).
336+
Ordinary-column selections sit BELOW at `z-[5]`, so scrolling one behind the gutter
337+
hides it by paint order with nothing to sync per frame; pinned ones must sit above
338+
at `z-[8]` or that cell's own opaque background swallows them. Both still clear
339+
ordinary cells, which carry no background and no z-index. */}
340+
<div className='absolute inset-0 z-[5]'>
341+
{scrollingBoxes.map((box) => (
342+
<SelectionRect key={box.socketId} box={box} />
343+
))}
344+
</div>
345+
<div className='absolute inset-0 z-[8]'>
346+
{frozenBoxes.map((box) => (
347+
<SelectionRect key={box.socketId} box={box} />
348+
))}
349+
</div>
286350
</div>
287351
{/* The name label portals to the body so it floats on top of the grid (and its
288352
sticky header) instead of being clipped by the overlay's overflow-hidden; it's

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4528,6 +4528,7 @@ export function TableGrid({
45284528
columnIndexById={columnIndexById}
45294529
rowIndexById={rowIndexById}
45304530
localSelection={normalizedSelection}
4531+
stickyLeftWidth={pinnedStickyLeftEdge}
45314532
scrollElement={scrollRef.current}
45324533
/>
45334534
)}

0 commit comments

Comments
 (0)