Skip to content

Commit e2bd1d0

Browse files
committed
fix(ui): table popout — annotation flow, chrome stacking, sidebar tabs
Tightens the popout so annotations work inside it and chrome doesn't overlap the dialog. Annotation flow inside popout: - Radix Dialog modal={false} so the focus trap doesn't yank focus back from CommentPopover's textarea (CommentPopover portals to document.body, outside the dialog's DOM subtree). - Dialog.Content onInteractOutside handler whitelists the annotation toolbar, CommentPopover, and FloatingQuickLabelPicker so clicking them doesn't dismiss the dialog. Backdrop click + Escape still close. - aria-describedby={undefined} on Dialog.Content (Radix opt-out; the popout doesn't need a description). - React.memo on TablePopout with a custom comparator (block id/content, open, container, imageBaseDir, githubRepo). Prevents upstream Viewer re-renders from re-running TanStack's flexRender on every cell, which conflicted with web-highlighter's live DOM mutations and caused a NotFoundError in React's reconciler. Widget markers for :has()-based chrome stacking: - [data-comment-popover="true"] on CommentPopover (both popover + dialog variants). - [data-floating-picker="true"] on FloatingQuickLabelPicker. - [data-sidebar-tabs="true"] on SidebarTabs (left-side TOC/Files/Versions flags that sit on top of the dialog otherwise). - theme.css extended: sidebar tabs join the annotation sidebar, sticky header lane, app header, and overlay scrollbars in dropping to z-index -1 while body:has([data-popout="true"]) matches. Known limitation (not addressed): annotations created inside the popout show their <mark> only while the popout is open; when it closes, the <mark> unmounts with the popout's DOM and does not reappear on the collapsed table. The annotation itself persists in state (sidebar, shared URLs, exports). Round-tripping visual marks between popout and collapsed view requires either a second web-highlighter instance or a switch to the CSS Custom Highlight API — out of scope here. For provenance purposes, this commit was AI assisted.
1 parent 66b48fb commit e2bd1d0

5 files changed

Lines changed: 44 additions & 3 deletions

File tree

packages/ui/components/CommentPopover.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ export const CommentPopover: React.FC<CommentPopoverProps> = ({
137137

138138
if (mode === 'dialog') {
139139
return createPortal(
140-
<div className="fixed inset-0 z-[100] flex items-center justify-center p-4">
140+
<div data-comment-popover="true" className="fixed inset-0 z-[100] flex items-center justify-center p-4">
141141
{/* Backdrop */}
142142
<div className="absolute inset-0 bg-background/80 backdrop-blur-sm" />
143143

@@ -226,6 +226,7 @@ export const CommentPopover: React.FC<CommentPopoverProps> = ({
226226
return createPortal(
227227
<div
228228
ref={popoverRef}
229+
data-comment-popover="true"
229230
className="fixed z-[100] bg-popover border border-border rounded-xl shadow-2xl flex flex-col"
230231
style={dragPosition
231232
? { top: dragPosition.top, left: dragPosition.left, width: position.width }

packages/ui/components/FloatingQuickLabelPicker.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ export const FloatingQuickLabelPicker: React.FC<FloatingQuickLabelPickerProps> =
113113
<div
114114
ref={ref}
115115
data-quick-label-picker
116+
data-floating-picker="true"
116117
className="fixed z-[100]"
117118
style={{
118119
top: position.top,

packages/ui/components/blocks/TablePopout.tsx

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ interface TablePopoutProps {
3232
// Table expects for an accessor-based column definition.
3333
type Row = Record<string, string>;
3434

35-
export const TablePopout: React.FC<TablePopoutProps> = ({
35+
const TablePopoutImpl: React.FC<TablePopoutProps> = ({
3636
block,
3737
open,
3838
onClose,
@@ -135,14 +135,31 @@ export const TablePopout: React.FC<TablePopoutProps> = ({
135135
const totalRows = data.length;
136136

137137
return (
138-
<Dialog.Root open={open} onOpenChange={(next) => { if (!next) onClose(); }}>
138+
<Dialog.Root open={open} onOpenChange={(next) => { if (!next) onClose(); }} modal={false}>
139139
<Dialog.Portal container={container ?? undefined}>
140140
<Dialog.Overlay className="fixed inset-0 z-50 bg-black/50 backdrop-blur-[2px] data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=open]:fade-in-0 data-[state=closed]:fade-out-0" />
141141
<Dialog.Content
142142
className="fixed left-1/2 top-1/2 z-50 w-[calc(100vw-4rem)] max-w-[min(calc(100vw-4rem),1500px)] max-h-[calc(100vh-4rem)] -translate-x-1/2 -translate-y-1/2 bg-background border border-border rounded-xl shadow-2xl flex flex-col overflow-hidden"
143143
data-block-id={block.id}
144144
data-popout="true"
145+
aria-describedby={undefined}
145146
onOpenAutoFocus={(e) => e.preventDefault()}
147+
onInteractOutside={(e) => {
148+
// With modal={false}, Radix treats any click outside Dialog.Content
149+
// as dismissal. Our annotation stack (toolbar, comment popover,
150+
// quick-label picker) portals to document.body — clicks on them
151+
// are outside the dialog DOM but logically part of this session.
152+
// Keep the dialog open when the interaction lands on any of them.
153+
const target = e.target as Node | null;
154+
if (!target || !(target instanceof Element)) return;
155+
if (
156+
target.closest('.annotation-toolbar') ||
157+
target.closest('[data-comment-popover="true"]') ||
158+
target.closest('[data-floating-picker="true"]')
159+
) {
160+
e.preventDefault();
161+
}
162+
}}
146163
>
147164
<Dialog.Title className="sr-only">Table</Dialog.Title>
148165
<Dialog.Close asChild>
@@ -266,6 +283,26 @@ export const TablePopout: React.FC<TablePopoutProps> = ({
266283
);
267284
};
268285

286+
// Memoize on meaningful props (block identity, content, open, container).
287+
// Upstream Viewer re-renders (annotation toolbar opening, selection change,
288+
// hover state shuffling) keep firing while the popout is mounted. Without
289+
// this memo, TanStack's flexRender re-evaluates every cell on every parent
290+
// re-render, which conflicts with web-highlighter's DOM mutations (the
291+
// library inserts <mark> tags into the live DOM) and React's reconciler
292+
// throws NotFoundError trying to remove nodes it doesn't own.
293+
// Callback identity is intentionally ignored — the behavior is stable even
294+
// if the parent hands us new function references.
295+
export const TablePopout = React.memo(
296+
TablePopoutImpl,
297+
(prev, next) =>
298+
prev.block.id === next.block.id &&
299+
prev.block.content === next.block.content &&
300+
prev.open === next.open &&
301+
prev.container === next.container &&
302+
prev.imageBaseDir === next.imageBaseDir &&
303+
prev.githubRepo === next.githubRepo,
304+
);
305+
269306
const SortIndicator: React.FC<{ dir: false | 'asc' | 'desc' }> = ({ dir }) => {
270307
const activeUp = dir === 'asc';
271308
const activeDown = dir === 'desc';

packages/ui/components/sidebar/SidebarTabs.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export const SidebarTabs: React.FC<SidebarTabsProps> = ({
2929
}) => {
3030
return (
3131
<div
32+
data-sidebar-tabs="true"
3233
className={`flex flex-col gap-1 pt-3 pl-0.5 flex-shrink-0 ${className ?? ""}`}
3334
>
3435
{/* TOC tab */}

packages/ui/theme.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ body {
302302
body:has([data-popout="true"]) [data-annotation-panel="true"],
303303
body:has([data-popout="true"]) [data-sticky-header-lane="true"],
304304
body:has([data-popout="true"]) [data-app-header="true"],
305+
body:has([data-popout="true"]) [data-sidebar-tabs="true"],
305306
body:has([data-popout="true"]) .os-scrollbar {
306307
z-index: -1;
307308
}

0 commit comments

Comments
 (0)