From 8da797751bc4b35871e68eafc0d08ff568c378b2 Mon Sep 17 00:00:00 2001 From: mARTin-B78 Date: Wed, 30 Sep 2026 12:48:27 +0200 Subject: [PATCH] fix(magazine): draw and edit panels are mounted on body so the sticky bars cannot cover them Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 1 + static/js/magazine.js | 40 +++++++++++---- static/style.css | 4 +- tests/magazine.spec.js | 113 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17a020f..1aa6997 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ยท versioned wi ## [Unreleased] ### Fixed +- **The Draw region panel (and the "save new position" panel) could sit underneath the sticky Back / Approve bar and the status bar, cutting off Cancel / Add region.** The panel lived inside the page frame, which is in a sticky container; a sticky element is its own stacking context, so the panel's z-index only ranked it against the frame's content and the bars painted over it. Panels are now mounted on the page body (z-index 300: above the sticky bars, below dialogs), placed and followed as before, and cleaned up when the review redraws or is left. The region-type list opens upwards when there is no room below. The existing test only ever drew above the bar, so it could not see this; the new one draws where the panel lands over the bar and checks, with `elementFromPoint`, that the buttons are what a click would actually hit (verified to fail with the old layering). - **Turning a page no longer fails when the OCR engine cannot read the page in its new orientation.** On a real page (c't p. 156) the vision engine read the sideways image well (word-F1 0.878) but returned an empty answer for the turned page, so the turn was rolled back with "The OCR engine returned nothing usable". A turn now reads through a ladder: the turned page with the vision engines, then the page as scanned with its boxes carried into the turned page, then tesseract; the first plausible reading wins (on that page: the second, 13 regions, 168 words, F1 0.878), a picture page with no text can still be turned, and only if nothing can read it is the page put back. ### Added diff --git a/static/js/magazine.js b/static/js/magazine.js index 7664f57..e37e6a2 100644 --- a/static/js/magazine.js +++ b/static/js/magazine.js @@ -1927,6 +1927,7 @@ function renderReview() { const host = $('mag-review'); const a = state.article; if (!host || !a) return; + magClearFloatingPanels(); // A half-built click order belongs to the page it was started on. if (state.orderMode && state.orderPage !== state.reviewPage) { state.orderMode = false; state.orderSeq = []; } if (state.selectPage !== state.reviewPage) { state.selected = new Set(); state.selectPage = state.reviewPage; } @@ -2833,10 +2834,7 @@ function magReviewOpenEditPanel(blockId, box, orig) { '' : '') + ''; - wrap.appendChild(panel); - const top = parseFloat(box.style.top) || 0, left = parseFloat(box.style.left) || 0, height = parseFloat(box.style.height) || 0; - panel.style.top = Math.min(80, top + height + 1) + '%'; - panel.style.left = Math.min(55, left) + '%'; + magMountFloatingPanel(panel, box); } async function magReviewSubmitEdit(reocr) { @@ -3327,10 +3325,23 @@ function magReviewOpenDrawPanel(box) { '' + ''; - wrap.appendChild(panel); + magMountFloatingPanel(panel, box); + if ($('mag-draw-kind-picker')) _magSetupDrawKindPicker(drawKindOptions); +} + +// Panels mounted on outlive the review list that opened them, so whatever redraws or leaves the review clears them. +function magClearFloatingPanels() { + document.querySelectorAll('.mag-draw-panel').forEach((n) => n.remove()); +} + +// The panels that belong to a rectangle on the page (draw, edit) are mounted on , not inside the page frame. +// The frame sits in a sticky container, and a sticky element is its own stacking context: a panel inside it is ranked +// only against the frame's other content, so the sticky Back/Approve bar and the status bar painted over it and cut off +// its buttons however high its z-index was. On the panel is ranked against the page, above those bars. +// It is placed in the window, follows the rectangle while the page scrolls or resizes, and goes away with its panel. +function magMountFloatingPanel(panel, box) { + document.body.appendChild(panel); magPlaceDrawPanel(panel, box); - // The panel is placed in the window, not in the page image, so it follows the rectangle when the - // page scrolls or the window changes size, and goes away with its panel. const follow = () => { if (!panel.isConnected) { window.removeEventListener('scroll', follow, true); @@ -3341,7 +3352,6 @@ function magReviewOpenDrawPanel(box) { }; window.addEventListener('scroll', follow, true); window.addEventListener('resize', follow); - if ($('mag-draw-kind-picker')) _magSetupDrawKindPicker(drawKindOptions); } // Below the drawn rectangle when it fits, above it when not, and always fully inside the window @@ -3400,17 +3410,26 @@ function _magSetupDrawKindPicker(options) { _magDrawKindOnPick(value); }; + // Opens below the field, or above it when the window has more room there (a rectangle drawn at the foot of the page). + const place = () => { + const r = search.getBoundingClientRect(); + const below = window.innerHeight - r.bottom - 12, above = r.top - 12; + const up = below < 200 && above > below; + list.classList.toggle('opens-up', up); + list.style.maxHeight = Math.max(110, Math.min(260, up ? above : below)) + 'px'; + }; const open = () => { renderList(search.value === (options.find(([v]) => v === hidden.value) || [])[1] ? '' : search.value); list.hidden = false; search.setAttribute('aria-expanded', 'true'); + place(); }; // Start on the default (Paragraph) with its label showing, matching the // hidden input's own default value set in the HTML above. pick(hidden.value, (options.find(([v]) => v === hidden.value) || [])[1] || ''); search.addEventListener('focus', () => { search.value = ''; open(); }); - search.addEventListener('input', () => { list.hidden = false; search.setAttribute('aria-expanded', 'true'); renderList(search.value); }); + search.addEventListener('input', () => { list.hidden = false; search.setAttribute('aria-expanded', 'true'); renderList(search.value); place(); }); search.addEventListener('keydown', (e) => { if (e.key === 'Escape') { list.hidden = true; search.setAttribute('aria-expanded', 'false'); search.blur(); } else if (e.key === 'Enter') { @@ -5361,6 +5380,7 @@ async function magPlayPodcast() { function magShow(view) { state.view = view; + if (view !== 'review') magClearFloatingPanels(); ['library', 'contents', 'review', 'read'].forEach((v) => { const el = $('mag-view-' + v); if (el) el.hidden = v !== view; @@ -5774,6 +5794,8 @@ root.magReviewToggleAllPage = magReviewToggleAllPage; root.magReviewClearCurrentPage = magReviewClearCurrentPage; root.magReviewRemoveAllBoxes = magReviewRemoveAllBoxes; root.magReocrPage = magReocrPage; +root.magMountFloatingPanel = magMountFloatingPanel; +root.magReviewOpenEditPanel = magReviewOpenEditPanel; root.magRotatePage = magRotatePage; root.magRefineFigures = magRefineFigures; root.magRefineAllFigures = magRefineAllFigures; diff --git a/static/style.css b/static/style.css index 61da88f..a7c01a9 100644 --- a/static/style.css +++ b/static/style.css @@ -5128,7 +5128,7 @@ input[list="lang-codes"] { .mag-draw-rect { border: 2px dashed var(--green); background: rgba(22,163,74,.14); cursor: crosshair; transition: none; z-index: 3; } #mag-draw-toggle.active { background: var(--accent); color: var(--on-accent); border-color: var(--accent); } -.mag-draw-panel { position: fixed; z-index: 60; max-width: calc(100vw - 16px); display: flex; flex-direction: column; gap: 6px; +.mag-draw-panel { position: fixed; z-index: 300; /* mounted on : above the sticky bars (30-100), below dialogs (1000+) */ max-width: calc(100vw - 16px); display: flex; flex-direction: column; gap: 6px; padding: 8px; min-width: 220px; background: var(--panel); border: 1px solid var(--border); border-radius: var(--radius); box-shadow: 0 4px 16px rgba(0,0,0,.25); } .mag-draw-panel-row { display: flex; gap: 10px; align-items: center; justify-content: space-between; } @@ -5647,3 +5647,5 @@ details.mag-report-group summary { cursor: pointer; margin-bottom: 6px; } @media (max-width: 860px) { .mag-hl-wrap { font-size: 16px; } } /* after the desktop rule above, so it wins */ .mag-turn-group { display: inline-flex; gap: 4px; padding: 2px; border: 1px dashed var(--border); border-radius: var(--radius); } +.mag-type-picker-list.opens-up { top: auto; bottom: 100%; margin: 0 0 2px; } +.mag-type-picker-list { overflow-y: auto; } diff --git a/tests/magazine.spec.js b/tests/magazine.spec.js index 4764439..56ec5f6 100644 --- a/tests/magazine.spec.js +++ b/tests/magazine.spec.js @@ -2076,3 +2076,116 @@ test.describe('Turning a page scanned sideways', () => { await expect(page.getByRole('button', { name: /Turn right/ })).toBeEnabled(); }); }); + +test.describe('Draw region panel layering', () => { + async function openDrawing(page, height = 640) { + await page.setViewportSize({ width: 1300, height }); + await mockBackend(page); + await page.goto('/', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#app-skeleton', { state: 'detached' }); + await navTo(page, 's-magazine'); + await page.locator('#mag-library-list .mag-card').first().click(); + await page.getByRole('button', { name: /^Review/ }).click(); + await page.getByRole('button', { name: 'Draw region', exact: true }).click(); + await scrollPageImageBelowToolbar(page, 300); + } + // Drag a rectangle whose bottom edge is at yEnd (window coordinates) over the page image. + async function drawEndingAt(page, yEnd, height = 40) { + const wrap = await page.locator('#mag-review-page-wrap').boundingBox(); + await page.mouse.move(wrap.x + 40, yEnd - height); + await page.mouse.down(); + await page.mouse.move(wrap.x + 200, yEnd, { steps: 4 }); + await page.mouse.up(); + } + // What the user would actually hit at the centre of each button: it must be that button, not a bar laid over it. + const topmostIsOwn = (page, name) => page.locator('.mag-draw-panel').getByRole('button', { name, exact: true }).evaluate((btn) => { + const r = btn.getBoundingClientRect(); + const hit = document.elementFromPoint(r.x + r.width / 2, r.y + r.height / 2); + return !!hit && (hit === btn || btn.contains(hit)); + }); + async function geometry(page) { + const vp = page.viewportSize(); + const wrap = await page.locator('#mag-review-page-wrap').boundingBox(); + const footTop = await page.locator('.mag-review-foot').evaluate(el => el.getBoundingClientRect().top); + await drawEndingAt(page, wrap.y + 80); // a first rectangle, only to measure the panel + const height = (await page.locator('.mag-draw-panel').boundingBox()).height; + await page.getByRole('button', { name: 'Cancel', exact: true }).click(); + return { vp, wrap, footTop, height }; + } + + test('a panel opened where the sticky bottom bar lies is on top of it, so every button can be clicked', async ({ page }) => { + await openDrawing(page); + const { vp, wrap, footTop, height } = await geometry(page); + // A rectangle that leaves room for the panel below it *only just*: it fits in the window, but reaches into the + // band the sticky Back / Approve bar occupies -- the case that hid Cancel / Add region. + const yEnd = Math.round(Math.min(vp.height - 24 - height, footTop - 8)); + expect(yEnd - 40).toBeGreaterThan(wrap.y); // the rectangle is on the page image + expect(yEnd + 6 + height).toBeGreaterThan(footTop); // and the panel below it does overlap the bar + await drawEndingAt(page, yEnd); + const panel = page.locator('.mag-draw-panel'); + await expect(panel).toBeVisible(); + const box = await panel.boundingBox(); + expect(box.y + box.height).toBeLessThanOrEqual(vp.height); // inside the window... + expect(box.y + box.height).toBeGreaterThan(footTop); // ...and over the bar's band + expect(await topmostIsOwn(page, 'Add region')).toBe(true); + expect(await topmostIsOwn(page, 'Cancel')).toBe(true); + await page.getByRole('button', { name: 'Cancel', exact: true }).click(); + await expect(page.locator('.mag-draw-panel')).toHaveCount(0); + }); + + test('the panel is mounted on the page body, outside the sticky stacking contexts', async ({ page }) => { + await openDrawing(page); + const wrap = await page.locator('#mag-review-page-wrap').boundingBox(); + await drawEndingAt(page, wrap.y + 80); + const info = await page.locator('.mag-draw-panel').evaluate((el) => ({ parent: el.parentElement.tagName, z: getComputedStyle(el).zIndex })); + expect(info.parent).toBe('BODY'); + expect(Number(info.z)).toBeGreaterThan(100); // above the sticky bars (30-100) + expect(Number(info.z)).toBeLessThan(1000); // below dialogs + }); + + test('the region type list stays inside the window and its options can be clicked', async ({ page }) => { + await openDrawing(page); + const { vp, footTop, height } = await geometry(page); + await drawEndingAt(page, Math.round(Math.min(vp.height - 24 - height, footTop - 8))); + await page.locator('#mag-draw-kind-search').focus(); + const list = page.locator('#mag-draw-kind-list'); + await expect(list).toBeVisible(); + const r = await list.boundingBox(); + expect(r.y).toBeGreaterThanOrEqual(0); + expect(r.y + r.height).toBeLessThanOrEqual(vp.height); + const reachable = await list.locator('.mag-type-picker-option').first().evaluate((el) => { + const b = el.getBoundingClientRect(); + const hit = document.elementFromPoint(b.x + b.width / 2, b.y + b.height / 2); + return !!hit && el.contains(hit); + }); + expect(reachable).toBe(true); + await list.locator('.mag-type-picker-option').nth(2).click(); // a real click on an option + await expect(list).toBeHidden(); + }); + + test('panels are removed when the review is left', async ({ page }) => { + await openDrawing(page); + const wrap = await page.locator('#mag-review-page-wrap').boundingBox(); + await drawEndingAt(page, wrap.y + 80); + await expect(page.locator('.mag-draw-panel')).toHaveCount(1); + await page.evaluate(() => window.magBackToContents()); + await expect(page.locator('.mag-draw-panel')).toHaveCount(0); + }); + + test('the save-new-position panel is mounted and placed the same way', async ({ page }) => { + await openDrawing(page); + await page.evaluate(() => { + const box = document.querySelector('#mag-review-page-wrap .mag-review-box'); + window.magReviewOpenEditPanel(box.dataset.block, box, { origLeft: 0, origTop: 0, origW: 10, origH: 10 }); + }); + const panel = page.locator('.mag-edit-panel'); + await expect(panel).toBeVisible(); + expect(await panel.evaluate(el => el.parentElement.tagName)).toBe('BODY'); + const box = await panel.boundingBox(); + const vp = page.viewportSize(); + expect(box.x).toBeGreaterThanOrEqual(0); + expect(box.y).toBeGreaterThanOrEqual(0); + expect(box.y + box.height).toBeLessThanOrEqual(vp.height); + expect(box.x + box.width).toBeLessThanOrEqual(vp.width); + }); +});