Skip to content

Commit d6066dc

Browse files
cscheidclaude
andcommitted
fix(preview): flip edit chrome below the block when cropped at the viewport top (bd-pvcnea83)
Editing the first block of a title-less document (or any block scrolled near the viewport top) cropped the floating edit chrome: the rich-text toolbar and the standalone breadcrumb chip float ABOVE the edit box (`bottom:100%`), so for a block flush against the top of the scroll area they landed above the viewport (scrollTop already 0) and were clipped — only a sliver showed. The inline nesting breadcrumb lives in the toolbar, so it cropped too. Collision-aware flip: when there isn't room above (`surfaceTop - chromeHeight - gap < 0`, viewport-relative), render the chrome BELOW the block instead. Parity-neutral (no change to document spacing) and generalizes to any near-top block, not just the literal first one. - New pure helper `editChromeGeometry.shouldPlaceChromeBelow` (unit-tested). - `RichTextToolbar`: useLayoutEffect measures the edit box top + toolbar height and adds `q2-rt-toolbar-below` (`top:100%`) when it would clip; guarded on a non-zero measured height so degenerate (jsdom) layouts keep the default above. - `BreadcrumbChip`: the geometry effect flips `top` below the surface under the same condition (guarded on `sRect.height > 0`); horizontal left-spill geometry unchanged. Both decide from the surface's stable top (not the chrome's flipped position), so there is no flip/re-measure loop. Verified end-to-end in the q2 binary: editing the first paragraph flips the toolbar below (top 48.5, uncropped); editing a first code block flips the standalone chip below (top 83.8, uncropped). e2e: q2-preview-spa/e2e/edit-chrome-placement.spec.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b6b1a31 commit d6066dc

6 files changed

Lines changed: 211 additions & 4 deletions

File tree

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/**
2+
* bd-pvcnea83 — floating edit chrome must not be cropped at the top of the
3+
* viewport. Editing the first block of a title-less document (flush against the
4+
* scroll-area top) previously clipped the chrome, which floats ABOVE the edit
5+
* box (`bottom:100%`), above the viewport top with no way to scroll up to it.
6+
* The fix flips the chrome BELOW the block when there's no room above.
7+
*
8+
* Real-binary e2e (drives target/debug/q2 via startPreviewServer). The pure
9+
* placement threshold is unit-tested in
10+
* ts-packages/preview-renderer/src/q2-preview/editChromeGeometry.test.ts; jsdom
11+
* rects are degenerate, so the actual flip is only observable here.
12+
*
13+
* Build chain prerequisite (the binary does NOT auto-rebuild the embedded SPA):
14+
* cargo xtask build-q2-preview-spa
15+
* cargo build -p quarto --bin q2
16+
*/
17+
18+
import { test, expect, type Page } from '@playwright/test';
19+
import { startPreviewServer, type PreviewServerHandle } from './helpers/previewServer';
20+
21+
let server: PreviewServerHandle;
22+
23+
/** Viewport-relative top of an element inside the preview iframe (0 = top of the
24+
* visible scroll area). A clipped-above-the-top element has a negative top. */
25+
async function iframeRectTop(page: Page, selector: string): Promise<number> {
26+
return page.frameLocator('iframe').locator(selector).first().evaluate(
27+
(el) => el.getBoundingClientRect().top,
28+
);
29+
}
30+
31+
test.describe('bd-pvcnea83 — edit chrome flips below at the top of the viewport', () => {
32+
test.setTimeout(120_000);
33+
34+
test.afterEach(async () => {
35+
await server?.stop();
36+
});
37+
38+
test('rich-text toolbar: editing the first (title-less) paragraph flips the toolbar below, uncropped', async ({ page }) => {
39+
server = await startPreviewServer({
40+
allowEdit: true,
41+
fixtureFiles: [{
42+
path: 'index.qmd',
43+
// No title front matter: the first paragraph is flush against the top.
44+
content: 'First paragraph at the very top.\n\nSecond paragraph.\n',
45+
}],
46+
});
47+
await page.goto(server.url);
48+
const iframe = page.frameLocator('iframe');
49+
await page.waitForFunction(() => {
50+
const inner = document.querySelector('iframe')?.contentDocument;
51+
return inner?.querySelector('p[data-block-pool-id]') != null;
52+
}, null, { timeout: 30_000 });
53+
54+
await iframe.locator('p[data-block-pool-id]').first().click();
55+
56+
const toolbar = iframe.locator('.q2-rt-toolbar').first();
57+
await toolbar.waitFor({ timeout: 10_000 });
58+
59+
// Flipped below (the collision-avoidance class) ...
60+
await expect(toolbar, 'toolbar must flip below at the top of the document')
61+
.toHaveClass(/q2-rt-toolbar-below/);
62+
// ... and consequently not cropped above the viewport top.
63+
await expect
64+
.poll(() => iframeRectTop(page, '.q2-rt-toolbar'), {
65+
timeout: 8000,
66+
message: 'toolbar top must be >= 0 (not clipped above the viewport)',
67+
})
68+
.toBeGreaterThanOrEqual(0);
69+
});
70+
71+
test('standalone breadcrumb chip: editing a first (title-less) code block flips the chip below, uncropped', async ({ page }) => {
72+
server = await startPreviewServer({
73+
allowEdit: true,
74+
fixtureFiles: [{
75+
path: 'index.qmd',
76+
// First block is a code block (non-rich) → textarea + standalone chip.
77+
content: '```python\nx = 1\ny = 2\n```\n\nA paragraph after.\n',
78+
}],
79+
});
80+
await page.goto(server.url);
81+
const iframe = page.frameLocator('iframe');
82+
await page.waitForFunction(() => {
83+
const inner = document.querySelector('iframe')?.contentDocument;
84+
return inner?.querySelector('[data-block-pool-id]') != null;
85+
}, null, { timeout: 30_000 });
86+
87+
await iframe.locator('[data-block-pool-id]').first().click();
88+
await iframe.locator('#q2-active-edit-region textarea').first().waitFor({ timeout: 10_000 });
89+
90+
const chip = iframe.locator('[data-testid="q2-breadcrumb-chip"]');
91+
await chip.waitFor({ timeout: 5000 });
92+
await expect
93+
.poll(() => iframeRectTop(page, '[data-testid="q2-breadcrumb-chip"]'), {
94+
timeout: 8000,
95+
message: 'standalone chip top must be >= 0 (not clipped above the viewport)',
96+
})
97+
.toBeGreaterThanOrEqual(0);
98+
});
99+
});

ts-packages/preview-renderer/src/q2-preview/BreadcrumbChip.tsx

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ import { buildAncestorPath, currentSourceNodeType } from './nestingNav';
5757
import type { AncestorCrumb } from './nestingNav';
5858
import { BreadcrumbCrumbs, type CrumbDisplayItem } from './BreadcrumbCrumbs';
5959
import { richEditorActiveForType } from './richTextSupport';
60+
import { shouldPlaceChromeBelow } from './editChromeGeometry';
61+
62+
/** Gap (px) between the chip and the surface when flipped below it (bd-pvcnea83). */
63+
const CHIP_FLIP_GAP = 4;
6064

6165
// ── Constants ──────────────────────────────────────────────────────────────────
6266

@@ -255,9 +259,20 @@ export function BreadcrumbChip(): React.ReactElement | null {
255259
);
256260
const displayItems = selectDisplayItems(crumbs, slots);
257261

258-
// --- Chip top: bottom edge flush at surface top ---
262+
// --- Chip top: above the surface by default; flip below when clipped ---
263+
// Default: bottom edge flush at the surface top (top = surfaceTop − chipH).
264+
// But for a surface flush against the viewport top (e.g. the first block of
265+
// a title-less document) that lands above the scroll area and is cropped, so
266+
// flip BELOW the surface instead (bd-pvcnea83). Decide from the surface's
267+
// viewport-relative top (sRect.top), guarded on real geometry so jsdom's
268+
// zero-rects keep the default 'above' placement.
259269
const chipH = chipRef.current?.getBoundingClientRect().height ?? 0;
260-
const top = surfaceTop - chipH;
270+
const haveRealGeometry = sRect.height > 0;
271+
const flipBelow = haveRealGeometry
272+
&& shouldPlaceChromeBelow(sRect.top, chipH, CHIP_FLIP_GAP);
273+
const top = flipBelow
274+
? (sRect.bottom - hostRect.top) + CHIP_FLIP_GAP // below the surface
275+
: surfaceTop - chipH; // above (default)
261276

262277
setGeom({ top, chipLeft, bandWidth, displayItems });
263278
}, [active, et?.anchorR0, et?.anchorR1, ctx?.activeEditRegionRef, ctx?.sourceIndex]);
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* Unit tests for shouldPlaceChromeBelow (bd-pvcnea83): floating edit chrome
3+
* flips below the surface when there isn't room above it at the viewport top.
4+
*/
5+
6+
import { describe, it, expect } from 'vitest';
7+
import { shouldPlaceChromeBelow } from './editChromeGeometry';
8+
9+
describe('shouldPlaceChromeBelow', () => {
10+
it('flips below when the chrome would clip above the viewport top (the measured first-block case)', () => {
11+
// editor top 15, toolbar height 26, gap 4 → 15 - 26 - 4 = -15 < 0.
12+
expect(shouldPlaceChromeBelow(15, 26, 4)).toBe(true);
13+
});
14+
15+
it('stays above when there is ample room above', () => {
16+
expect(shouldPlaceChromeBelow(100, 26, 4)).toBe(false);
17+
});
18+
19+
it('stays above with exactly enough room (chromeHeight + gap)', () => {
20+
// surfaceTop === chromeHeight + gap → 30 - 26 - 4 = 0, not < 0.
21+
expect(shouldPlaceChromeBelow(30, 26, 4)).toBe(false);
22+
});
23+
24+
it('flips below one pixel short of enough room', () => {
25+
expect(shouldPlaceChromeBelow(29, 26, 4)).toBe(true);
26+
});
27+
28+
it('flips below for a surface flush at the viewport top', () => {
29+
expect(shouldPlaceChromeBelow(0, 26, 4)).toBe(true);
30+
});
31+
});
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* editChromeGeometry.ts — shared vertical-placement geometry for floating edit
3+
* chrome (the rich-text toolbar and the standalone breadcrumb chip). Pure (no
4+
* DOM, no React) so it is unit-testable; the components measure rects and call
5+
* in. bd-pvcnea83.
6+
*/
7+
8+
/**
9+
* True when floating chrome of `chromeHeight` placed ABOVE a surface whose
10+
* viewport-relative top edge is `surfaceTop` would be clipped above the viewport
11+
* top — i.e. there isn't `chromeHeight + gap` of room above it. The caller then
12+
* flips the chrome BELOW the surface instead.
13+
*
14+
* `surfaceTop` is the surface's `getBoundingClientRect().top` (viewport
15+
* coordinates; in the preview iframe, 0 is the top of the visible scroll area).
16+
* Computing from the *surface's* top (which doesn't move when the chrome flips)
17+
* keeps the decision stable — no flip/re-measure loop.
18+
*/
19+
export function shouldPlaceChromeBelow(
20+
surfaceTop: number,
21+
chromeHeight: number,
22+
gap: number,
23+
): boolean {
24+
return surfaceTop - chromeHeight - gap < 0;
25+
}

ts-packages/preview-renderer/src/q2-preview/richtext/RichTextToolbar.tsx

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@
1313
// input DOES take focus; the editor's commit is scoped to "focus left the whole
1414
// edit box" (see RichTextEditor), so focusing the input keeps the session open.
1515

16-
import { useEffect, useRef, useState, type MouseEvent, type ReactNode } from 'react';
16+
import { useEffect, useLayoutEffect, useRef, useState, type MouseEvent, type ReactNode } from 'react';
1717
import type { Editor } from '@tiptap/core';
18+
import { shouldPlaceChromeBelow } from '../editChromeGeometry';
19+
20+
/** Gap (px) between the toolbar and the edit box, matching the CSS margin. */
21+
const TOOLBAR_GAP = 4;
1822

1923
interface MarkSpec {
2024
name: string;
@@ -51,6 +55,26 @@ export function RichTextToolbar({
5155
};
5256
}, [editor]);
5357

58+
// Vertical placement (bd-pvcnea83): the toolbar floats ABOVE the edit box by
59+
// default, but flips BELOW when there isn't room above (e.g. editing the first
60+
// block of a title-less document, flush against the viewport top — otherwise it
61+
// is clipped above the scroll area, with no way to scroll up to it).
62+
const toolbarRef = useRef<HTMLDivElement | null>(null);
63+
const [placeBelow, setPlaceBelow] = useState(false);
64+
useLayoutEffect(() => {
65+
const tb = toolbarRef.current;
66+
const box = tb?.closest('.q2-richtext-editor');
67+
if (!tb || !box) return;
68+
const height = tb.offsetHeight;
69+
// Degenerate layout (jsdom zero-rects): keep the default 'above' placement.
70+
if (height <= 0) return;
71+
const surfaceTop = box.getBoundingClientRect().top;
72+
setPlaceBelow(shouldPlaceChromeBelow(surfaceTop, height, TOOLBAR_GAP));
73+
// Mount-only: the toolbar remounts per edit target, and the edit box's top is
74+
// stable for a given target, so a single measurement suffices.
75+
// eslint-disable-next-line react-hooks/exhaustive-deps
76+
}, []);
77+
5478
const [linkOpen, setLinkOpen] = useState(false);
5579
const [linkUrl, setLinkUrl] = useState('');
5680
const linkInputRef = useRef<HTMLInputElement | null>(null);
@@ -103,7 +127,11 @@ export function RichTextToolbar({
103127
};
104128

105129
return (
106-
<div className="q2-rt-toolbar" contentEditable={false}>
130+
<div
131+
ref={toolbarRef}
132+
className={`q2-rt-toolbar${placeBelow ? ' q2-rt-toolbar-below' : ''}`}
133+
contentEditable={false}
134+
>
107135
{!linkOpen ? (
108136
<>
109137
{MARKS.map((m) => (

ts-packages/preview-renderer/src/q2-preview/richtext/styles.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ const CSS = `
7878
font-size: 0.8rem;
7979
line-height: 1;
8080
}
81+
/* bd-pvcnea83: flip below the edit box when there is no room above (e.g. the
82+
first block of a title-less document, flush against the viewport top — the
83+
default bottom:100% placement would clip the toolbar above the scroll area). */
84+
.q2-rt-toolbar.q2-rt-toolbar-below {
85+
bottom: auto;
86+
top: 100%;
87+
margin-bottom: 0;
88+
margin-top: 4px;
89+
}
8190
.q2-rt-tb-btn {
8291
appearance: none;
8392
border: none;

0 commit comments

Comments
 (0)