diff --git a/src/App.svelte b/src/App.svelte index fcd6b5d..b032f8e 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -1496,6 +1496,7 @@ {pageDecor} {lineNumbering} {foldMarks} + commentAuthor={docProps.author} bind:extraHfSections {zoom} onZoom={setZoom} diff --git a/src/lib/components/CLAUDE.md b/src/lib/components/CLAUDE.md index 00c2c72..bff08b9 100644 --- a/src/lib/components/CLAUDE.md +++ b/src/lib/components/CLAUDE.md @@ -107,8 +107,9 @@ a 177-page document. Word and LibreOffice draw only the visible pages; we can't. forever; with one shared scroller that loop never settles. - The editor's own view **moves** to pane 0's new host when the layout changes (`$effect` on `hosts[0]`), rather than being rebuilt from the document. -- `min-width: 0` on the row and its scroller: a flex item's automatic minimum is its - content, so without it the grid widens the whole app instead of scrolling. +- `min-width: 0` on the row and its scroller, in every mode: a flex item's automatic + minimum is its content, so without it a canvas wider than the window — the grid, or a + page beside the balloon strip — widens the app and the toolbar ends short of it. - No ruler here: it would repeat per column and eat into the page's own box. - **`sectionPaper` stays unrounded.** `pageBreaks` builds its grid from the height this publishes as `--pb-section-page`, so rounding A4's 1122.52px to 1123 ran every page @@ -258,6 +259,61 @@ mounted: with nothing marked it renders nothing. It sits inside `.paper`, so the export and print paths clone it and the bar prints — see `export/reviewPrint.ts` for what the vector path has to rebuild instead. +## What the review views may show (`reviewItems.ts`) + +Bars, balloons, the leader line, both panes and the two navigation pairs read +`visibleComments` / `visibleCommentRanges` / `visibleRevisions` instead of the document's +own `comments()` / `revisions()`, so the markup settings (`storage/markup`) are read in one +place and cannot drift between the five views. A **resolved** comment is not this rule's +business — each caller handles it as it did. The text itself is hidden by two levers that +have to agree: `.paper[data-hide-*]` in `editor.css` takes the mark's text and tint off the +page, and `trackChanges`' `plain` option drops the decoration that carries the underline, +the strikethrough and the author colour — CSS alone would lose against `pm-rev-a`. +An anchor inside hidden text has no box, so `ReviewMarginLayer` drops a card whose line has +collapsed; the mode change dispatches `FORCE_PAGE_RECALC`, since hiding text reflows pages. + +## Margin balloons (`ReviewMarginLayer.svelte`, `storage/markup.svelte.ts`) + +What both word processors draw beside the page: one card per unresolved comment and per +recorded change, at the height of the line it belongs to, with a dashed leader to it. +Accept/reject and edit/resolve/remove happen in the balloon — `CommentCard.svelte` and +`RevisionCard.svelte` are the same cards the two panes build their rows from. + +- **It sits outside `.paper`**, as a sibling inside `.paper-scaler`, carrying that same + `transform: scale()` so it zooms with the page. Inside `.paper` it could not: that box + is `overflow-x: clip`, and its width is the one `.tiptap` and the pagination measure. +- The strip is `REVIEW_MARGIN_CM` (gap + balloon) and is **only reserved when the document + has markup the margin shows** — a kind switched to the pane, or a mode that hides it, + leaves `.paper-scaler` exactly the page, centred as before. + `scaledCanvasWidth` (`Editor.svelte`) adds it; `scaledWidth` stays the page, which is + what the `Ruler` is sized from. +- **The leader runs in the gap under the anchor's line** (`coordsAtPos(...).bottom`, not the + line's middle) and eases into the card's left edge at the anchor's own height where the + card reaches that far — so a balloon sitting at its line is joined by one straight line + that crosses no letters. `ConnectorLayer` draws its leader to the pane the same way. +- A balloon wants its anchor's line and is pushed down where the one above it reaches + (`utils/balloonStack.ts`, unit-tested). Its height is **measured**, not computed — a card + grows when its text is edited — so a `ResizeObserver` feeds the stack. +- **The column belongs to its page, not to the document.** Balloons are grouped by the page + their anchor is on and stacked inside that page's band; a stack that would end past the + sheet is pulled back up (second pass in `stackTops`). The cards of one page also share its + height as a `max-height`, so a comment of a few paragraphs scrolls in its own card instead + of pushing the ones below it off the page — `MIN_BALLOON_H` is where the shrinking stops + and the column is allowed to run over. +- Its x comes from the anchor's own page box, so a narrower section's page keeps its + balloons at its own right edge. Not mounted in the page grid: `.page-cell` is one page + wide and clips. +- The **⋯** menu and the reply box are part of the shared card, so a balloon and a pane row + answer a comment the same way; both grow the card, and the stack follows the measurement. +- **The card is authored for the pane's 260px and lives here in `BALLOON_W_CM`**, so everything in + it has to wrap: the action row is `flex-wrap: wrap` (unwrapped, German labels pushed + "Bearbeiten" clean out of the balloon) and the balloon carries `overflow-wrap: anywhere` + plus `overflow-x: hidden`. `tests/smoke/run.mjs` guards it — a comment in the longer + locale, one unbreakable word and one comment far too long for its share, then every + descendant's rect against its card's and every card's against the page. +- The printout carries the margin bar and the comment list instead (`export/reviewPrint.ts`); + `@media print` hides this layer. + ## Reviewing leader (`ConnectorLayer.svelte`) The dashed leader both word processors draw from an annotated range to its balloon, here diff --git a/src/lib/components/ConnectorLayer.svelte b/src/lib/components/ConnectorLayer.svelte index a964193..5c2075c 100644 --- a/src/lib/components/ConnectorLayer.svelte +++ b/src/lib/components/ConnectorLayer.svelte @@ -63,7 +63,9 @@ if (clip && (c.bottom < clip.top || c.top > clip.bottom || c.left < clip.left || c.left > clip.right)) return; const r = card.getBoundingClientRect(); const ax = c.left - box.left; - const ay = (c.top + c.bottom) / 2 - box.top; + // The bottom of the line box: the leader runs through the gap between two lines, + // where it crosses no letters, as both word processors draw it. + const ay = c.bottom - box.top; const cx = r.left - box.left; const cy = r.top - box.top + Math.min(14, r.height / 2); if (cx <= ax) return; diff --git a/src/lib/components/Editor.svelte b/src/lib/components/Editor.svelte index a64eb5d..51ecccc 100644 --- a/src/lib/components/Editor.svelte +++ b/src/lib/components/Editor.svelte @@ -28,7 +28,9 @@ import PageSheetLayer from './PageSheetLayer.svelte'; import LineNumberLayer from './LineNumberLayer.svelte'; import FoldMarkLayer from './FoldMarkLayer.svelte'; import ChangeBarLayer from './ChangeBarLayer.svelte'; - import { markupView } from '../storage/markup.svelte'; + import ReviewMarginLayer, { REVIEW_MARGIN_CM } from './ReviewMarginLayer.svelte'; + import { visibleComments, visibleRevisions } from './reviewItems'; + import { changesInMargin, commentsInMargin, markupView } from '../storage/markup.svelte'; import { DEFAULT_LINE_NUMBERING, type LineNumbering } from '../storage/lineNumbering'; import { EMPTY_PAGE_DECOR, type PageDecor } from '../storage/pageDecor'; import Ruler from './Ruler.svelte'; @@ -58,7 +60,7 @@ import { EMPTY_PAGE_DECOR, type PageDecor } from '../storage/pageDecor'; let { editor = $bindable(), tick = $bindable(0), currentPage = $bindable(1), numPages = $bindable(1), zoom = 100, onZoom, showFormattingMarks = false, showRuler = true, splitView = false, pageColumns = 1, pageMargins = DEFAULT_MARGINS, orientation = 'portrait', - pageFormat = 'A4', tabIntervalCm = DEFAULT_TAB_INTERVAL_CM, spacingModel = 'add', documentEpoch = 0, pageRtl = false, hyphenate = false, documentLanguage = 'en', pageNumbering = DEFAULT_PAGE_NUMBERING, pageDecor = EMPTY_PAGE_DECOR, lineNumbering = DEFAULT_LINE_NUMBERING, foldMarks = false, + pageFormat = 'A4', tabIntervalCm = DEFAULT_TAB_INTERVAL_CM, spacingModel = 'add', documentEpoch = 0, pageRtl = false, hyphenate = false, documentLanguage = 'en', pageNumbering = DEFAULT_PAGE_NUMBERING, pageDecor = EMPTY_PAGE_DECOR, lineNumbering = DEFAULT_LINE_NUMBERING, foldMarks = false, commentAuthor = '', headerDoc = $bindable(null), footerDoc = $bindable(null), hfDistances = DEFAULT_HF_DISTANCES, headerFirstDoc = $bindable(null), footerFirstDoc = $bindable(null), differentFirstPage = false, headerEvenDoc = $bindable(null), footerEvenDoc = $bindable(null), differentOddEven = false, @@ -76,6 +78,8 @@ import { EMPTY_PAGE_DECOR, type PageDecor } from '../storage/pageDecor'; documentEpoch?: number; /** A right-to-left page: the body's base direction, so its columns fill from the right. */ pageRtl?: boolean; + /** Comments and changes as balloons beside the page, and the author a new one takes. */ + commentAuthor?: string; /** Automatic hyphenation; the browser needs the document language to pick its patterns. */ hyphenate?: boolean; documentLanguage?: string; /** How the page-number field counts (format + start value). */ @@ -429,6 +433,16 @@ import { EMPTY_PAGE_DECOR, type PageDecor } from '../storage/pageDecor'; let scaledWidth = $state(0); let scaledHeight = $state(0); + // The balloon strip is only reserved where there is something to put in it, so a + // document without review markup keeps the page centred exactly as before. + let hasReviewMarkup = $derived.by(() => { + if (tick < 0 || !editor) return false; + const doc = editor.state.doc; + return (changesInMargin() && visibleRevisions(doc).length > 0) + || (commentsInMargin() && visibleComments(doc).some((c) => !c.resolved)); + }); + let reviewArea = $derived(hasReviewMarkup ? cmToPx(REVIEW_MARGIN_CM) : 0); + function recomputeScaledSize() { const paper = papers[0]; if (!paper) return; @@ -788,6 +802,11 @@ import { EMPTY_PAGE_DECOR, type PageDecor } from '../storage/pageDecor'; // Throttle the value actually written to the DOM to one update per animation frame, // so rapid slider events don't trigger 50+ layout/paint cycles per second. let appliedZoom = $state(untrack(() => zoom)); + + // The scroller's footprint holds page + strip; .paper-scaler's `margin: 0 auto` then + // centres the pair, so the page sits left of centre as it does in LibreOffice. + let scaledCanvasWidth = $derived(scaledWidth + Math.round(reviewArea * appliedZoom / 100)); + let zoomRaf: number | null = null; $effect(() => { @@ -1457,8 +1476,11 @@ import { EMPTY_PAGE_DECOR, type PageDecor } from '../storage/pageDecor'; {/if} -
+
{@render paper(i, 0, 0)} + {#if hasReviewMarkup} + + {/if}
{@render chrome(i)}
@@ -1607,11 +1629,14 @@ import { EMPTY_PAGE_DECOR, type PageDecor } from '../storage/pageDecor'; diff --git a/src/lib/utils/balloonStack.ts b/src/lib/utils/balloonStack.ts new file mode 100644 index 0000000..3d3fd4e --- /dev/null +++ b/src/lib/utils/balloonStack.ts @@ -0,0 +1,33 @@ +// Where the margin balloons end up (ReviewMarginLayer.svelte). Each one wants to sit at +// its anchor's line; two anchors a line apart carry balloons far taller than a line, so +// a balloon that would overlap its predecessor is pushed down, as in both word processors. + +/** Tops for balloons in document order: none closer than `gap`, all inside the band. */ +export function stackTops( + desired: number[], + heights: number[], + gap: number, + bandTop = -Infinity, + bandBottom = Infinity, +): number[] { + const out: number[] = []; + let floor = bandTop; + for (const [i, want] of desired.entries()) { + const top = Math.max(want, floor); + out.push(top); + floor = top + (heights[i] ?? 0) + gap; + } + // More cards than the band holds: they keep their distance and run past its end — + // overlapping them would hide the ones underneath. The caller caps heights so this + // stays the rare case. + const total = heights.slice(0, out.length).reduce((s, h) => s + (h ?? 0), 0) + Math.max(0, out.length - 1) * gap; + if (total > bandBottom - bandTop) return out; + // Back up again: the column may not hang past the band, and pulling the last card up + // pulls its predecessors with it. + let ceiling = bandBottom; + for (let i = out.length - 1; i >= 0; i--) { + out[i] = Math.min(out[i], ceiling - (heights[i] ?? 0)); + ceiling = out[i] - gap; + } + return out; +} diff --git a/src/styles/editor.css b/src/styles/editor.css index 12b7c7c..20e3a10 100644 --- a/src/styles/editor.css +++ b/src/styles/editor.css @@ -4,6 +4,10 @@ by App.svelte); extra bottom padding clears the fixed status bar. */ padding: calc(var(--toolbar-overlay-h, 0px) + 1.25rem) 1rem 3rem; overflow: auto; + /* A flex item's automatic minimum is its content: without this a canvas wider than + the window — a page grid, or a page beside the balloon strip — widens the whole app + instead of scrolling here, and the toolbar above ends short of it. */ + min-width: 0; /* Positioning context for the floating table toolbar (TableToolbar.svelte), which is an absolutely-positioned child that scrolls with the content. */ position: relative; @@ -11,8 +15,10 @@ /* Reserves the SCALED footprint of the transformed .paper (a CSS transform takes no layout space of its own), so the scrollbars and horizontal centering reflect - the zoomed page. Its width/height are sized in JS (Editor.svelte). */ + the zoomed page. Its width/height are sized in JS (Editor.svelte). It is also the + origin the margin balloons position in (ReviewMarginLayer.svelte). */ .paper-scaler { + position: relative; margin: 0 auto; } @@ -1112,6 +1118,12 @@ height: auto; } + /* The printout carries the margin bar and the comment list instead + (export/reviewPrint.ts); the balloons are a screen view. */ + .review-margin-layer { + display: none; + } + .paper { width: 100%; max-width: none; diff --git a/tests/smoke/run.mjs b/tests/smoke/run.mjs index 9e3d085..112e51c 100644 --- a/tests/smoke/run.mjs +++ b/tests/smoke/run.mjs @@ -1,6 +1,7 @@ // Browser smoke test: boots the production build in headless Chromium and checks the // paths jsdom cannot — the bundle boots, typing works, autosave survives a reload, -// a corpus document imports and paginates. Fails on any uncaught page error. +// a corpus document imports and paginates, a margin balloon holds its content. Fails on +// any uncaught page error. import { spawn, execSync } from 'node:child_process'; import { existsSync } from 'node:fs'; import { join, resolve, dirname } from 'node:path'; @@ -87,6 +88,93 @@ try { const pdf = await page.evaluate(() => ({ head: window.__pdfHead, size: window.__pdfSize })); check(pdf.head === '%PDF-' && pdf.size > 20_000, `PDF export produces a PDF (${Math.round(pdf.size / 1024)} KB)`); + + // Margin balloons: every card's content stays inside its box and the column inside the + // page. An unwrapped button row once pushed "Edit" clean out of the balloon, and a + // comment of a few paragraphs pushed the cards under it off the sheet. + await page.evaluate(() => { + const p = (text) => ({ type: 'paragraph', content: [{ type: 'text', text }] }); + const m = (text, mark) => ({ type: 'paragraph', content: [{ type: 'text', text, marks: [mark] }] }); + const long = 'Donaudampfschifffahrtsgesellschaftskapitaenspatentpruefungsordnung'; + const essay = Array.from({ length: 16 }, (_, i) => `Absatz ${i + 1} eines sehr langen Kommentars, der ohne Deckel weit ueber das Seitenende hinaus reicht.`).join('\n\n'); + localStorage.setItem('edentext-app-language', 'de'); + localStorage.removeItem('edentext-markup-mode'); + // The flag the previous document's load left behind would send this one to + // edentext-doc-broken and start the app empty. + localStorage.removeItem('edentext-doc-loading'); + localStorage.setItem('edentext-doc', JSON.stringify({ type: 'doc', content: [ + p('Head.'), + m('commented', { type: 'comment', attrs: { id: 'c1', author: 'Test Autor', date: '2026-03-04T05:06:07.000Z', text: 'Bitte kuerzen.', resolved: false, replies: [{ author: 'Zweiter Autor', date: '2026-03-05T05:06:07.000Z', text: 'Einverstanden, gekuerzt.' }] } }), + m(long, { type: 'comment', attrs: { id: 'c2', author: 'Ein-Autor-Ohne-Leerzeichen-Im-Namen', date: '2026-03-04T05:06:07.000Z', text: long, resolved: false } }), + m('lang kommentiert', { type: 'comment', attrs: { id: 'c3', author: 'Test Autor', date: '2026-03-04T05:06:07.000Z', text: essay, resolved: false } }), + m('inserted', { type: 'insertion', attrs: { id: 'r1', author: 'Rev Autor', date: '2026-05-06T07:08:09.000Z' } }), + ...Array.from({ length: 6 }, (_, i) => p(`Absatz ${i + 1}.`)), + ] })); + }); + await page.reload({ waitUntil: 'load' }); + await page.waitForSelector('.tiptap', { timeout: 15_000 }); + await page.waitForFunction(() => document.querySelectorAll('.balloon').length >= 4, + null, { timeout: 15_000 }).catch(() => {}); + // The ⋯ menu opens inside the card: floating, it would be clipped by the balloon. + const menuSpill = async () => page.evaluate(() => { + const out = []; + const cards = [...document.querySelectorAll('.balloon')]; + // A card taller than its share of the page scrolls; every other overflow is a bug. + const layer = document.querySelector('.review-margin-layer')?.getBoundingClientRect(); + for (const b of cards) { + const box = b.getBoundingClientRect(); + const scrolls = b.scrollHeight > b.clientHeight + 1; + if (b.scrollWidth > b.clientWidth + 1) out.push('card clips its content sideways'); + for (const el of b.querySelectorAll('*')) { + const r = el.getBoundingClientRect(); + if (!r.width && !r.height) continue; + if (r.left < box.left - 0.5 || r.right > box.right + 0.5) + out.push(`<${el.tagName.toLowerCase()}> "${el.textContent.trim().slice(0, 16)}" outside the card`); + if (!scrolls && (r.top < box.top - 0.5 || r.bottom > box.bottom + 0.5)) + out.push(`<${el.tagName.toLowerCase()}> "${el.textContent.trim().slice(0, 16)}" below the card`); + } + if (layer && (box.top < layer.top - 0.5 || box.bottom > layer.bottom + 0.5)) out.push('balloon hangs off the page'); + } + const rects = cards.map((e) => e.getBoundingClientRect()); + for (let i = 1; i < rects.length; i++) if (rects[i].top < rects[i - 1].bottom - 0.5) out.push(`balloon ${i} overlaps the one above`); + // The leader belongs in the gap under its line, never across the letters. + const anchor = document.querySelector('[data-comment="c1"]')?.getBoundingClientRect(); + const first = document.querySelector('.review-margin-layer path')?.getAttribute('d'); + if (layer && anchor && first && +first.split(' ')[2] < anchor.bottom - layer.top - 1) + out.push('leader crosses its own line'); + return { count: cards.length, out: [...new Set(out)] }; + }); + // The stack settles on the next frame after a card is measured, so let it. + const settle = () => page.waitForTimeout(300); + await settle(); + const spill = await menuSpill(); + await page.click('.balloon .more'); + await settle(); + const opened = await menuSpill(); + check(spill.count === 4 && spill.out.length === 0 && opened.out.length === 0, + `margin balloons hold their content (${spill.count} balloons${[...spill.out, ...opened.out].length ? ': ' + [...spill.out, ...opened.out].join(', ') : ''})`); + + // The display modes, through the ribbon: no markup takes bars and balloons off the + // page, and the full view brings the same ones back. + const shown = () => page.evaluate(() => ({ + balloons: document.querySelectorAll('.balloon').length, + bars: document.querySelectorAll('.change-bar-layer *').length, + })); + const pickMode = async (label) => { + await page.locator('[role=tab], .ribbon-tabs button').filter({ hasText: /Überprüfen|Review/ }).first().click(); + await page.locator('button', { hasText: /Anzeige für Überprüfung|Display for review/ }).first().click(); + await page.locator('.ribbon-menu button', { hasText: label }).first().click(); + await settle(); + await settle(); + }; + const full = await shown(); + await pickMode(/Kein Markup|No markup/); + const bare = await shown(); + await pickMode(/Alle Markups|All markup/); + const back = await shown(); + check(full.balloons > 0 && full.bars > 0 && bare.balloons === 0 && bare.bars === 0 + && back.balloons === full.balloons && back.bars === full.bars, + `no markup empties the margin and all markup fills it again (${full.balloons}/${full.bars} → ${bare.balloons}/${bare.bars} → ${back.balloons}/${back.bars})`); } catch (err) { check(false, `smoke run threw: ${err.message ?? err}`); } finally { diff --git a/tests/unit/balloon-stack.test.ts b/tests/unit/balloon-stack.test.ts new file mode 100644 index 0000000..43dbedb --- /dev/null +++ b/tests/unit/balloon-stack.test.ts @@ -0,0 +1,28 @@ +import { describe, it, expect } from 'vitest'; +import { stackTops } from '../../src/lib/utils/balloonStack'; + +describe('the margin balloon stack', () => { + it('leaves a balloon at its anchor where there is room', () => { + expect(stackTops([0, 200, 400], [50, 50, 50], 6)).toEqual([0, 200, 400]); + }); + + it('pushes an overlapping balloon below its predecessor', () => { + expect(stackTops([0, 10, 20], [50, 50, 50], 6)).toEqual([0, 56, 112]); + }); + + it('never lifts one above the anchor it belongs to', () => { + expect(stackTops([100, 0], [20, 20], 4)).toEqual([100, 124]); + }); + + it('pulls the column up so it ends inside the page', () => { + expect(stackTops([0, 70], [40, 40], 6, 0, 100)).toEqual([0, 60]); + }); + + it('keeps the cards apart where the page cannot hold them all', () => { + expect(stackTops([10, 20], [50, 50], 6, 0, 80)).toEqual([10, 66]); + }); + + it('holds an empty list', () => { + expect(stackTops([], [], 6)).toEqual([]); + }); +});