From 99f2f2260e6277af1c621bf6e6797259a4eb4a47 Mon Sep 17 00:00:00 2001 From: mARTin-B78 Date: Mon, 21 Sep 2026 12:11:14 +0200 Subject: [PATCH] feat(magazine): edit, split, merge and re-OCR a region from its row, with keyboard control Split at the cursor and merge-with-next keep article membership right; typed letters in the editor never fire shortcuts; focus returns to the same row after a redraw. v1.38.0. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 ++ Manual.md | 12 ++- TODO.md | 2 +- VERSION | 2 +- architecture/magazine-ingestion.md | 11 +++ core/magazine.py | 81 +++++++++++++++++++ routes/magazine.py | 54 +++++++++++++ static/index.html | 12 +-- static/js/magazine.js | 124 ++++++++++++++++++++++++++++- static/style.css | 11 +++ tests/magazine.spec.js | 69 ++++++++++++++++ tests/test_magazine_routes.py | 91 +++++++++++++++++++++ 12 files changed, 463 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f222c36..6a91faa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi ## [Unreleased] +## [1.38.0] — 2026-09-21 + +### Added +- **Magazine review: quick corrections per region.** Edit a region's text in place and save it, split a region in two at the cursor, read one region again with OCR, or merge it with the next region (hyphenated words at the join are closed up) — each from its row, without leaving the page. The list can also be worked from the keyboard: ↑/↓ move, Space tick, Enter edit, R read again, M merge, Delete delete; the focused row keeps focus after a change. + ## [1.37.0] — 2026-09-21 ### Added diff --git a/Manual.md b/Manual.md index ef6a9a0..1ba41f4 100644 --- a/Manual.md +++ b/Manual.md @@ -168,7 +168,17 @@ turned into a podcast discussion between two or three voices you pick. (`layout.backup-.json` in the issue's folder). New imports and *OCR again* do this automatically. Longer text inside a picture is left for you to judge: it can be real text caught in a picture frame that is - too big. Use + too big. Each text row also has three quick actions: **Edit text** (the + pencil) opens the region's text in place — fix OCR mistakes and *Save + text*, or put the cursor where one paragraph ends and press **Split + here** to make two regions (the second is read right after the first); + **Read again** re-runs OCR on just that region; and **Merge with the next + region** joins a paragraph the OCR cut in two (a word broken by a hyphen + at the line end is closed up again). Everything works from the keyboard + too: ↑/↓ move between regions, Space ticks or unticks, Enter edits the + text, R reads again, M merges with the next region and Delete deletes + (after confirming); letters typed in the text editor are never taken as + shortcuts. Use **"Two-page spread"** to see this page next to its facing page (e.g. 22 and 23), each with its own OCR-results list directly beneath it — useful for lining up a figure or a jump that crosses the gutter. Only one page diff --git a/TODO.md b/TODO.md index 10c7a96..53ec566 100644 --- a/TODO.md +++ b/TODO.md @@ -80,7 +80,7 @@ not part of this order. read, or type a number into each one. The order drives playback and the OCR-results list. Today the order is whatever the layout analysis produced, and a page with two text columns comes out interleaved, so a listener hears half a sentence from the wrong column. -- [ ] **L — Magazine: overhaul region boxes and how they are handled in OCR and approval.** (Done 2026-09-21: type picker and delete per row; Clean up boxes — text in pictures, OCR noise, duplicates, run-on author lines. Still open: keyboard navigation, merge/split, per-row re-OCR; the 333 overlapping boxes with different text need a human.) +- [ ] **L — Magazine: overhaul region boxes and how they are handled in OCR and approval.** (Done 2026-09-21: type picker and delete per row; Clean up boxes — text in pictures, OCR noise, duplicates, run-on author lines. Done too: keyboard navigation, merge with next, split at cursor, per-row re-OCR and text editing. Still open: snapping text boxes to the ink of their text; the 333 overlapping boxes with different text on the measured issue need a human.) The boxes the layout analysis produces need a pass of their own, on top of the ordering work above. Seen on real pages: a byline box that runs on into the first body line ("Von Hartmut Gieselmann Ein guter Butler kennt die Vorlieben seines"), so author and paragraph both come diff --git a/VERSION b/VERSION index e4dce85..4afb1d2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.37.0 \ No newline at end of file +1.38.0 \ No newline at end of file diff --git a/architecture/magazine-ingestion.md b/architecture/magazine-ingestion.md index d327e92..f535a91 100644 --- a/architecture/magazine-ingestion.md +++ b/architecture/magazine-ingestion.md @@ -328,6 +328,17 @@ new paragraph is inserted after the author line in every article that holds it; .../boxes/clean-up`, writes `layout.backup-.json` first). Idempotent. `_reorder_page` now writes only position/column back, so fields the Block model does not carry (e.g. `original_text`) survive. +## Quick corrections from the review list + +`PATCH .../regions/{id}` also takes `text` (trusted verbatim, like a hand-drawn region's typed text). +`POST .../regions/{id}/merge-next` joins a text region with the next one in reading order (first keeps id, +type and place; hyphenated line-end words are closed up; bbox is the union) and drops the folded region +from articles. `POST .../regions/{id}/split {at}` splits at a character position; the box is cut +horizontally in proportion to the text on each side, the second part gets a new id, is read directly +after the first and joins the same articles. Keyboard: ↑/↓ (j/k), Space, Enter (edit), R, M, Delete; +ignored while typing in the editor, a select or a position box; focus returns to the same row after the +list is redrawn. + ## Region types from the review list Every region in the review list has a type picker and a delete button. `PATCH .../regions/{id}` takes diff --git a/core/magazine.py b/core/magazine.py index 9756b9a..99bc3de 100644 --- a/core/magazine.py +++ b/core/magazine.py @@ -2959,6 +2959,87 @@ def change_region_type(doc_dir: Path, layout: dict[str, Any], page_no: int, regi return _reorder_page(layout, page_no), new_id +def _page_text_blocks_in_order(layout: dict[str, Any], page_no: int) -> list[dict]: + return sorted((b for b in layout.get("blocks", []) if b["page"] == page_no and b["type"] not in _VISUAL_TYPES), + key=lambda b: b.get("reading_order", 0)) + + +def set_region_text(layout: dict[str, Any], page_no: int, region_id: str, text: str) -> dict[str, Any]: + """Replace a text region's OCR text with what the reader typed (trusted verbatim).""" + text = str(text or "").strip() + if not text: + raise ValueError("Text must not be empty -- delete the region instead") + if len(text) > 20000: + raise ValueError("Text is too long") + if not any(b["id"] == region_id and b["page"] == page_no for b in layout.get("blocks", [])): + raise ValueError("Region not found on this page") + layout = dict(layout) + layout["blocks"] = [dict(b, text=text, confidence=1.0) if b["id"] == region_id else b + for b in layout.get("blocks", [])] + return layout + + +def merge_with_next(layout: dict[str, Any], page_no: int, region_id: str) -> tuple[dict[str, Any], str]: + """Join a text region with the one read after it (a paragraph the OCR cut in two). + + The first region keeps its id, type and place; the text is joined (a word split by a hyphen at + the line end is closed up again); the box grows to cover both. Returns the layout and the id of + the region that was folded in, so the caller can drop it from articles. + """ + ordered = _page_text_blocks_in_order(layout, page_no) + ids = [b["id"] for b in ordered] + if region_id not in ids: + raise ValueError("Region not found on this page") + i = ids.index(region_id) + if i + 1 >= len(ordered): + raise ValueError("This is the last region on the page") + first, second = ordered[i], ordered[i + 1] + a, b = " ".join(first["text"].split()), " ".join(second["text"].split()) + joined = a[:-1] + b if re.search(r"[a-zäöüß]-$", a) and b[:1].islower() else f"{a} {b}" + x1, y1 = min(first["bbox"][0], second["bbox"][0]), min(first["bbox"][1], second["bbox"][1]) + x2 = max(first["bbox"][0] + first["bbox"][2], second["bbox"][0] + second["bbox"][2]) + y2 = max(first["bbox"][1] + first["bbox"][3], second["bbox"][1] + second["bbox"][3]) + layout = dict(layout) + layout["blocks"] = [dict(x, text=joined, bbox=[x1, y1, x2 - x1, y2 - y1]) if x["id"] == first["id"] else x + for x in layout.get("blocks", []) if x["id"] != second["id"]] + return layout, second["id"] + + +def split_region(layout: dict[str, Any], page_no: int, region_id: str, at: int) -> tuple[dict[str, Any], str]: + """Split a text region at a character position into two (two paragraphs the OCR fused). + + The box is cut horizontally in proportion to the text on each side, the second part gets a new + id and is read directly after the first. Returns the layout and the new region's id. + """ + block = next((b for b in layout.get("blocks", []) if b["id"] == region_id and b["page"] == page_no), None) + if block is None: + raise ValueError("Region not found on this page") + text = block["text"] + head, tail = text[:at].strip(), text[at:].strip() + if not head or not tail: + raise ValueError("Split inside the text, not at its start or end") + taken = {b["id"] for b in layout.get("blocks", [])} + n = 0 + while f"t{page_no:04d}-{n:03d}" in taken: + n += 1 + new_id = f"t{page_no:04d}-{n:03d}" + x, y, w, h = block["bbox"] + share = len(head) / max(1, len(head) + len(tail)) + first = dict(block, text=head, bbox=[x, y, w, max(1.0, h * share)]) + second = {**block, "id": new_id, "text": tail, "bbox": [x, y + h * share, w, max(1.0, h * (1 - share))], + "reading_order": block.get("reading_order", 0) + 0.5} + second.pop("original_text", None) + layout = dict(layout) + blocks = [] + for b in layout.get("blocks", []): + blocks.extend([first, second] if b["id"] == region_id else [b]) + # Renumber the page so the new part sits directly after the first, whatever else is locked. + page = sorted((b for b in blocks if b["page"] == page_no), key=lambda b: b.get("reading_order", 0)) + position = {b["id"]: i for i, b in enumerate(page)} + layout["blocks"] = [dict(b, reading_order=position[b["id"]]) if b["page"] == page_no else b for b in blocks] + return layout, new_id + + def set_page_order(layout: dict[str, Any], page_no: int, order: Sequence[str]) -> dict[str, Any]: """Set a page's reading order by hand and lock it, so nothing automatic reshuffles it again. diff --git a/routes/magazine.py b/routes/magazine.py index 8607827..eebcbe2 100644 --- a/routes/magazine.py +++ b/routes/magazine.py @@ -754,6 +754,54 @@ def _add_split_paragraphs_to_articles(articles: list[dict], report: dict) -> boo return changed +@router.post("/api/magazine/{doc_id}/pages/{page}/regions/{block_id}/merge-next") +async def magazine_merge_region(doc_id: str, page: int, block_id: str): + """Join a text region with the one read after it (a paragraph the OCR cut in two).""" + d, _ = _require_magazine(doc_id) + layout = mag.load_layout(d) + if not layout: + raise HTTPException(404, "No layout yet -- run ingestion first") + try: + layout, removed = mag.merge_with_next(layout, page, block_id) + except ValueError as exc: + raise HTTPException(400, str(exc)) + mag.save_layout(d, layout) + articles = mag.load_articles(d) + changed = False + for article in articles: + if removed in (article.get("blocks") or []): + article["blocks"] = [b for b in article["blocks"] if b != removed] + if block_id not in article["blocks"]: + article["blocks"].append(block_id) + changed = True + if changed: + mag.save_articles(d, articles) + return {"ok": True, "id": block_id, "removed": removed} + + +@router.post("/api/magazine/{doc_id}/pages/{page}/regions/{block_id}/split") +async def magazine_split_region(doc_id: str, page: int, block_id: str, request: Request): + """Split a text region at a character position into two (paragraphs the OCR fused). + Body: {at: int}. The second part is read directly after the first, in the same articles.""" + d, _ = _require_magazine(doc_id) + layout = mag.load_layout(d) + if not layout: + raise HTTPException(404, "No layout yet -- run ingestion first") + body = await request.json() + at = body.get("at") if isinstance(body, dict) else None + if not isinstance(at, int) or isinstance(at, bool) or at < 1: + raise HTTPException(400, "at must be a positive character position") + try: + layout, new_id = mag.split_region(layout, page, block_id, at) + except ValueError as exc: + raise HTTPException(400, str(exc)) + mag.save_layout(d, layout) + articles = mag.load_articles(d) + if _add_split_paragraphs_to_articles(articles, {"byline_split": [{"id": block_id, "new_id": new_id}]}): + mag.save_articles(d, articles) + return {"ok": True, "id": block_id, "new_id": new_id} + + @router.post("/api/magazine/{doc_id}/boxes/clean-up") async def magazine_clean_up_boxes(doc_id: str): """Fix measured box problems across the issue: text inside a picture, OCR noise, boxes that @@ -923,6 +971,9 @@ async def magazine_update_region(doc_id: str, page: int, block_id: str, request: new_type = body.get("type") if new_type is not None and new_type not in mag.REGION_TYPES: raise HTTPException(400, f"type must be one of {', '.join(mag.REGION_TYPES)}") + new_text = body.get("text") + if new_text is not None and not isinstance(new_text, str): + raise HTTPException(400, "text must be a string") settings = _load_settings() vision_ocr_url = str(settings.get("vision_ocr_url") or "").strip() or None @@ -938,6 +989,9 @@ async def magazine_update_region(doc_id: str, page: int, block_id: str, request: [float(v) for v in bbox] if bbox is not None else None, reocr, vision_ocr_url, vision_ocr_model, vision_ocr_fallback_url, vision_ocr_fallback_model, ) + if new_text is not None: + layout = mag.set_region_text(layout, page, block_id, new_text) + text = new_text.strip() if new_type is not None: layout, region_id = await asyncio.to_thread( mag.change_region_type, d, layout, page, block_id, new_type, diff --git a/static/index.html b/static/index.html index d22f96a..3845709 100644 --- a/static/index.html +++ b/static/index.html @@ -27,12 +27,12 @@ - - - - - - + + + + + + diff --git a/static/js/magazine.js b/static/js/magazine.js index 8a651cf..f0af3ff 100644 --- a/static/js/magazine.js +++ b/static/js/magazine.js @@ -805,7 +805,9 @@ function magReviewPageColumn(page, wrapId, interactive) { pageHtml = '

No page to show.

'; } - let listHtml = '
'; + let listHtml = '
' + + (interactive ? '

Keys: ↑/↓ move between regions · Space tick · Enter edit text · R read again · M merge with the next region · Delete delete

' : ''); regions.forEach((r, i) => { const isImage = r.kind === 'image'; const owner = magBlockOwner(r.id); @@ -839,7 +841,8 @@ function magReviewPageColumn(page, wrapId, interactive) { '' + (owner ? 'Article ' + owner.index + ': ' + esc(owner.title) + ' ' : '') + (b.continues_from_previous_page ? 'Continued from previous page ' : '') + - esc(b.text.slice(0, 220)) + (b.text.length > 220 ? '…' : '') + ''; + esc(b.text.slice(0, 220)) + (b.text.length > 220 ? '…' : '') + '' + + (interactive ? magRowActions(b.id, i + 1, i + 1 < regions.length) : '') + ''; }); if (!regions.length) listHtml += '

Nothing detected on this page.

'; listHtml += '
'; @@ -1512,6 +1515,118 @@ async function magFixReadingOrder() { renderReview(); } +// ── Quick corrections from the review list ──────────────────────────────── +// Edit a region's text in place (and split it at the cursor), read it again, merge it with the next +// region, and move between regions with the keyboard -- without leaving the page. + +function magRowActions(id, number, hasNext) { + const btn = (action, icon, label) => ''; + return '' + + btn('magRowEdit', 'mdi-pencil-outline', 'Edit text') + + btn('magRowReocr', 'mdi-text-recognition', 'Read again') + + (hasNext ? btn('magRowMerge', 'mdi-table-merge-cells', 'Merge with the next region') : '') + + ''; +} + +function magRegionUrl(id) { + return '/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(id); +} + +async function magReloadReview(message) { + // Keep a keyboard user's place: the row that had focus gets it back after the list is redrawn. + const had = document.activeElement?.closest?.('[data-block-row]')?.dataset.blockRow; + state.layout = null; + await ensureLayout(); + renderReview(); + if (had) document.querySelector('#mag-review [data-block-row="' + CSS.escape(had) + '"] input[type="checkbox"]')?.focus(); + if (message) say(message, 'ok'); +} + +function magRowEdit(id) { + const block = (state.layout?.blocks || []).find((b) => b.id === id); + const row = document.querySelector('#mag-review [data-block-row="' + CSS.escape(id) + '"]'); + const span = row?.querySelector('.mag-block-text'); + if (!block || !span || row.querySelector('.mag-row-editor')) return; + span.hidden = true; + const editor = document.createElement('span'); + editor.className = 'mag-row-editor'; + editor.innerHTML = '' + + '' + + '' + + '' + + ''; + span.after(editor); + const area = editor.querySelector('textarea'); + area.value = block.text; + ['click', 'keydown', 'keyup'].forEach((ev) => editor.addEventListener(ev, (e) => e.stopPropagation())); + area.addEventListener('keydown', (e) => { if (e.key === 'Escape') { e.preventDefault(); magReloadReview(); } }); + editor.addEventListener('click', async (e) => { + const act = e.target.closest('[data-act]')?.dataset.act; + if (!act) return; + e.preventDefault(); + try { + if (act === 'save') { + await api(magRegionUrl(id), { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ text: area.value }) }); + return magReloadReview('Text saved'); + } + if (act === 'split') { + await api(magRegionUrl(id) + '/split', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ at: area.selectionStart }) }); + return magReloadReview('Region split in two'); + } + magReloadReview(); + } catch (err) { + say('Could not change the region: ' + err.message, 'err'); + } + }); + area.focus(); +} + +async function magRowReocr(id) { + say('Reading the region again…', 'info'); + try { + await api(magRegionUrl(id), { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ reocr: true }) }); + } catch (e) { + return say('Could not read the region again: ' + e.message, 'err'); + } + magReloadReview('Region read again'); +} + +async function magRowMerge(id) { + try { + await api(magRegionUrl(id) + '/merge-next', { method: 'POST' }); + } catch (e) { + return say('Could not merge: ' + e.message, 'err'); + } + magReloadReview('Merged with the next region'); +} + +function magListFocus(event) { + const row = event.target.closest('[data-block-row]'); + if (row) magReviewHighlightBox(row.dataset.blockRow); +} + +function magListKey(event) { + const target = event.target; + if (target.closest('textarea, select, .mag-row-editor') || (target.tagName === 'INPUT' && target.type === 'number')) return; + const row = target.closest('[data-block-row]'); + if (!row) return; + const rows = [...document.querySelectorAll('#mag-review .mag-blocks [data-block-row]')]; + const at = rows.indexOf(row); + const focusRow = (r) => (r?.querySelector('input[type="checkbox"]') || r)?.focus(); + const id = row.dataset.blockRow; + const isText = row.tagName === 'LABEL'; + const key = event.key.length === 1 ? event.key.toLowerCase() : event.key; + if (key === 'ArrowDown' || key === 'j') { event.preventDefault(); focusRow(rows[at + 1]); } + else if (key === 'ArrowUp' || key === 'k') { event.preventDefault(); focusRow(rows[at - 1]); } + else if (key === 'Enter' && isText) { event.preventDefault(); magRowEdit(id); } + else if (key === 'r' && isText) { event.preventDefault(); magRowReocr(id); } + else if (key === 'm' && isText) { event.preventDefault(); magRowMerge(id); } + else if (key === 'Delete') { event.preventDefault(); magReviewDeleteRegion(id); } +} + async function magReviewDeleteRegion(blockId) { const ok = await confirmDialog('Delete this region? This removes it from the page entirely, not just from this article.', { title: 'Delete region?', okLabel: 'Delete', danger: true }); @@ -2578,6 +2693,11 @@ root.magReviewDeleteRegion = magReviewDeleteRegion; root.magReviewChangeType = magReviewChangeType; root.magFixReadingOrder = magFixReadingOrder; root.magCleanUpBoxes = magCleanUpBoxes; +root.magRowEdit = magRowEdit; +root.magRowReocr = magRowReocr; +root.magRowMerge = magRowMerge; +root.magListKey = magListKey; +root.magListFocus = magListFocus; root.magOrderToggle = magOrderToggle; root.magOrderSave = magOrderSave; root.magOrderUndo = magOrderUndo; diff --git a/static/style.css b/static/style.css index c6cbdd9..5d589bc 100644 --- a/static/style.css +++ b/static/style.css @@ -5171,3 +5171,14 @@ input[list="lang-codes"] { .mag-review-page-wrap.is-ordering .mag-review-box { cursor: pointer; } .mag-review-box.is-ordered { outline: 3px solid var(--accent, #2563eb); outline-offset: -1px; } .mag-review-box.is-ordered .mag-review-box-num { background: var(--accent, #2563eb); color: #fff; } + +/* Quick corrections in the magazine review list */ +.mag-list-keys { font-size: 12px; margin: 0 0 6px; } +.mag-row-actions { display: inline-flex; gap: 2px; grid-column: -2 / -1; justify-self: end; } +.mag-row-action { padding: 2px 6px; min-width: 28px; min-height: 28px; color: var(--subtext); } +.mag-row-action:hover, .mag-row-action:focus-visible { color: var(--text); } +.mag-row-editor { display: flex; flex-direction: column; gap: 6px; grid-column: 1 / -1; } +.mag-row-editor textarea { width: 100%; font: inherit; font-size: 13px; padding: 6px; border: 1px solid var(--border); + border-radius: var(--radius); background: var(--surface); color: var(--text); resize: vertical; } +.mag-row-editor-actions { display: flex; flex-wrap: wrap; gap: 6px; } +.mag-blocks [data-block-row]:focus-within { outline: 2px solid var(--accent, #2563eb); outline-offset: 1px; } diff --git a/tests/magazine.spec.js b/tests/magazine.spec.js index 377488d..bdedd8b 100644 --- a/tests/magazine.spec.js +++ b/tests/magazine.spec.js @@ -476,3 +476,72 @@ test('a box marked as text in a picture shows that type in its picker', async ({ await openReview(page); await expect(page.locator('[data-block-row="b3"] select.mag-block-type-select')).toHaveValue('in_picture'); }); + +test('a region text can be corrected and split at the cursor from the list', async ({ page }) => { + await mockBackend(page); + const calls = []; + await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/b3**`, route => { + calls.push({ method: route.request().method(), url: route.request().url(), body: route.request().postDataJSON() }); + return route.fulfill({ json: { ok: true, id: 'b3', new_id: 't0003-000' } }); + }); + await openReview(page); + const row = page.locator('[data-block-row="b3"]'); + await row.getByRole('button', { name: /Edit text \(region 3\)/ }).click(); + const area = row.getByRole('textbox', { name: 'Text of this region' }); + await expect(area).toHaveValue('Battery technology is entering a new phase. It will take years to mature.'); + // Typing letters that are shortcuts elsewhere must stay in the editor. + await area.press('End'); + await area.type(' mr'); + expect(calls).toEqual([]); + await row.getByRole('button', { name: 'Save text', exact: true }).click(); + await expect.poll(() => calls.length).toBe(1); + expect(calls[0]).toMatchObject({ method: 'PATCH', body: { text: 'Battery technology is entering a new phase. It will take years to mature. mr' } }); + + await row.getByRole('button', { name: /Edit text \(region 3\)/ }).click(); + const again = row.getByRole('textbox', { name: 'Text of this region' }); + await again.evaluate(el => el.setSelectionRange(43, 43)); + await row.getByRole('button', { name: 'Split here', exact: true }).click(); + await expect.poll(() => calls.length).toBe(2); + expect(calls[1].url).toContain('/regions/b3/split'); + expect(calls[1].body).toEqual({ at: 43 }); +}); + +test('merge with next and read again are one click away', async ({ page }) => { + await mockBackend(page); + const calls = []; + await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/b2**`, route => { + calls.push({ method: route.request().method(), url: route.request().url(), body: route.request().postDataJSON() }); + return route.fulfill({ json: { ok: true, id: 'b2', removed: 'b3' } }); + }); + await openReview(page); + const row = page.locator('[data-block-row="b2"]'); + await row.getByRole('button', { name: /Merge with the next region/ }).click(); + await expect.poll(() => calls.length).toBe(1); + expect(calls[0].url).toContain('/regions/b2/merge-next'); + await row.getByRole('button', { name: /Read again/ }).click(); + await expect.poll(() => calls.length).toBe(2); + expect(calls[1]).toMatchObject({ method: 'PATCH', body: { reocr: true } }); + // The last region has nothing after it to merge with. + await expect(page.locator('[data-block-row="b3"]').getByRole('button', { name: /Merge with the next region/ })).toHaveCount(0); +}); + +test('the review list can be worked from the keyboard', async ({ page }) => { + await mockBackend(page); + const calls = []; + await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/**`, route => { + calls.push({ method: route.request().method(), url: route.request().url(), body: route.request().postDataJSON() }); + return route.fulfill({ json: { ok: true } }); + }); + await openReview(page); + await page.locator('#mag-review input[data-block="b1"]').focus(); + await page.keyboard.press('ArrowDown'); + await expect(page.locator('#mag-review input[data-block="b2"]')).toBeFocused(); + await page.keyboard.press('r'); + await expect.poll(() => calls.length).toBe(1); + expect(calls[0].url).toContain('/regions/b2'); + expect(calls[0].body).toEqual({ reocr: true }); + await page.keyboard.press('ArrowDown'); + await expect(page.locator('#mag-review input[data-block="b3"]')).toBeFocused(); + await page.keyboard.press('Enter'); + await expect(page.locator('[data-block-row="b3"]').getByRole('textbox', { name: 'Text of this region' })).toBeFocused(); +}); diff --git a/tests/test_magazine_routes.py b/tests/test_magazine_routes.py index 11f83ee..3bfe7a5 100644 --- a/tests/test_magazine_routes.py +++ b/tests/test_magazine_routes.py @@ -920,3 +920,94 @@ class CleanUpBoxesTests(unittest.TestCase): self.assertEqual(saved["a"][0]["blocks"], ["by", new_id, "body"]) finally: shutil.rmtree(d, ignore_errors=True) + + +class QuickCorrectionTests(unittest.TestCase): + """Edit text, merge with the next region, split a region -- from the review list.""" + + def layout(self): + return {"pages": [{"page": 1, "width": 595.0, "height": 842.0}], "images": [], "blocks": [ + {"id": "a", "page": 1, "bbox": [50, 100, 200, 40], "type": "body", "text": "Der Anfang eines Absat-", "reading_order": 0}, + {"id": "b", "page": 1, "bbox": [50, 140, 200, 40], "type": "body", "text": "zes geht hier weiter.", "reading_order": 1}, + {"id": "c", "page": 1, "bbox": [50, 200, 200, 80], "type": "body", "text": "Erster Satz. Zweiter Absatz beginnt.", "reading_order": 2}, + ]} + + def order(self, layout): + return [b["id"] for b in sorted(layout["blocks"], key=lambda b: b["reading_order"])] + + def test_typed_text_replaces_the_ocr_text(self): + from core.magazine import set_region_text + out = set_region_text(self.layout(), 1, "c", " Corrected by hand. ") + self.assertEqual(next(b for b in out["blocks"] if b["id"] == "c")["text"], "Corrected by hand.") + with self.assertRaises(ValueError): + set_region_text(self.layout(), 1, "c", " ") + + def test_merging_closes_a_hyphenated_word_and_covers_both_boxes(self): + from core.magazine import merge_with_next + out, removed = merge_with_next(self.layout(), 1, "a") + merged = next(b for b in out["blocks"] if b["id"] == "a") + self.assertEqual(removed, "b") + self.assertEqual(merged["text"], "Der Anfang eines Absatzes geht hier weiter.") + self.assertEqual(merged["bbox"], [50, 100, 200, 80]) + self.assertNotIn("b", [x["id"] for x in out["blocks"]]) + + def test_the_last_region_has_nothing_to_merge_with(self): + from core.magazine import merge_with_next + with self.assertRaises(ValueError): + merge_with_next(self.layout(), 1, "c") + + def test_splitting_makes_a_second_region_read_right_after_the_first(self): + from core.magazine import split_region + out, new_id = split_region(self.layout(), 1, "c", len("Erster Satz.")) + texts = {b["id"]: b["text"] for b in out["blocks"]} + self.assertEqual((texts["c"], texts[new_id]), ("Erster Satz.", "Zweiter Absatz beginnt.")) + self.assertEqual(self.order(out), ["a", "b", "c", new_id]) + first, second = (next(b for b in out["blocks"] if b["id"] == i)["bbox"] for i in ("c", new_id)) + self.assertAlmostEqual(first[1] + first[3], second[1]) # stacked, no gap or overlap + + def test_a_split_at_the_edge_is_rejected(self): + from core.magazine import split_region + for at in (0, 999): + with self.assertRaises(ValueError): + split_region(self.layout(), 1, "c", at) + + def test_merge_route_drops_the_folded_region_from_articles(self): + import asyncio + from unittest.mock import patch + from routes.magazine import magazine_merge_region + articles = [{"id": "x", "blocks": ["a", "b", "c"], "images": []}] + saved = {} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=self.layout()), \ + patch('routes.magazine.mag.save_layout'), \ + patch('routes.magazine.mag.load_articles', return_value=articles), \ + patch('routes.magazine.mag.save_articles', side_effect=lambda _d, a: saved.setdefault('a', a)): + asyncio.run(magazine_merge_region('a' * 32, 1, 'a')) + self.assertEqual(saved['a'][0]['blocks'], ['a', 'c']) + + def test_split_route_adds_the_new_part_after_the_original_in_articles(self): + import asyncio + from unittest.mock import AsyncMock, patch + from routes.magazine import magazine_split_region + articles = [{"id": "x", "blocks": ["a", "b", "c"], "images": []}] + saved = {} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=self.layout()), \ + patch('routes.magazine.mag.save_layout'), \ + patch('routes.magazine.mag.load_articles', return_value=articles), \ + patch('routes.magazine.mag.save_articles', side_effect=lambda _d, a: saved.setdefault('a', a)): + request = AsyncMock(); request.json.return_value = {"at": 12} + out = asyncio.run(magazine_split_region('a' * 32, 1, 'c', request)) + self.assertEqual(saved['a'][0]['blocks'], ['a', 'b', 'c', out['new_id']]) + + def test_split_route_rejects_a_bad_position(self): + import asyncio + from unittest.mock import AsyncMock, patch + from routes.magazine import magazine_split_region + for body in ({}, {"at": "3"}, {"at": 0}, {"at": True}): + with self.subTest(body=body), \ + patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=self.layout()): + request = AsyncMock(); request.json.return_value = body + with self.assertRaises(HTTPException): + asyncio.run(magazine_split_region('a' * 32, 1, 'c', request))