Skip to content

Commit 673892a

Browse files
test(block-editing): repair edit-cell-sizing no-reflow e2e for measure-and-set
The active editor REPLACES the block with a synthetic measure-and-set wrapper (no data-block-pool-id, different tag), so the old tests re-queried <tag>[data-block-pool-id] after activation and either re-resolved to a different block or detached and timed out — they never reached the actual no-reflow assertion. - Add a namespaced id #q2-active-edit-region to the wrapper div so the active editor has a stable selector (kept alongside activeEditRegionRef; namespaced to avoid colliding with Pandoc heading auto-ids in rendered content). - Rewrite the three adjacency tests to assert BOTH contract parts: (1) active-region height == block height (sizing), measured on the new wrapper; (2) following-sibling document-top unchanged (no reflow), measured scroll-invariantly. Verified green against the real q2 binary.
1 parent da80b14 commit 673892a

2 files changed

Lines changed: 97 additions & 109 deletions

File tree

Lines changed: 96 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,26 @@
11
/**
22
* E2E tests for Plan 2b P1: edit surface no-reflow (sizing + spacing).
33
*
4-
* Acceptance criteria (§P1 of the Plan 2b design prerequisites):
5-
* - Activating a block for editing does NOT shift the following sibling
6-
* (i.e. the document does not reflow). Specifically:
7-
* sibling.getBoundingClientRect().top is unchanged (±1px).
8-
* - The textarea height matches the original block's height (±1px).
4+
* The no-reflow contract has TWO parts, both asserted here:
5+
* 1. Sizing — the active edit region reproduces the original block's box:
6+
* its height is unchanged (±1px) when the block is activated for editing.
7+
* 2. No reflow — the following sibling does not move: its document-relative
8+
* top is unchanged (±1px) on activation.
9+
* (1) catches the block's own box changing size; (2) catches margin/flow
10+
* changes that a border-box height comparison alone would miss. Both are
11+
* needed — losing `margin-bottom` passes (1) but fails (2).
912
*
10-
* These tests will FAIL with the current implementation (bare <textarea>
11-
* replacing the block loses Bootstrap's element-type margin rules) and
12-
* PASS after the wrapper-element fix (keeping the <p> / <hN> element live
13-
* so CSS margins are preserved automatically).
14-
*
15-
* Plan 2b deferred note: "The reflow criterion requires real layout —
16-
* it is a Playwright test in q2-preview-spa/." These are those tests.
13+
* As-built note (measure-and-set): activating a block REPLACES it with a
14+
* synthetic wrapper `<div id="q2-active-edit-region">` (renderMeasuredEdit in
15+
* dispatchers.tsx) that reproduces the measured box; the original `<p>`/`<hN>`
16+
* is GONE while editing. So the "after" box is measured on
17+
* `#q2-active-edit-region`, never by re-querying `<tag>[data-block-pool-id]`
18+
* (that locator would re-resolve to a different block, or detach and time out).
19+
* The following-sibling locators (`<h2>`/`<ul>`) are NOT replaced, so they stay
20+
* valid across activation.
1721
*/
1822

19-
import { test, expect, type Page } from '@playwright/test';
23+
import { test, expect, type Page, type Locator } from '@playwright/test';
2024
import { startPreviewServer, type PreviewServerHandle } from './helpers/previewServer';
2125

2226
// Fixture exercises three adjacency pairs:
@@ -46,12 +50,70 @@ bottom margin must be preserved when editing is activated, just like the first p
4650
- Beta
4751
`;
4852

53+
/** Selector for the measure-and-set wrapper of the currently-active editor. */
54+
const ACTIVE_REGION = '#q2-active-edit-region';
55+
4956
/** Wait for the q2-preview sourceIndex to build and editable blocks to appear. */
5057
async function waitForEditableBlocks(page: Page): Promise<void> {
5158
const iframe = page.frameLocator('iframe');
5259
await iframe.locator('[data-block-pool-id]').first().waitFor({ timeout: 15_000 });
5360
}
5461

62+
/**
63+
* Document-relative top of an element inside the iframe (scroll-invariant —
64+
* focusing the textarea on activation can scroll the iframe, which would move
65+
* the viewport-relative `boundingBox().y` without any reflow).
66+
*/
67+
async function docTop(loc: Locator): Promise<number> {
68+
return loc.evaluate((el) => {
69+
const scroller = el.ownerDocument.scrollingElement ?? el.ownerDocument.documentElement;
70+
return el.getBoundingClientRect().top + (scroller?.scrollTop ?? 0);
71+
});
72+
}
73+
74+
/**
75+
* Drive the no-reflow contract for one adjacency case. Measures `block`'s box
76+
* and `sibling`'s top BEFORE activation (while their locators are still valid),
77+
* activates `block`, then asserts both contract parts against the active region
78+
* and the (unchanged) sibling.
79+
*/
80+
async function assertNoReflowOnActivation(
81+
page: Page,
82+
block: Locator,
83+
sibling: Locator,
84+
label: string,
85+
): Promise<void> {
86+
const iframe = page.frameLocator('iframe');
87+
88+
// Before: block box + sibling top (locators valid pre-activation).
89+
const blockBox = await block.boundingBox();
90+
expect(blockBox, `${label}: block must be visible before activation`).not.toBeNull();
91+
const siblingTopBefore = await docTop(sibling);
92+
93+
// Activate; wait for the textarea to mount.
94+
await block.click();
95+
await iframe.locator('textarea').first().waitFor({ timeout: 5_000 });
96+
97+
// After: measure the active region (the original block element is gone).
98+
const region = iframe.locator(ACTIVE_REGION);
99+
const regionBox = await region.boundingBox();
100+
expect(regionBox, `${label}: active edit region must exist after activation`).not.toBeNull();
101+
const siblingTopAfter = await docTop(sibling);
102+
103+
// Part 1 (sizing): the active region reproduces the block's box height.
104+
expect(
105+
Math.abs(regionBox!.height - blockBox!.height),
106+
`${label}: active region height changed from ${blockBox!.height}px to ${regionBox!.height}px on activation`,
107+
).toBeLessThanOrEqual(1);
108+
109+
// Part 2 (no reflow): the following sibling does not move.
110+
expect(
111+
Math.abs(siblingTopAfter - siblingTopBefore),
112+
`${label}: sibling top shifted by ${Math.abs(siblingTopAfter - siblingTopBefore).toFixed(1)}px on activation ` +
113+
`(was ${siblingTopBefore.toFixed(1)}px, now ${siblingTopAfter.toFixed(1)}px); margin-bottom was likely lost`,
114+
).toBeLessThanOrEqual(1);
115+
}
116+
55117
let server: PreviewServerHandle;
56118

57119
test.beforeEach(async () => {
@@ -68,112 +130,38 @@ test.afterEach(async () => {
68130
await server?.stop();
69131
});
70132

71-
test('paragraph: wrapper height and following heading top are unchanged on activation', async ({ page }) => {
133+
test('paragraph: active region height and following heading top unchanged on activation', async ({ page }) => {
72134
await page.goto(server.url);
73135
await waitForEditableBlocks(page);
74-
75136
const iframe = page.frameLocator('iframe');
76-
const para = iframe.locator('p[data-block-pool-id]').first();
77-
const heading = iframe.locator('h2').first();
78-
79-
// Measure before activation.
80-
const paraBox = await para.boundingBox();
81-
expect(paraBox, 'paragraph must be visible before activation').not.toBeNull();
82-
const headingTopBefore = (await heading.boundingBox())?.y ?? 0;
83-
84-
// Click to activate; wait for the textarea.
85-
await para.click();
86-
const textarea = iframe.locator('textarea').first();
87-
await textarea.waitFor({ timeout: 5_000 });
88-
89-
// Measure wrapper height after activation (P1 sizing: wrapper must keep its original size).
90-
const paraBoxAfter = await para.boundingBox();
91-
expect(paraBoxAfter, 'paragraph wrapper must stay in DOM after activation').not.toBeNull();
92-
const headingTopAfter = (await heading.boundingBox())?.y ?? 0;
93-
94-
// Wrapper (p) must not change height on activation.
95-
expect(
96-
Math.abs(paraBoxAfter!.height - paraBox!.height),
97-
`paragraph height changed from ${paraBox!.height}px to ${paraBoxAfter!.height}px on activation`,
98-
).toBeLessThanOrEqual(1);
99-
100-
// Heading must not shift (P1 no-reflow — the key assertion, fails without wrapper fix).
101-
expect(
102-
Math.abs(headingTopAfter - headingTopBefore),
103-
`heading top shifted by ${Math.abs(headingTopAfter - headingTopBefore).toFixed(1)}px on paragraph activation (was ${headingTopBefore.toFixed(1)}px, now ${headingTopAfter.toFixed(1)}px); margin-bottom was likely lost`,
104-
).toBeLessThanOrEqual(1);
137+
await assertNoReflowOnActivation(
138+
page,
139+
iframe.locator('p[data-block-pool-id]').first(),
140+
iframe.locator('h2').first(),
141+
'paragraph→heading',
142+
);
105143
});
106144

107-
test('heading: wrapper height and following list top are unchanged on activation', async ({ page }) => {
145+
test('heading: active region height and following list top unchanged on activation', async ({ page }) => {
108146
await page.goto(server.url);
109147
await waitForEditableBlocks(page);
110-
111148
const iframe = page.frameLocator('iframe');
112-
const heading = iframe.locator('h2[data-block-pool-id]').first();
113-
const list = iframe.locator('ul').first();
114-
115-
// Measure before activation.
116-
const headingBox = await heading.boundingBox();
117-
expect(headingBox, 'heading must be visible before activation').not.toBeNull();
118-
const listTopBefore = (await list.boundingBox())?.y ?? 0;
119-
120-
// Click to activate; wait for the textarea.
121-
await heading.click();
122-
const textarea = iframe.locator('textarea').first();
123-
await textarea.waitFor({ timeout: 5_000 });
124-
125-
// Measure wrapper height after activation (P1 sizing: wrapper must keep its original size).
126-
const headingBoxAfter = await heading.boundingBox();
127-
expect(headingBoxAfter, 'heading wrapper must stay in DOM after activation').not.toBeNull();
128-
const listTopAfter = (await list.boundingBox())?.y ?? 0;
129-
130-
// Wrapper (h2) must not change height on activation.
131-
expect(
132-
Math.abs(headingBoxAfter!.height - headingBox!.height),
133-
`heading height changed from ${headingBox!.height}px to ${headingBoxAfter!.height}px on activation`,
134-
).toBeLessThanOrEqual(1);
135-
136-
// List must not shift (P1 no-reflow — the key assertion, fails without wrapper fix).
137-
expect(
138-
Math.abs(listTopAfter - listTopBefore),
139-
`list top shifted by ${Math.abs(listTopAfter - listTopBefore).toFixed(1)}px on heading activation (was ${listTopBefore.toFixed(1)}px, now ${listTopAfter.toFixed(1)}px); margin-bottom was likely lost`,
140-
).toBeLessThanOrEqual(1);
149+
await assertNoReflowOnActivation(
150+
page,
151+
iframe.locator('h2[data-block-pool-id]').first(),
152+
iframe.locator('ul').first(),
153+
'heading→list',
154+
);
141155
});
142156

143-
test('paragraph above list: wrapper height and list top unchanged on activation', async ({ page }) => {
157+
test('paragraph above list: active region height and list top unchanged on activation', async ({ page }) => {
144158
await page.goto(server.url);
145159
await waitForEditableBlocks(page);
146-
147160
const iframe = page.frameLocator('iframe');
148-
// Second paragraph — sits directly above a list with no heading between them.
149-
const para = iframe.locator('p[data-block-pool-id]').nth(1);
150-
// Second list — the direct following sibling of the second paragraph.
151-
const list = iframe.locator('ul').nth(1);
152-
153-
// Measure before activation.
154-
const paraBox = await para.boundingBox();
155-
expect(paraBox, 'second paragraph must be visible before activation').not.toBeNull();
156-
const listTopBefore = (await list.boundingBox())?.y ?? 0;
157-
158-
// Click to activate; wait for the textarea.
159-
await para.click();
160-
const textarea = iframe.locator('textarea').first();
161-
await textarea.waitFor({ timeout: 5_000 });
162-
163-
// Measure wrapper height after activation.
164-
const paraBoxAfter = await para.boundingBox();
165-
expect(paraBoxAfter, 'paragraph wrapper must stay in DOM after activation').not.toBeNull();
166-
const listTopAfter = (await list.boundingBox())?.y ?? 0;
167-
168-
// Wrapper (p) must not change height on activation.
169-
expect(
170-
Math.abs(paraBoxAfter!.height - paraBox!.height),
171-
`paragraph height changed from ${paraBox!.height}px to ${paraBoxAfter!.height}px on activation`,
172-
).toBeLessThanOrEqual(1);
173-
174-
// List must not shift (para→list adjacency, no heading between them).
175-
expect(
176-
Math.abs(listTopAfter - listTopBefore),
177-
`list top shifted by ${Math.abs(listTopAfter - listTopBefore).toFixed(1)}px on paragraph activation (was ${listTopBefore.toFixed(1)}px, now ${listTopAfter.toFixed(1)}px); margin-bottom was likely lost`,
178-
).toBeLessThanOrEqual(1);
161+
await assertNoReflowOnActivation(
162+
page,
163+
iframe.locator('p[data-block-pool-id]').nth(1),
164+
iframe.locator('ul').nth(1),
165+
'paragraph→list',
166+
);
179167
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ function renderMeasuredEdit(
6464
}
6565
return (
6666
<AttributionWrap node={node} as="div">
67-
<div ref={activeEditRegionRef} style={wrapperStyle}>{textarea}</div>
67+
<div ref={activeEditRegionRef} id="q2-active-edit-region" style={wrapperStyle}>{textarea}</div>
6868
</AttributionWrap>
6969
);
7070
}

0 commit comments

Comments
 (0)