fix(magazine): draw and edit panels are mounted on body so the sticky bars cannot cover them
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
130fd273c6
commit
8da797751b
@ -7,6 +7,7 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
|
|||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
### Fixed
|
### 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.
|
- **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
|
### Added
|
||||||
|
|||||||
@ -1927,6 +1927,7 @@ function renderReview() {
|
|||||||
const host = $('mag-review');
|
const host = $('mag-review');
|
||||||
const a = state.article;
|
const a = state.article;
|
||||||
if (!host || !a) return;
|
if (!host || !a) return;
|
||||||
|
magClearFloatingPanels();
|
||||||
// A half-built click order belongs to the page it was started on.
|
// 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.orderMode && state.orderPage !== state.reviewPage) { state.orderMode = false; state.orderSeq = []; }
|
||||||
if (state.selectPage !== state.reviewPage) { state.selected = new Set(); state.selectPage = state.reviewPage; }
|
if (state.selectPage !== state.reviewPage) { state.selected = new Set(); state.selectPage = state.reviewPage; }
|
||||||
@ -2833,10 +2834,7 @@ function magReviewOpenEditPanel(blockId, box, orig) {
|
|||||||
'<button type="button" class="btn-sm primary" onclick="magReviewSubmitEdit(true)"><span class="mdi mdi-text-recognition"></span> Save & re-OCR</button>'
|
'<button type="button" class="btn-sm primary" onclick="magReviewSubmitEdit(true)"><span class="mdi mdi-text-recognition"></span> Save & re-OCR</button>'
|
||||||
: '<button type="button" class="btn-sm primary" onclick="magReviewSubmitEdit(false)">Save</button>') +
|
: '<button type="button" class="btn-sm primary" onclick="magReviewSubmitEdit(false)">Save</button>') +
|
||||||
'</div>';
|
'</div>';
|
||||||
wrap.appendChild(panel);
|
magMountFloatingPanel(panel, box);
|
||||||
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) + '%';
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async function magReviewSubmitEdit(reocr) {
|
async function magReviewSubmitEdit(reocr) {
|
||||||
@ -3327,10 +3325,23 @@ function magReviewOpenDrawPanel(box) {
|
|||||||
'<button type="button" class="btn-sm primary" onclick="magReviewSubmitDraw()">' +
|
'<button type="button" class="btn-sm primary" onclick="magReviewSubmitDraw()">' +
|
||||||
'<span class="mdi mdi-plus" aria-hidden="true"></span> Add region</button>' +
|
'<span class="mdi mdi-plus" aria-hidden="true"></span> Add region</button>' +
|
||||||
'</div>';
|
'</div>';
|
||||||
wrap.appendChild(panel);
|
magMountFloatingPanel(panel, box);
|
||||||
|
if ($('mag-draw-kind-picker')) _magSetupDrawKindPicker(drawKindOptions);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Panels mounted on <body> 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 <body>, 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 <body> 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);
|
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 = () => {
|
const follow = () => {
|
||||||
if (!panel.isConnected) {
|
if (!panel.isConnected) {
|
||||||
window.removeEventListener('scroll', follow, true);
|
window.removeEventListener('scroll', follow, true);
|
||||||
@ -3341,7 +3352,6 @@ function magReviewOpenDrawPanel(box) {
|
|||||||
};
|
};
|
||||||
window.addEventListener('scroll', follow, true);
|
window.addEventListener('scroll', follow, true);
|
||||||
window.addEventListener('resize', follow);
|
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
|
// 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);
|
_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 = () => {
|
const open = () => {
|
||||||
renderList(search.value === (options.find(([v]) => v === hidden.value) || [])[1] ? '' : search.value);
|
renderList(search.value === (options.find(([v]) => v === hidden.value) || [])[1] ? '' : search.value);
|
||||||
list.hidden = false;
|
list.hidden = false;
|
||||||
search.setAttribute('aria-expanded', 'true');
|
search.setAttribute('aria-expanded', 'true');
|
||||||
|
place();
|
||||||
};
|
};
|
||||||
|
|
||||||
// Start on the default (Paragraph) with its label showing, matching the
|
// Start on the default (Paragraph) with its label showing, matching the
|
||||||
// hidden input's own default value set in the HTML above.
|
// hidden input's own default value set in the HTML above.
|
||||||
pick(hidden.value, (options.find(([v]) => v === hidden.value) || [])[1] || '');
|
pick(hidden.value, (options.find(([v]) => v === hidden.value) || [])[1] || '');
|
||||||
search.addEventListener('focus', () => { search.value = ''; open(); });
|
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) => {
|
search.addEventListener('keydown', (e) => {
|
||||||
if (e.key === 'Escape') { list.hidden = true; search.setAttribute('aria-expanded', 'false'); search.blur(); }
|
if (e.key === 'Escape') { list.hidden = true; search.setAttribute('aria-expanded', 'false'); search.blur(); }
|
||||||
else if (e.key === 'Enter') {
|
else if (e.key === 'Enter') {
|
||||||
@ -5361,6 +5380,7 @@ async function magPlayPodcast() {
|
|||||||
|
|
||||||
function magShow(view) {
|
function magShow(view) {
|
||||||
state.view = view;
|
state.view = view;
|
||||||
|
if (view !== 'review') magClearFloatingPanels();
|
||||||
['library', 'contents', 'review', 'read'].forEach((v) => {
|
['library', 'contents', 'review', 'read'].forEach((v) => {
|
||||||
const el = $('mag-view-' + v);
|
const el = $('mag-view-' + v);
|
||||||
if (el) el.hidden = v !== view;
|
if (el) el.hidden = v !== view;
|
||||||
@ -5774,6 +5794,8 @@ root.magReviewToggleAllPage = magReviewToggleAllPage;
|
|||||||
root.magReviewClearCurrentPage = magReviewClearCurrentPage;
|
root.magReviewClearCurrentPage = magReviewClearCurrentPage;
|
||||||
root.magReviewRemoveAllBoxes = magReviewRemoveAllBoxes;
|
root.magReviewRemoveAllBoxes = magReviewRemoveAllBoxes;
|
||||||
root.magReocrPage = magReocrPage;
|
root.magReocrPage = magReocrPage;
|
||||||
|
root.magMountFloatingPanel = magMountFloatingPanel;
|
||||||
|
root.magReviewOpenEditPanel = magReviewOpenEditPanel;
|
||||||
root.magRotatePage = magRotatePage;
|
root.magRotatePage = magRotatePage;
|
||||||
root.magRefineFigures = magRefineFigures;
|
root.magRefineFigures = magRefineFigures;
|
||||||
root.magRefineAllFigures = magRefineAllFigures;
|
root.magRefineAllFigures = magRefineAllFigures;
|
||||||
|
|||||||
@ -5128,7 +5128,7 @@ input[list="lang-codes"] {
|
|||||||
.mag-draw-rect { border: 2px dashed var(--green); background: rgba(22,163,74,.14); cursor: crosshair;
|
.mag-draw-rect { border: 2px dashed var(--green); background: rgba(22,163,74,.14); cursor: crosshair;
|
||||||
transition: none; z-index: 3; }
|
transition: none; z-index: 3; }
|
||||||
#mag-draw-toggle.active { background: var(--accent); color: var(--on-accent); border-color: var(--accent); }
|
#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 <body>: 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);
|
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); }
|
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; }
|
.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 */
|
@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-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; }
|
||||||
|
|||||||
@ -2076,3 +2076,116 @@ test.describe('Turning a page scanned sideways', () => {
|
|||||||
await expect(page.getByRole('button', { name: /Turn right/ })).toBeEnabled();
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user