Skip to content

Commit fed4689

Browse files
committed
fix(tables): stop remote cell selections painting over the row gutter
1 parent c530d27 commit fed4689

4 files changed

Lines changed: 109 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: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,9 @@ export const DataRow = React.memo(function DataRow({
315315
data-row={rowIndex}
316316
data-row-id={row.id}
317317
data-col={colIndex}
318+
// Read by overlays measured off these cells (see remote-selection-overlay.tsx)
319+
// to tell a cell frozen in the sticky zone from one scrolled behind it.
320+
data-pinned={isPinnedCell ? '' : undefined}
318321
className={cn(
319322
CELL,
320323
(isHighlighted || isAnchor || isEditing) && 'relative',

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

Lines changed: 103 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,25 @@ 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+
/** The cell `<td>` for a (rowId, columnIndex), or null when virtualized off-window. */
54+
function cellElement(
4855
scrollEl: HTMLElement,
4956
rowId: string,
5057
columnIndex: number | undefined
51-
): DOMRect | undefined {
52-
if (columnIndex === undefined) return undefined
58+
): HTMLElement | null {
59+
if (columnIndex === undefined) return null
5360
// `rowId` is a remote peer's value — escape it so a hostile id can't break the
5461
// selector and throw (`columnIndex` is a local numeric index, already safe).
55-
const cell = scrollEl.querySelector(
62+
return scrollEl.querySelector<HTMLElement>(
5663
`[data-row-id="${CSS.escape(rowId)}"][data-col="${columnIndex}"]`
5764
)
58-
return cell?.getBoundingClientRect()
5965
}
6066

6167
/**
@@ -76,6 +82,30 @@ function isSelectionCovered(
7682
)
7783
}
7884

85+
/**
86+
* One peer's selection rectangle. The border is an inset box-shadow (no layout width, so it
87+
* never stacks with an adjacent cell's border) plus a subtle fill, darker while they edit.
88+
*/
89+
interface SelectionRectProps {
90+
box: SelectionBox
91+
}
92+
93+
function SelectionRect({ box }: SelectionRectProps) {
94+
return (
95+
<div
96+
className='absolute rounded-xs'
97+
style={{
98+
top: box.top,
99+
left: box.left,
100+
width: box.width,
101+
height: box.height,
102+
boxShadow: `inset 0 0 0 2px ${box.color}`,
103+
backgroundColor: withAlpha(box.color, box.editing ? 0.22 : 0.08),
104+
}}
105+
/>
106+
)
107+
}
108+
79109
/**
80110
* Renders remote collaborators' cell selections over the table grid — a colored
81111
* border per user (Google-Sheets style), a darker fill while they are editing, and
@@ -93,6 +123,7 @@ export function RemoteSelectionOverlay({
93123
columnIndexById,
94124
rowIndexById,
95125
localSelection,
126+
stickyLeftWidth,
96127
scrollElement,
97128
}: RemoteSelectionOverlayProps) {
98129
const rootRef = useRef<HTMLDivElement>(null)
@@ -112,28 +143,44 @@ export function RemoteSelectionOverlay({
112143
// Read only by the pointer hit-test (never in render) to skip a locally-covered box.
113144
const localSelectionRef = useRef(localSelection)
114145
localSelectionRef.current = localSelection
146+
// Read via ref so a column resize or a pin/unpin never re-subscribes the listeners.
147+
const stickyLeftWidthRef = useRef(stickyLeftWidth)
148+
stickyLeftWidthRef.current = stickyLeftWidth
115149
// Cached content-wrapper origin, refreshed on each measure (scroll/resize/data change),
116150
// so the pointer hit-test never forces a layout read per mouse move.
117151
const originRef = useRef({ top: 0, left: 0 })
152+
// Content-space x of the frozen zone's right edge. Paint order hides a box behind the zone
153+
// (see the layers in render), but the hover hit-test is plain JS and has to exclude it by
154+
// hand — so this is refreshed on every scroll event, not just on the rAF-throttled measure,
155+
// and can never trail the pointer.
156+
const frozenEdgeXRef = useRef(0)
118157

119158
const measure = useCallback(() => {
120159
const scrollEl = scrollElement
121160
const root = rootRef.current
122161
if (!scrollEl || !root) return
162+
frozenEdgeXRef.current = scrollEl.scrollLeft + stickyLeftWidthRef.current
123163
const origin = root.getBoundingClientRect()
124164
originRef.current = { top: origin.top, left: origin.left }
165+
// The wrapper is the scroller's only child, so its origin already encodes the scroll
166+
// offset — no second `getBoundingClientRect()` for the frozen zone's viewport x.
167+
const stickyViewportX = origin.left + frozenEdgeXRef.current
125168
const next: SelectionBox[] = []
126169
for (const selection of remoteSelectionsRef.current) {
127170
const { anchor, focus, editing } = selection.cell
128171
const anchorCol = columnIndexByIdRef.current.get(anchor.columnId)
129172
const focusCol = columnIndexByIdRef.current.get(focus.columnId)
130173
const anchorRow = rowIndexByIdRef.current.get(anchor.rowId)
131174
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
175+
const cells = [
176+
cellElement(scrollEl, anchor.rowId, anchorCol),
177+
cellElement(scrollEl, focus.rowId, focusCol),
178+
].filter((cell): cell is HTMLElement => cell !== null)
179+
if (cells.length === 0) continue
180+
const rects = cells.map((cell) => cell.getBoundingClientRect())
181+
// A range straddling the boundary defers to the frozen zone, so its scrolled-away half
182+
// can't bleed over the gutter — the conservative half of the trade, and the rarer case.
183+
const pinned = cells.every((cell) => cell.hasAttribute('data-pinned'))
137184

138185
const viewportTop = Math.min(...rects.map((r) => r.top))
139186
const viewportLeft = Math.min(...rects.map((r) => r.left))
@@ -150,8 +197,9 @@ export function RemoteSelectionOverlay({
150197
left,
151198
width: right - left,
152199
height: bottom - top,
200+
pinned,
153201
viewportTop,
154-
viewportLeft,
202+
viewportLeft: pinned ? viewportLeft : Math.max(viewportLeft, stickyViewportX),
155203
anchorRow,
156204
anchorCol,
157205
focusRow,
@@ -169,6 +217,9 @@ export function RemoteSelectionOverlay({
169217

170218
let raf = 0
171219
const schedule = () => {
220+
// Plain number, no DOM write: the hit-test needs the frozen edge on every event, but
221+
// the boxes' own occlusion is paint-order and needs nothing from JS.
222+
frozenEdgeXRef.current = scrollEl.scrollLeft + stickyLeftWidthRef.current
172223
if (!raf)
173224
raf = requestAnimationFrame(() => {
174225
raf = 0
@@ -181,6 +232,9 @@ export function RemoteSelectionOverlay({
181232
const y = event.clientY - top
182233
const hit = boxesRef.current.find(
183234
(b) =>
235+
// Only the part of the box that clears the frozen zone is painted — hovering the
236+
// row gutter it hides behind must not pop the peer's name tag.
237+
(b.pinned || x >= frozenEdgeXRef.current) &&
184238
x >= b.left &&
185239
x <= b.left + b.width &&
186240
y >= b.top &&
@@ -233,56 +287,48 @@ export function RemoteSelectionOverlay({
233287
// subscribed). Layout effect so positions update before paint — no one-frame lag as a
234288
// peer moves. NOT keyed on `localSelection`: moving the local caret changes only which
235289
// boxes are `covered`, which the cheap in-memory pass below handles without a reflow.
290+
// `stickyLeftWidth` is a dep too: pinning a column moves the frozen zone's edge without
291+
// resizing the content, so nothing else would refresh the hit-test's boundary.
236292
useLayoutEffect(() => {
237293
measure()
238-
}, [remoteSelections, columnIndexById, measure])
294+
}, [remoteSelections, columnIndexById, stickyLeftWidth, measure])
239295

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
296+
// Partitioned in render so it reacts to `localSelection`: a cell the local user also has
297+
// selected shows only the local selection — the remote box isn't drawn (its `boxes` entry
298+
// still drives the hover name). Resolving the hovered box in the same pass means that when
299+
// the local selection grows to cover it without another pointer move, the floating name tag
300+
// drops rather than lingering over cells with no visible remote selection.
301+
const scrollingBoxes: SelectionBox[] = []
302+
const frozenBoxes: SelectionBox[] = []
303+
let hoveredBox: SelectionBox | undefined
304+
for (const box of boxes) {
305+
if (
306+
isSelectionCovered(box.anchorRow, box.anchorCol, box.focusRow, box.focusCol, localSelection)
307+
) {
308+
continue
309+
}
310+
;(box.pinned ? frozenBoxes : scrollingBoxes).push(box)
311+
if (box.socketId === hoveredSocketId) hoveredBox = box
312+
}
256313

257314
return (
258315
<>
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-
)}
316+
<div ref={rootRef} className='pointer-events-none absolute inset-0 overflow-hidden'>
317+
{/* Split by the frozen left zone (row gutter + pinned columns, both opaque at `z-[6]`).
318+
Ordinary-column selections sit BELOW at `z-[5]`, so scrolling one behind the gutter
319+
hides it by paint order with nothing to sync per frame; pinned ones must sit above
320+
at `z-[8]` or that cell's own opaque background swallows them. Both still clear
321+
ordinary cells, which carry no background and no z-index. */}
322+
<div className='absolute inset-0 z-[5]'>
323+
{scrollingBoxes.map((box) => (
324+
<SelectionRect key={box.socketId} box={box} />
325+
))}
326+
</div>
327+
<div className='absolute inset-0 z-[8]'>
328+
{frozenBoxes.map((box) => (
329+
<SelectionRect key={box.socketId} box={box} />
330+
))}
331+
</div>
286332
</div>
287333
{/* The name label portals to the body so it floats on top of the grid (and its
288334
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)