Skip to content

Commit b611f5e

Browse files
hellpanderrrclaude
andcommitted
fix(macronizer): details toggle dragged popup+summary up (M-023h.4)
Growing a popup that floats ABOVE its word extends it upward — expanding Analysis details slid the just-clicked summary ~250px out from under the cursor (visible on low words like "qui"). The rAF fix removed the flash but the final position still moved. Fix: on expand, do NOT reposition — keep the popup's top pinned and clamp the expanded content into the room below (maxHeight = viewport - offsetTop, overflowY:auto), so the summary stays under the cursor. Collapse restores normal placement. Verified across 7 words: top stable within 1px on both expand and collapse, never hidden, never out of viewport. 7 popup + 19 macronizer e2e + 22 unit + 81 IPA + 2067 golden + 348 census green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 127af4d commit b611f5e

3 files changed

Lines changed: 49 additions & 20 deletions

File tree

wiktionary_pron/docs/ISSUES.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,3 +1159,24 @@ stable position (no flash/jump) and no viewport overflow.
11591159

11601160
**Status: FIXED.** 7 popup + 19 macronizer e2e + 22 unit + 81 IPA + 2067 golden
11611161
+ 348 census, all green.
1162+
1163+
## M-023h.4 — Details toggle dragged the popup (and summary) up (2026-08-12) ✅
1164+
1165+
**Symptom.** On words near the bottom (e.g. "qui" in a full sentence), clicking
1166+
"Analysis details" made the popup jump up: the just-clicked <summary> slid
1167+
~250px out from under the cursor. The rAF fix (h.3) removed the two-frame
1168+
flash but the FINAL position itself moved, because growing a popup that floats
1169+
ABOVE the word extends it upward.
1170+
1171+
**Root cause.** Repositioning on expand recomputed top for the new, taller
1172+
height — the top moved up and the popup grew away from the cursor.
1173+
1174+
**Fix.** On expand, do NOT reposition: keep the popup's top pinned where it is
1175+
and clamp the expanded content into the room below (`maxHeight = viewport -
1176+
offsetTop`, `overflowY:auto`). The <summary> stays under the cursor; extra
1177+
detail scrolls internally. On collapse, restore normal placement (so a stale
1178+
tall maxHeight doesn't persist). Verified across 7 words: top stays within
1179+
1px on both expand and collapse, never hidden, never out of viewport.
1180+
1181+
**Status: FIXED.** 7 popup + 19 macronizer e2e + 22 unit + 81 IPA + 2067 golden
1182+
+ 348 census, all green.

wiktionary_pron/e2e/popup-check.spec.js

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,12 @@ test("CSV split button exports per word (default) and per line from the dropdown
249249
expect(lineDownload2.suggestedFilename()).toMatch(/\.csv$/);
250250
});
251251

252-
test("popup renders above the word, survives details toggle, stays in viewport", async () => {
253-
// Regression for M-023h (+ .1, + .3): the popup is anchored ABOVE the word so
254-
// expanding "Analysis details" grows it downward, away from the cursor. The
255-
// toggle reposition must (a) not dismiss the popup, (b) stay in the viewport,
256-
// and (c) settle on ONE position — a reflow race previously flashed it at the
257-
// old spot then moved it. Deferring positionPopup to the next frame fixes (c).
252+
test("popup renders above the word, details toggle keeps it in place", async () => {
253+
// Regression for M-023h (+ .1–.4): expanding "Analysis details" must NOT move
254+
// the popup. Growing it upward (when floating above the word) drags the just-
255+
// clicked <summary> out from under the cursor — the "popup jumps up" complaint.
256+
// The fix keeps the popup's top pinned and clamps the expanded content into the
257+
// space below with an internal scrollbar; collapsing restores normal placement.
258258
const page = sharedPage;
259259
test.setTimeout(300_000);
260260
await page.goto(PAGE);
@@ -272,7 +272,7 @@ test("popup renders above the word, survives details toggle, stays in viewport",
272272
expect(before.y + before.height).toBeLessThanOrEqual(wordRect.y + wordRect.height + 1);
273273
const summary = page.locator(".word-popup details.popup-analysis summary");
274274
const sb = await summary.boundingBox();
275-
// sample the popup position around the click — must settle on ONE y, not flash
275+
// sample the popup y around the click — the top must NOT move at all (no jump)
276276
const ys = [];
277277
const poll = (async () => {
278278
for (let t = 0; t < 8; t++) {
@@ -283,10 +283,15 @@ test("popup renders above the word, survives details toggle, stays in viewport",
283283
})();
284284
await page.mouse.click(sb.x + sb.width / 2, sb.y + sb.height / 2);
285285
await poll;
286-
// still visible after the toggle reposition
286+
// still visible after the toggle
287287
await expect(page.locator(".word-popup")).toBeVisible({ timeout: 5000 });
288-
expect(new Set(ys).size).toBeLessThanOrEqual(1); // no jump
289-
const box = await page.locator(".word-popup").boundingBox();
288+
const unique = [...new Set(ys)];
289+
expect(unique.length).toBeLessThanOrEqual(1);
290+
if (unique.length) expect(Math.abs(unique[0] - Math.round(before.y))).toBeLessThanOrEqual(1);
291+
// collapse restores normal placement (top comes back if it had been clamped)
292+
await page.mouse.click(sb.x + sb.width / 2, sb.y + sb.height / 2);
293+
await page.waitForTimeout(400);
294+
const after = await page.locator(".word-popup").boundingBox();
290295
const vh = await page.evaluate(() => window.innerHeight);
291-
expect(box.y + box.height).toBeLessThanOrEqual(vh + 1);
296+
expect(after.y + after.height).toBeLessThanOrEqual(vh + 1);
292297
});

wiktionary_pron/macronizer.html

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,19 +1345,22 @@ <h2 class="result-heading">Macronized</h2>
13451345
if (!details) return;
13461346
details.addEventListener('toggle', () => {
13471347
if (popupEl.classList.contains('sheet')) return;
1348-
// Reposition for the new height — expanding may move the popup up, away
1349-
// from the cursor, which fires mouseleave and (on a small popup) hides it.
13501348
// Pin while the analysis is open so reading it can't be dismissed by the
13511349
// cursor slipping off; unpin on collapse so hover-out closes it again.
13521350
popupPinned = details.open;
1353-
// Defer to the next frame: the <details> height change hasn't been laid
1354-
// out yet when 'toggle' fires, so offsetHeight would read the OLD (collapsed)
1355-
// size and we'd place the popup for the wrong height — visible as a jump
1356-
// (popup flashes at the old spot, then repositions) on some words.
1351+
// Do NOT reposition on expand: growing the popup upward (when it floats
1352+
// above the word) would drag the just-clicked <summary> out from under the
1353+
// cursor — the visible "jumps up" complaint. Instead keep the popup's top
1354+
// pinned where it is and clamp the expanded content into the space below
1355+
// with an internal scrollbar. Only the collapsed case re-runs the normal
1356+
// placement (so the popup doesn't keep a stale tall maxHeight).
13571357
requestAnimationFrame(() => {
1358-
if (popupEl.style.display === 'block' && !popupEl.classList.contains('sheet')) {
1359-
positionPopup(popupSpan);
1360-
}
1358+
if (popupEl.style.display !== 'block' || popupEl.classList.contains('sheet')) return;
1359+
if (!details.open) { positionPopup(popupSpan); return; }
1360+
const MARGIN = 8;
1361+
const roomBelow = window.innerHeight - popupEl.offsetTop - MARGIN;
1362+
popupEl.style.maxHeight = Math.max(120, roomBelow) + 'px';
1363+
popupEl.style.overflowY = 'auto';
13611364
});
13621365
});
13631366
}

0 commit comments

Comments
 (0)