diff --git a/CHANGELOG.md b/CHANGELOG.md index a9d8af5..8e39c49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi ## [Unreleased] +## [1.39.0] — 2026-09-21 + +### Added +- **Magazine review: select many boxes and act once.** *Select boxes* lets you click boxes or drag a rectangle over them, then change all their types or delete them in one step. +- **Undo instead of "are you sure?".** Deleting, changing a type, editing text, merging and splitting happen at once and offer **Undo** (the latest change, while nothing newer has happened). +- **Text inside a new picture folds into it.** When a region is drawn as, or changed into, a picture, the short text inside it (a chart's legend and axis labels) is no longer read aloud — no more deleting a dozen labels one by one. + +### Changed +- Deleting a single region no longer asks for confirmation; it can be undone instead. + +### Fixed +- Opening a region's text editor right after another change could have it wiped by the redraw still in flight. + ## [1.38.1] — 2026-09-21 ### Fixed diff --git a/Manual.md b/Manual.md index 1ba41f4..4ffe79e 100644 --- a/Manual.md +++ b/Manual.md @@ -177,8 +177,18 @@ turned into a podcast discussion between two or three voices you pick. 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 + (with Undo); letters typed in the text editor are never taken as + shortcuts. To fix **many boxes at once**, press **"Select boxes"**, then + click boxes on the page or drag a rectangle over them (every box at least + half inside is selected; X selects the focused row from the keyboard). A + bar shows how many are selected: change their type in one go, or delete + them. Deleting, changing a type, merging and splitting no longer ask + "are you sure?" — they happen at once and an **Undo** button appears + above the list (it undoes the latest change, and only while nothing else + has changed since). When a region becomes a picture — drawn as one or + changed into one — the short text inside it (a chart's legend and axis + labels) automatically becomes *Text in picture*, so there is nothing left + to delete by hand. 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 53ec566..b75611c 100644 --- a/TODO.md +++ b/TODO.md @@ -64,6 +64,17 @@ not part of this order. ### P1 — next, highest user impact +- [ ] **L — Magazine: model-based layout detection and reading order.** Evaluate replacing the hand-written + region and order rules with dedicated models, as in the approach from + [Building agentic document understanding with OCR and LLMs](https://damimartinez.github.io/building-agentic-document-understanding-with-ocr-and-llms/): + **PaddleOCR PP-DocLayout** (regions: text, title, figure, chart, table, footnote) and **LayoutReader** + (a LayoutLMv3 fine-tune that orders regions from text + boxes). Before building: licences (PaddleOCR + Apache-2.0; LayoutLMv3 weights CC BY-NC-SA 4.0 -- non-commercial only), GPU/RAM on the DGX (already near + its limit), and a measurement on the same 204-page issue with the existing continuation score (current + heuristics: 89 confirmed continuations; any model must beat that). A tagged PDF, when a publisher ships one, + already carries the reading order in its structure tree -- read that first when present + ([reading-order best practice](https://www.globalvision.co/artwork-best-practices/reading-order)). + - [ ] **M — Magazine podcasts via Open Notebook (multi-speaker).** [Open Notebook](https://github.com/lfnovo/open-notebook) has an advanced multi-speaker podcast generator ("Generate professional podcasts") that could make the magazine podcast mode much better. It already runs on the LAN at `192.168.178.19` (Docker: API diff --git a/VERSION b/VERSION index 7bf334d..9c235b4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.38.1 \ No newline at end of file +1.39.0 \ No newline at end of file diff --git a/architecture/magazine-ingestion.md b/architecture/magazine-ingestion.md index f535a91..bcd0bb1 100644 --- a/architecture/magazine-ingestion.md +++ b/architecture/magazine-ingestion.md @@ -339,6 +339,25 @@ after the first and joins the same articles. Keyboard: ↑/↓ (j/k), Space, Ent ignored while typing in the editor, a select or a position box; focus returns to the same row after the list is redrawn. +## Many boxes at once, and undo instead of confirmation + +`POST .../pages/{page}/regions/bulk {ids, action: delete|type, type?}` applies one action to a selection +(≤ 500 ids, all on the page; validated before anything changes) with one undo point, and moves article +membership like the single-region routes. Every destructive route (delete, type change, text edit, merge, +split, bulk) writes `undo.json` = {token, label, layout, articles} *after* validating and *before* saving +(the files on disk still hold the old state), and returns the token; `POST .../undo {token}` restores both +files only if that token is still the latest (409 otherwise), so an old Undo can never roll back a newer +change. One level. UI: *Select boxes* mode (click or drag a rectangle; a box ≥ 50% inside is selected), +a bulk bar, and an Undo bar replacing the per-delete confirmation dialog (per NN/g-style guidance: frequent, +reversible actions get Undo, not "are you sure?"). + +`fold_text_into_picture`: when a region becomes a picture (drawn, or changed into one), text ≤ 80 chars and +≥ 90% inside it becomes `in_picture` -- the same limits as the clean-up. A real chart page had 13 legend and +axis-label boxes that had to be deleted one by one after the chart was marked as an image. + +Redraw race: the review list redraws asynchronously after a change; actions that build UI in the list (the +text editor) wait for a redraw in flight, and focus goes to the row focused when the redraw happens. + ## 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 99bc3de..65929ad 100644 --- a/core/magazine.py +++ b/core/magazine.py @@ -2589,6 +2589,7 @@ def add_manual_region(doc_dir: Path, layout: dict[str, Any], page_no: int, images.append({"id": name, "page": page_no, "bbox": list(bbox), "filename": f"{name}.png", "reading_order": next_ro}) layout["images"] = images + layout, _ = fold_text_into_picture(layout, page_no, bbox) return layout manual_type, region_label = { @@ -2887,6 +2888,28 @@ def clean_up_regions(layout: dict[str, Any], pages: Iterable[int] | None = None) return layout, report +def fold_text_into_picture(layout: dict[str, Any], page_no: int, bbox: Sequence[float]) -> tuple[dict[str, Any], list[str]]: + """Short text lying inside a picture becomes part of it (not read aloud). Returns the ids changed. + + Used when a region becomes a picture -- drawn as one, or changed into one. A chart's legend and + axis labels had to be deleted by hand one by one after the chart was marked as an image (a real + page carried 13 of them). Same limits as clean_up_regions: at most 80 characters, at least 90% + inside, so real running text caught under a too-large frame is left alone. + """ + changed: list[str] = [] + blocks = [] + for b in layout.get("blocks", []): + text = " ".join((b.get("text") or "").split()) + if (b["page"] == page_no and b["type"] not in _VISUAL_TYPES and b["type"] != BLOCK_FURNITURE + and text and len(text) <= 80 and _area_inside(b["bbox"], bbox) >= 0.9): + b = dict(b, type=BLOCK_FURNITURE, region_label="in_picture", confidence=0.9) + changed.append(b["id"]) + blocks.append(b) + layout = dict(layout) + layout["blocks"] = blocks + return layout, changed + + # What a region can be changed into from the review screen: text kinds map to a block type (and, # for furniture, which kind of furniture); "image" turns the region into a figure crop. REGION_TYPES: dict[str, tuple[str, str]] = { @@ -2939,7 +2962,8 @@ def change_region_type(doc_dir: Path, layout: dict[str, Any], page_no: int, regi before = {im["id"] for im in layout.get("images", [])} layout = add_manual_region(doc_dir, layout, page_no, block["bbox"], "image") new_id = next(im["id"] for im in layout["images"] if im["id"] not in before) - return _reorder_page(delete_region(layout, region_id), page_no), new_id + layout, _ = fold_text_into_picture(delete_region(layout, region_id), page_no, block["bbox"]) + return _reorder_page(layout, page_no), new_id block_type, region_label = REGION_TYPES[new_type] if block is not None: diff --git a/routes/magazine.py b/routes/magazine.py index 2de97aa..1fee5ab 100644 --- a/routes/magazine.py +++ b/routes/magazine.py @@ -754,6 +754,95 @@ def _add_split_paragraphs_to_articles(articles: list[dict], report: dict) -> boo return changed +def _snapshot_for_undo(d: Path, label: str) -> str: + """Keep the layout and articles as they are now, so the change about to happen can be undone. + One level: the next snapshot replaces this one. Returns the token the client undoes with.""" + token = uuid.uuid4().hex + snapshot = {"token": token, "label": label, "layout": mag.load_layout(d), "articles": mag.load_articles(d)} + (d / "undo.json").write_text(json.dumps(snapshot, ensure_ascii=False), "utf-8") + return token + + +@router.post("/api/magazine/{doc_id}/undo") +async def magazine_undo(doc_id: str, request: Request): + """Undo the last region change (delete, type change, merge, split, bulk). Body: {token}. + Only the latest change can be undone, so a stale token never rolls back a newer one.""" + d, _ = _require_magazine(doc_id) + body = await request.json() + token = body.get("token") if isinstance(body, dict) else None + path = d / "undo.json" + if not isinstance(token, str) or not path.is_file(): + raise HTTPException(409, "Nothing to undo") + snapshot = json.loads(path.read_text("utf-8")) + if snapshot.get("token") != token: + raise HTTPException(409, "Something else has changed since -- this can no longer be undone") + mag.save_layout(d, snapshot["layout"]) + mag.save_articles(d, snapshot["articles"]) + path.unlink(missing_ok=True) + return {"ok": True, "label": snapshot.get("label", "")} + + +@router.post("/api/magazine/{doc_id}/pages/{page}/regions/bulk") +async def magazine_bulk_regions(doc_id: str, page: int, request: Request): + """Delete or re-type several regions at once. Body: {ids: [...], action: "delete"|"type", + type?: one of REGION_TYPES}. One undo point for the whole batch.""" + 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() + if not isinstance(body, dict): + raise HTTPException(400, "Body must be an object") + ids, action, new_type = body.get("ids"), body.get("action"), body.get("type") + if not isinstance(ids, list) or not ids or len(ids) > 500 or not all(isinstance(i, str) for i in ids): + raise HTTPException(400, "ids must be a non-empty list of region ids") + if action not in ("delete", "type"): + raise HTTPException(400, "action must be delete or type") + if action == "type" and new_type not in mag.REGION_TYPES: + raise HTTPException(400, f"type must be one of {', '.join(mag.REGION_TYPES)}") + on_page = {b["id"] for b in layout.get("blocks", []) if b["page"] == page} | \ + {i["id"] for i in layout.get("images", []) if i["page"] == page} + missing = [i for i in ids if i not in on_page] + if missing: + raise HTTPException(400, f"Not on page {page}: {missing[:5]}") + ids = list(dict.fromkeys(ids)) + token = _snapshot_for_undo(d, f"{'Deleted' if action == 'delete' else 'Changed'} {len(ids)} region{'s' if len(ids) != 1 else ''}") + remap: dict[str, str | None] = {} + if action == "delete": + for rid in ids: + layout = mag.delete_region(layout, rid) + remap[rid] = None + else: + settings = _load_settings() + ocr = [str(settings.get(k) or "").strip() or None for k in + ("vision_ocr_url", "vision_ocr_model", "vision_ocr_fallback_url", "vision_ocr_fallback_model")] + for rid in ids: + if not any(b["id"] == rid for b in layout.get("blocks", [])) and \ + not any(i["id"] == rid for i in layout.get("images", [])): + continue # already folded into a picture made earlier in this batch + try: + layout, new_id = await asyncio.to_thread(mag.change_region_type, d, layout, page, rid, new_type, *ocr) + except ValueError as exc: + raise HTTPException(400, str(exc)) + if new_id != rid: + remap[rid] = new_id + mag.save_layout(d, layout) + articles = mag.load_articles(d) + changed = False + for article in articles: + for rid, new_id in remap.items(): + for key in ("blocks", "images"): + if rid in (article.get(key) or []): + article[key] = [x for x in article[key] if x != rid] + if new_id: + target = "images" if new_type == "image" else "blocks" + article[target] = list(article.get(target) or []) + [new_id] + changed = True + if changed: + mag.save_articles(d, articles) + return {"ok": True, "undo": token, "remap": remap, "count": len(ids)} + + @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).""" @@ -765,6 +854,7 @@ async def magazine_merge_region(doc_id: str, page: int, block_id: str): layout, removed = mag.merge_with_next(layout, page, block_id) except ValueError as exc: raise HTTPException(400, str(exc)) + token = _snapshot_for_undo(d, "Merged two regions") if d is not None else None mag.save_layout(d, layout) articles = mag.load_articles(d) changed = False @@ -776,7 +866,7 @@ async def magazine_merge_region(doc_id: str, page: int, block_id: str): changed = True if changed: mag.save_articles(d, articles) - return {"ok": True, "id": block_id, "removed": removed} + return {"ok": True, "id": block_id, "removed": removed, "undo": token} @router.post("/api/magazine/{doc_id}/pages/{page}/regions/{block_id}/split") @@ -795,11 +885,12 @@ async def magazine_split_region(doc_id: str, page: int, block_id: str, request: layout, new_id = mag.split_region(layout, page, block_id, at) except ValueError as exc: raise HTTPException(400, str(exc)) + token = _snapshot_for_undo(d, "Split a region") if d is not None else None 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} + return {"ok": True, "id": block_id, "new_id": new_id, "undo": token} @router.post("/api/magazine/{doc_id}/boxes/clean-up") @@ -999,10 +1090,12 @@ async def magazine_update_region(doc_id: str, page: int, block_id: str, request: ) except ValueError as exc: raise HTTPException(400, str(exc)) + # Taken once the change is known to be valid, from the files on disk (still the old state). + token = _snapshot_for_undo(d, "Changed a region") if (new_type is not None or new_text is not None) and d is not None else None mag.save_layout(d, layout) if region_id != block_id: _move_article_membership(d, block_id, region_id, to_image=new_type == "image") - return {"ok": True, "text": text, "id": region_id, "type": new_type} + return {"ok": True, "text": text, "id": region_id, "type": new_type, "undo": token} def _move_article_membership(d: Path, old_id: str, new_id: str, to_image: bool) -> None: @@ -1035,6 +1128,7 @@ async def magazine_delete_region(doc_id: str, page: int, block_id: str): if not any(b["id"] == block_id for b in layout.get("blocks", [])) and \ not any(im["id"] == block_id for im in layout.get("images", [])): raise HTTPException(404, "Region not found") + token = _snapshot_for_undo(d, "Deleted a region") if d is not None else None layout = mag.delete_region(layout, block_id) mag.save_layout(d, layout) # Also drop it from articles: an article still listing a deleted id made Approve fail with @@ -1048,7 +1142,7 @@ async def magazine_delete_region(doc_id: str, page: int, block_id: str): changed = True if changed: mag.save_articles(d, articles) - return {"ok": True} + return {"ok": True, "undo": token} @router.post("/api/magazine/{doc_id}/pages/{page}/extract") diff --git a/static/index.html b/static/index.html index 5475b2f..0ec6475 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 c34932f..ca06dd3 100644 --- a/static/js/magazine.js +++ b/static/js/magazine.js @@ -705,6 +705,7 @@ async function magReviewChangeType(regionId, type, select) { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ type }), }); if (r && r.id && r.id !== regionId) magRemapSelection(regionId, r.id, toImage); + magOfferUndo(r?.undo, 'Region type changed.'); } catch (e) { if (select) select.disabled = false; return say('Could not change the region type: ' + e.message, 'err'); @@ -808,7 +809,7 @@ function magReviewPageColumn(page, wrapId, interactive) { let listHtml = '
' + - (interactive ? '

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

' : ''); + (interactive ? '

Keys: ↑/↓ move between regions · Space tick · X select · Enter edit text · R read again · M merge with the next region · Delete delete (Undo appears above)

' : ''); regions.forEach((r, i) => { const isImage = r.kind === 'image'; const owner = magBlockOwner(r.id); @@ -890,6 +891,7 @@ function renderReview() { if (!host || !a) return; // 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; } const conf = confidenceLabel(a.confidence); const page = state.reviewPage; const pageCount = Number(state.layout?.page_count || 0); @@ -935,6 +937,9 @@ function renderReview() { '' + + '' + '' + @@ -960,7 +965,9 @@ function renderReview() { '' + '
' + '' + - '
' + magOrderBarHtml() + '
'; + '
' + magOrderBarHtml() + '
' + + '
' + magSelectBarHtml() + '
' + + '
' + magUndoBarHtml() + '
'; html += ''; html += '' + esc(conf.text) + ''; @@ -1015,6 +1022,7 @@ function renderReview() { $('mag-review-page-wrap')?.classList.add('is-ordering'); magOrderRefreshBar(); } + if (state.selectMode) magSelectRefresh(); setupMagDrawing(); setupMagBoxEdit(); } @@ -1248,8 +1256,126 @@ async function magOrderReset() { renderReview(); } +// ── Select several boxes, act once; undo instead of confirming ───────────── + +function magSelectBarHtml() { + if (!state.selectMode) return ''; + const n = (state.selected || new Set()).size; + const options = MAG_REGION_TYPES.map(([v, label]) => '').join(''); + return ' ' + + (n ? n + ' selected.' : 'Click boxes on the page, or drag a rectangle over them, to select them.') + ' ' + + ' ' + + ' ' + + ' ' + + ''; +} + +function magSelectRefresh() { + const bar = $('mag-select-bar'); + if (bar) { + bar.innerHTML = magSelectBarHtml(); + $('mag-bulk-type')?.addEventListener('change', (e) => { if (e.target.value) magBulk('type', e.target.value); }); + } + const sel = state.selected || new Set(); + $('mag-review-page-wrap')?.classList.toggle('is-selecting', !!state.selectMode); + document.querySelectorAll('#mag-review [data-block], #mag-review [data-block-row]').forEach((el) => { + const id = el.dataset.block || el.dataset.blockRow; + if (el.matches('.mag-review-box, [data-block-row]')) el.classList.toggle('is-selected', !!state.selectMode && sel.has(id)); + }); +} + +function magSelectToggle() { + state.selectMode = !state.selectMode; + state.selected = new Set(); + state.selectPage = state.reviewPage; + if (state.selectMode && state.drawMode) magReviewToggleDraw(state.drawKind); + if (state.selectMode && state.editMode) magReviewToggleEdit(); + if (state.selectMode && state.orderMode) magOrderToggle(); + const btn = $('mag-select-toggle'); + if (btn) { btn.classList.toggle('active', state.selectMode); btn.setAttribute('aria-pressed', String(state.selectMode)); } + magSelectRefresh(); +} + +function magSelectToggleOne(id) { + const sel = state.selected || (state.selected = new Set()); + if (sel.has(id)) sel.delete(id); else sel.add(id); + magSelectRefresh(); +} + +function magSelectNone() { + state.selected = new Set(); + magSelectRefresh(); +} + +// Boxes at least half inside the dragged rectangle (all values in % of the page). +function magSelectInRect(left, top, width, height) { + const sel = state.selected || (state.selected = new Set()); + document.querySelectorAll('#mag-review-page-wrap .mag-review-box[data-block]').forEach((box) => { + const bl = parseFloat(box.style.left), bt = parseFloat(box.style.top); + const bw = parseFloat(box.style.width), bh = parseFloat(box.style.height); + const ix = Math.max(0, Math.min(bl + bw, left + width) - Math.max(bl, left)); + const iy = Math.max(0, Math.min(bt + bh, top + height) - Math.max(bt, top)); + if (bw > 0 && bh > 0 && ix * iy >= 0.5 * bw * bh) sel.add(box.dataset.block); + }); + magSelectRefresh(); +} + +async function magBulk(action, type) { + const ids = Array.from(state.selected || []); + if (!ids.length) return; + let r; + try { + r = await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/bulk', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(action === 'type' ? { ids, action, type } : { ids, action }), + }); + } catch (e) { + return say('Could not change the selected boxes: ' + e.message, 'err'); + } + Object.entries(r.remap || {}).forEach(([oldId, newId]) => magRemapSelection(oldId, newId, type === 'image')); + state.selected = new Set(); + magOfferUndo(r.undo, (action === 'delete' ? 'Deleted ' : 'Changed ') + r.count + ' box' + (r.count === 1 ? '' : 'es') + '.'); + await magReloadReview(); +} + +function magUndoBarHtml() { + if (!state.undo) return ''; + return ' ' + esc(state.undo.label) + ' ' + + ''; +} + +function magOfferUndo(token, label) { + state.undo = token ? { token, label } : null; + const bar = $('mag-undo-bar'); + if (bar) bar.innerHTML = magUndoBarHtml(); +} + +async function magUndo() { + const undo = state.undo; + if (!undo) return; + try { + await api('/api/magazine/' + state.doc.id + '/undo', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ token: undo.token }), + }); + } catch (e) { + state.undo = null; + magOfferUndo(null); + return say('Could not undo: ' + e.message, 'err'); + } + state.undo = null; + // The article itself was restored too: take it (and its ticks) from the server again. + const articleId = state.article?.id; + await magRefreshArticles(); + state.article = (state.articles || []).find((a) => a.id === articleId) || state.article; + _magReviewChecked = null; + await magReloadReview('Undone'); +} + function magReviewFocusBlock(blockId) { if (state.orderMode) return magOrderPick(blockId); + if (state.selectMode) return magSelectToggleOne(blockId); const cb = document.querySelector('#mag-review input[data-block="' + CSS.escape(blockId) + '"]'); if (state.drawMode || state.editMode) return; if (cb) { cb.checked = !cb.checked; magReviewToggleBox(cb); } @@ -1536,7 +1662,14 @@ function magRegionUrl(id) { return '/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(id); } -async function magReloadReview(message) { +function magReloadReview(message) { + // Other actions (opening the text editor) wait for a redraw in flight, or it would wipe them. + const run = magReloadReviewNow(message).finally(() => { if (state.reloadPending === run) state.reloadPending = null; }); + state.reloadPending = run; + return run; +} + +async function magReloadReviewNow(message) { // Keep a keyboard user's place: the row focused when the redraw happens (not when the change // started -- the user may have moved on meanwhile) gets focus back. state.layout = null; @@ -1547,7 +1680,8 @@ async function magReloadReview(message) { if (message) say(message, 'ok'); } -function magRowEdit(id) { +async function magRowEdit(id) { + if (state.reloadPending) await state.reloadPending; 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'); @@ -1577,6 +1711,7 @@ function magRowEdit(id) { if (act === 'split') { const r = await api(magRegionUrl(id) + '/split', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ at: area.selectionStart }) }); if (r?.new_id) magSelectionAddAlongside(id, r.new_id); + magOfferUndo(r?.undo, 'Region split in two.'); return magReloadReview('Region split in two'); } magReloadReview(); @@ -1601,6 +1736,7 @@ async function magRowMerge(id) { try { const r = await api(magRegionUrl(id) + '/merge-next', { method: 'POST' }); if (r?.removed) magRemapSelection(r.removed, null, false); + magOfferUndo(r?.undo, 'Merged with the next region.'); } catch (e) { return say('Could not merge: ' + e.message, 'err'); } @@ -1629,24 +1765,24 @@ function magListKey(event) { 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); } + else if (key === 'x') { event.preventDefault(); if (!state.selectMode) magSelectToggle(); magSelectToggleOne(id); } } +// Deleting is immediate and offers Undo -- a confirmation dialog for every box made clearing a page of +// chart labels a click-marathon. 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 }); - if (!ok) return; + let r; try { - await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(blockId), { + r = await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(blockId), { method: 'DELETE', }); magRemapSelection(blockId, null, false); + state.selected?.delete(blockId); } catch (e) { return say('Could not delete region: ' + e.message, 'err'); } - say('Region deleted', 'ok'); - state.layout = null; - await ensureLayout(); - renderReview(); + magOfferUndo(r?.undo, 'Region deleted.'); + await magReloadReview(); } let _magDrawState = null; // { startXPct, startYPct } while dragging @@ -1662,7 +1798,7 @@ function setupMagDrawing() { // only the window-level one below needs the one-time guard, since window // is never replaced. wrap.addEventListener('mousedown', (e) => { - if (!state.drawMode || e.button !== 0 || e.target.closest('.mag-draw-panel')) return; + if (!(state.drawMode || state.selectMode) || e.button !== 0 || e.target.closest('.mag-draw-panel')) return; magReviewCancelDrawPanel(); const rect = wrap.getBoundingClientRect(); _magDrawState = { @@ -1715,6 +1851,7 @@ function setupMagDrawing() { if (!box || box.hidden) return; const w = parseFloat(box.style.width) || 0, h = parseFloat(box.style.height) || 0; if (w < 1 || h < 1) { box.hidden = true; return; } // too small to be intentional + if (state.selectMode) { box.hidden = true; return magSelectInRect(parseFloat(box.style.left), parseFloat(box.style.top), w, h); } magReviewOpenDrawPanel(box); }); } @@ -2724,6 +2861,10 @@ root.magReviewChangeType = magReviewChangeType; root.magFixReadingOrder = magFixReadingOrder; root.magCleanUpBoxes = magCleanUpBoxes; root.magRowEdit = magRowEdit; +root.magSelectToggle = magSelectToggle; +root.magSelectNone = magSelectNone; +root.magBulk = magBulk; +root.magUndo = magUndo; root.magRowReocr = magRowReocr; root.magRowMerge = magRowMerge; root.magListKey = magListKey; diff --git a/static/style.css b/static/style.css index 5d589bc..021c823 100644 --- a/static/style.css +++ b/static/style.css @@ -5182,3 +5182,10 @@ input[list="lang-codes"] { 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; } + +/* Select several boxes (magazine review) */ +.mag-review-page-wrap.is-selecting { cursor: crosshair; } +.mag-review-page-wrap.is-selecting .mag-review-box { cursor: pointer; } +.mag-review-box.is-selected { outline: 3px solid var(--warning, #d97706); outline-offset: -1px; + background: color-mix(in srgb, var(--warning, #d97706) 18%, transparent); } +.mag-blocks [data-block-row].is-selected { border-color: var(--warning, #d97706); box-shadow: inset 3px 0 0 var(--warning, #d97706); } diff --git a/tests/magazine.spec.js b/tests/magazine.spec.js index 66fc6cd..3904ca2 100644 --- a/tests/magazine.spec.js +++ b/tests/magazine.spec.js @@ -360,25 +360,30 @@ test('every type the server knows is offered, image included', async ({ page }) 'page_number', 'source', 'ad', 'in_picture', 'ocr_noise', 'duplicate', 'image']); }); -test('each review row has its own delete button that asks first', async ({ page }) => { +test('each review row has its own delete button, which deletes at once and offers undo', async ({ page }) => { await mockBackend(page); - let deleted = false; + let deleted = false, undone = false; await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/b3`, route => { expect(route.request().method()).toBe('DELETE'); deleted = true; + return route.fulfill({ json: { ok: true, undo: 'tok-1' } }); + }); + await page.route(`**/api/magazine/${DOC_ID}/undo`, route => { + expect(route.request().postDataJSON()).toEqual({ token: 'tok-1' }); + undone = true; return route.fulfill({ json: { ok: true } }); }); await openReview(page); const button = page.locator('[data-block-row="b3"]').getByRole('button', { name: /Delete region/ }); - // The button sits in its own column and does not cover the paragraph text. const [btn, text] = [await button.boundingBox(), await page.locator('[data-block-row="b3"] .mag-block-text').boundingBox()]; expect(btn.x + btn.width).toBeLessThanOrEqual(text.x); await button.click(); - const dialog = page.getByRole('alertdialog', { name: 'Delete region?' }); - await expect(dialog).toContainText('Delete this region?'); - expect(deleted).toBe(false); - await dialog.getByRole('button', { name: 'Delete', exact: true }).click(); await expect.poll(() => deleted).toBe(true); + await expect(page.getByRole('alertdialog')).toHaveCount(0); + await expect(page.locator('#mag-undo-bar')).toContainText('Region deleted.'); + await page.locator('#mag-undo-bar').getByRole('button', { name: 'Undo', exact: true }).click(); + await expect.poll(() => undone).toBe(true); + await expect(page.locator('#mag-undo-bar')).toBeEmpty(); }); test('fix reading order asks first, then reports how many pages changed', async ({ page }) => { @@ -575,9 +580,52 @@ test('approving after deleting a ticked box does not send its id', async ({ page await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/b3`, route => { deleted = true; return route.fulfill({ json: { ok: true } }); }); await openReview(page); await page.locator('[data-block-row="b3"]').getByRole('button', { name: /Delete region/ }).click(); - await page.getByRole('alertdialog').getByRole('button', { name: 'Delete', exact: true }).click(); await expect(page.locator('[data-block-row="b3"]')).toHaveCount(0); const request = page.waitForRequest(r => r.method() === 'PATCH' && r.url().includes('/articles/article-001')); await page.getByRole('button', { name: 'Approve', exact: true }).click(); expect((await request).postDataJSON().blocks).not.toContain('b3'); }); + +test('select boxes by dragging a rectangle, then delete them in one go with undo', async ({ page }) => { + await mockBackend(page); + let sent = null; + await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/bulk`, route => { + sent = route.request().postDataJSON(); + return route.fulfill({ json: { ok: true, undo: 'tok-2', remap: { b2: null, b3: null }, count: 2 } }); + }); + await openReview(page); + await page.getByRole('button', { name: 'Select boxes', exact: true }).click(); + await expect(page.locator('#mag-select-bar')).toContainText('drag a rectangle'); + await page.locator('#mag-review-page-wrap').evaluate(el => { el.style.width = '400px'; el.scrollIntoView(); }); + const wrap = await page.locator('#mag-review-page-wrap').boundingBox(); + // b2 [50,120,200,14] and b3 [50,150,200,300] on a 595x842 page; b1 (the headline at y 60) stays out. + const at = (x, y) => [wrap.x + wrap.width * x / 595, wrap.y + wrap.height * y / 842]; + await page.mouse.move(...at(40, 112)); + await page.mouse.down(); + await page.mouse.move(...at(270, 470), { steps: 6 }); + await page.mouse.up(); + await expect(page.locator('#mag-select-bar')).toContainText('2 selected.'); + await expect(page.locator('#mag-review-page-wrap [data-block="b1"]')).not.toHaveClass(/is-selected/); + await page.locator('#mag-select-bar').getByRole('button', { name: /^Delete/ }).click(); + await expect.poll(() => sent).toEqual({ ids: expect.arrayContaining(['b2', 'b3']), action: 'delete' }); + await expect(page.locator('#mag-undo-bar')).toContainText('Deleted 2 boxes.'); +}); + +test('clicking boxes in select mode builds a selection whose type changes at once', async ({ page }) => { + await mockBackend(page); + let sent = null; + await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/bulk`, route => { + sent = route.request().postDataJSON(); + return route.fulfill({ json: { ok: true, undo: 'tok-3', remap: {}, count: 2 } }); + }); + await openReview(page); + const b1Checked = await page.locator('#mag-review input[data-block="b1"]').isChecked(); + await page.getByRole('button', { name: 'Select boxes', exact: true }).click(); + await page.locator('#mag-review-page-wrap [data-block="b1"]').click(); + await page.locator('#mag-review-page-wrap [data-block="b2"]').click(); + await expect(page.locator('#mag-select-bar')).toContainText('2 selected.'); + // Selecting is not ticking: the include checkboxes stay as they were. + await expect(page.locator('#mag-review input[data-block="b1"]')).toBeChecked({ checked: b1Checked }); + await page.locator('#mag-bulk-type').selectOption('ocr_noise'); + await expect.poll(() => sent).toEqual({ ids: ['b1', 'b2'], action: 'type', type: 'ocr_noise' }); +}); diff --git a/tests/test_magazine_routes.py b/tests/test_magazine_routes.py index 7edb988..7c983a9 100644 --- a/tests/test_magazine_routes.py +++ b/tests/test_magazine_routes.py @@ -1030,3 +1030,96 @@ class QuickCorrectionTests(unittest.TestCase): request = AsyncMock(); request.json.return_value = body with self.assertRaises(HTTPException): asyncio.run(magazine_split_region('a' * 32, 1, 'c', request)) + + +class BulkUndoFoldTests(unittest.TestCase): + """Folding labels into a new picture, bulk changes, and one-level undo.""" + + def setUp(self): + from pathlib import Path + from PIL import Image + self.dir = Path(tempfile.mkdtemp(prefix="ttvc-bulk-")) + (self.dir / "pages").mkdir() + Image.new("RGB", (600, 800), "white").save(self.dir / "pages" / "0001.png") + self.layout = {"pages": [{"page": 1, "width": 600.0, "height": 800.0}], "images": [], "blocks": [ + {"id": "chart", "page": 1, "bbox": [50, 300, 400, 300], "type": "body", "text": "Chart OCR text", "reading_order": 0}, + {"id": "lg1", "page": 1, "bbox": [60, 320, 80, 12], "type": "caption", "text": "Einsteiger (22-25 Jahre)", "reading_order": 1}, + {"id": "lg2", "page": 1, "bbox": [160, 320, 80, 12], "type": "caption", "text": "Senior (50 Jahre und älter)", "reading_order": 2}, + {"id": "body", "page": 1, "bbox": [50, 50, 400, 200], "type": "body", "text": "Running text above the chart.", "reading_order": 3}, + ]} + self.articles = [{"id": "a1", "blocks": ["chart", "lg1", "lg2", "body"], "images": []}] + from core import magazine as mag + mag.save_layout(self.dir, self.layout) + mag.save_articles(self.dir, self.articles) + + def tearDown(self): + shutil.rmtree(self.dir, ignore_errors=True) + + def run_route(self, fn, *args, body=None): + import asyncio + from unittest.mock import AsyncMock, patch + with patch('routes.magazine._require_magazine', return_value=(self.dir, {})), \ + patch('routes.magazine._load_settings', return_value={}), \ + patch('routes.magazine.asyncio.to_thread', new=_run_sync): + request = AsyncMock(); request.json.return_value = body or {} + return asyncio.run(fn('a' * 32, *args, request) if body is not None else fn('a' * 32, *args)) + + def test_labels_inside_a_region_turned_into_a_picture_fold_into_it(self): + from core.magazine import change_region_type + layout, _ = change_region_type(self.dir, self.layout, 1, "chart", "image") + types = {b["id"]: (b["type"], b.get("region_label")) for b in layout["blocks"]} + self.assertEqual(types["lg1"], ("furniture", "in_picture")) + self.assertEqual(types["lg2"], ("furniture", "in_picture")) + self.assertEqual(types["body"][0], "body") # outside the picture: untouched + + def test_a_drawn_picture_folds_the_labels_inside_it_too(self): + from core.magazine import add_manual_region + layout = add_manual_region(self.dir, self.layout, 1, [50, 300, 400, 300], "image") + self.assertEqual(next(b for b in layout["blocks"] if b["id"] == "lg1")["region_label"], "in_picture") + + def test_bulk_delete_removes_regions_and_article_entries_and_can_be_undone(self): + from core import magazine as mag + from routes.magazine import magazine_bulk_regions, magazine_undo + out = self.run_route(magazine_bulk_regions, 1, body={"ids": ["lg1", "lg2"], "action": "delete"}) + self.assertEqual(out["count"], 2) + self.assertEqual({b["id"] for b in mag.load_layout(self.dir)["blocks"]}, {"chart", "body"}) + self.assertEqual(mag.load_articles(self.dir)[0]["blocks"], ["chart", "body"]) + self.run_route(magazine_undo, body={"token": out["undo"]}) + self.assertEqual(len(mag.load_layout(self.dir)["blocks"]), 4) + self.assertEqual(mag.load_articles(self.dir)[0]["blocks"], ["chart", "lg1", "lg2", "body"]) + + def test_bulk_type_change_applies_to_every_selected_region(self): + from core import magazine as mag + from routes.magazine import magazine_bulk_regions + self.run_route(magazine_bulk_regions, 1, body={"ids": ["lg1", "lg2"], "action": "type", "type": "ocr_noise"}) + labels = {b["id"]: b.get("region_label") for b in mag.load_layout(self.dir)["blocks"]} + self.assertEqual((labels["lg1"], labels["lg2"]), ("ocr_noise", "ocr_noise")) + + def test_bulk_turning_text_into_a_picture_moves_it_to_the_article_images(self): + from core import magazine as mag + from routes.magazine import magazine_bulk_regions + out = self.run_route(magazine_bulk_regions, 1, body={"ids": ["chart"], "action": "type", "type": "image"}) + new_id = out["remap"]["chart"] + article = mag.load_articles(self.dir)[0] + self.assertNotIn("chart", article["blocks"]) + self.assertEqual(article["images"], [new_id]) + + def test_only_the_latest_change_can_be_undone(self): + from routes.magazine import magazine_bulk_regions, magazine_undo + first = self.run_route(magazine_bulk_regions, 1, body={"ids": ["lg1"], "action": "delete"}) + self.run_route(magazine_bulk_regions, 1, body={"ids": ["lg2"], "action": "delete"}) + with self.assertRaises(HTTPException) as err: + self.run_route(magazine_undo, body={"token": first["undo"]}) + self.assertEqual(err.exception.status_code, 409) + + def test_bulk_rejects_bad_requests_before_changing_anything(self): + from core import magazine as mag + from routes.magazine import magazine_bulk_regions + for body in ({"ids": [], "action": "delete"}, {"ids": ["lg1"], "action": "nuke"}, + {"ids": ["lg1"], "action": "type", "type": "nonsense"}, {"ids": ["elsewhere"], "action": "delete"}): + with self.subTest(body=body): + with self.assertRaises(HTTPException) as err: + self.run_route(magazine_bulk_regions, 1, body=body) + self.assertEqual(err.exception.status_code, 400) + self.assertEqual(len(mag.load_layout(self.dir)["blocks"]), 4) + self.assertFalse((self.dir / "undo.json").exists())