feat(magazine): show sidebars in the reading view as labelled, never-narrated asides
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
30b80fcbf7
commit
4a20a12f66
@ -7,6 +7,7 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
|
|||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
### Added
|
### Added
|
||||||
|
- **Magazine reading view shows sidebars.** A sidebar was kept out of narration and therefore never shown at all. The article text endpoint now also returns `asides` and the reflowed view displays each one where it sits, labelled "Sidebar · not read aloud · p. N", as plain text that is neither a sentence nor highlighted. Only for pages the article alone occupies (on a shared page there is no telling which article a sidebar belongs to); not for advertisement pages; a sidebar you ticked into the narration stays narrated.
|
||||||
- **Magazine review: extraction diff.** A region whose text you edited by hand is marked *Edited*, with *Show changes* (word-level comparison against the text as first recognised; removals struck through and marked "−", additions underlined and marked "+") and *Restore recognised text*. The first reading is kept in a new `ocr_text` field across repeated edits; re-reading the region or typing the original back clears it.
|
- **Magazine review: extraction diff.** A region whose text you edited by hand is marked *Edited*, with *Show changes* (word-level comparison against the text as first recognised; removals struck through and marked "−", additions underlined and marked "+") and *Restore recognised text*. The first reading is kept in a new `ocr_text` field across repeated edits; re-reading the region or typing the original back clears it.
|
||||||
- **"Continued from page N" now joins a feature's tail to its head.** Until now only the forward "continued on page N" reference linked pages. A tail that says "Fortsetzung von Seite 10" / "Continued from page 10" (found even when set as small furniture or sidebar text, on lines up to 160 characters) is attached to the single article on that page. Skipped when a contents entry anchors the tail, when the page is shared by several articles, or when the marker points forward or at an empty page. Articles gain a `continued_from` list.
|
- **"Continued from page N" now joins a feature's tail to its head.** Until now only the forward "continued on page N" reference linked pages. A tail that says "Fortsetzung von Seite 10" / "Continued from page 10" (found even when set as small furniture or sidebar text, on lines up to 160 characters) is attached to the single article on that page. Skipped when a contents entry anchors the tail, when the page is shared by several articles, or when the marker points forward or at an empty page. Articles gain a `continued_from` list.
|
||||||
- **Magazine import from PNG/JPEG page images.** Choose or drop several scans or phone photos; they are checked by content (not by name), ordered by filename with natural numbering, turned into one scan-style PDF (page size from the image's own resolution, EXIF rotation applied, 72 dpi treated as unknown) and ingested like any scanned issue. Limits: 300 images, 25 MB and 60 megapixels each; a bad image rejects the whole upload and leaves nothing behind; an existing source is never replaced. Verified on real pages: OCR through the image path scored like the original PDF (F1 0.85 / 0.78).
|
- **Magazine import from PNG/JPEG page images.** Choose or drop several scans or phone photos; they are checked by content (not by name), ordered by filename with natural numbering, turned into one scan-style PDF (page size from the image's own resolution, EXIF rotation applied, 72 dpi treated as unknown) and ingested like any scanned issue. Limits: 300 images, 25 MB and 60 megapixels each; a bad image rejects the whole upload and leaves nothing behind; an existing source is never replaced. Verified on real pages: OCR through the image path scored like the original PDF (F1 0.85 / 0.78).
|
||||||
|
|||||||
6
TODO.md
6
TODO.md
@ -680,9 +680,9 @@ casting. The original PDF remains the visual source of truth while approved arti
|
|||||||
|
|
||||||
### P1 — Magazine listening experience
|
### P1 — Magazine listening experience
|
||||||
|
|
||||||
- [~] Generate clean article HTML with headings, captions, figures, sidebars, and page links.
|
- [x] Generate clean article HTML with headings, captions, figures, sidebars, and page links.
|
||||||
*(headings/byline/captions styled and page-linked, figures now shown during playback; sidebars are auto-excluded
|
*(headings/byline/captions styled and page-linked, figures shown during playback, and (2026-09-29) sidebars shown in
|
||||||
from narration rather than rendered inline, per the P2 sidebar-exclusion item below)*
|
place as labelled, never-narrated asides)*
|
||||||
- [x] Generate narrator audio per article with resume/progress and a play-all queue.
|
- [x] Generate narrator audio per article with resume/progress and a play-all queue.
|
||||||
- [x] Synchronize paragraph/sentence highlighting with playback and keep PDF and article views linked.
|
- [x] Synchronize paragraph/sentence highlighting with playback and keep PDF and article views linked.
|
||||||
*(sentence-level; clicking a sentence or switching to the original-page view jumps to its source bbox)*
|
*(sentence-level; clicking a sentence or switching to the original-page view jumps to its source bbox)*
|
||||||
|
|||||||
@ -283,6 +283,15 @@ it can drift on a sentence with heavy punctuation-driven pauses. A checkbox
|
|||||||
(measured, not estimated — it just waits for `audio.onended`) always still
|
(measured, not estimated — it just waits for `audio.onended`) always still
|
||||||
works regardless.
|
works regardless.
|
||||||
|
|
||||||
|
### Sidebars in the reading view
|
||||||
|
|
||||||
|
`article_asides(layout, article, all_articles)` lists the sidebar regions on an article's own pages that are not part of
|
||||||
|
its narrated blocks; `GET .../articles/{id}/text` returns them as `asides` (`id, page, bbox, text, after`), where `after`
|
||||||
|
is the last narrated region before the sidebar in reading order (None = before the first on its page). A page that any
|
||||||
|
other article also lists is skipped, as are advertisement pages and empty boxes; jump cues are stripped. The client
|
||||||
|
inserts each as an `<aside>` after the sentence of region `after`, as escaped text, outside the sentence list, so nothing
|
||||||
|
about narration, highlighting, seeking or progress changes.
|
||||||
|
|
||||||
### Driving mode
|
### Driving mode
|
||||||
|
|
||||||
`#mag-driving` is a full-screen overlay, not a separate playback path: it
|
`#mag-driving` is a full-screen overlay, not a separate playback path: it
|
||||||
|
|||||||
@ -4992,6 +4992,39 @@ def page_is_advertisement(layout: dict[str, Any], page_no: int) -> bool:
|
|||||||
and not any(b.get("type") in NARRATABLE for b in on_page))
|
and not any(b.get("type") in NARRATABLE for b in on_page))
|
||||||
|
|
||||||
|
|
||||||
|
def article_asides(layout: dict[str, Any], article: dict[str, Any],
|
||||||
|
all_articles: Sequence[dict[str, Any]]) -> list[dict[str, Any]]:
|
||||||
|
"""Sidebars lying on this article's pages, to show beside the text but never narrate.
|
||||||
|
|
||||||
|
A sidebar is kept out of narration (mark_sidebars), which also keeps it out of the article's own block
|
||||||
|
list -- so the reading view never showed it. Only pages this article alone occupies are used: on a page
|
||||||
|
shared by several articles there is no telling which one a sidebar belongs to, and showing it in the
|
||||||
|
wrong one is worse than not showing it. Advertisement pages carry none. Each entry says which of the
|
||||||
|
article's narrated regions it follows (`after`, or None = before the first on its page).
|
||||||
|
"""
|
||||||
|
mine = {int(p) for p in article.get("pages") or []}
|
||||||
|
shared = {int(p) for a in all_articles if a.get("id") != article.get("id") for p in a.get("pages") or []}
|
||||||
|
pages = {p for p in mine - shared if not page_is_advertisement(layout, p)}
|
||||||
|
narrated = set(article.get("blocks") or [])
|
||||||
|
blocks = layout.get("blocks", [])
|
||||||
|
out = []
|
||||||
|
for b in blocks:
|
||||||
|
if b.get("type") != BLOCK_SIDEBAR or b.get("page") not in pages or b.get("id") in narrated:
|
||||||
|
continue
|
||||||
|
text = strip_continuation_markers(str(b.get("text") or "")).strip()
|
||||||
|
if not text:
|
||||||
|
continue
|
||||||
|
before = [x for x in blocks if x.get("page") == b["page"] and x.get("id") in narrated
|
||||||
|
and (x.get("reading_order") or 0) < (b.get("reading_order") or 0)]
|
||||||
|
after = max(before, key=lambda x: x.get("reading_order") or 0)["id"] if before else None
|
||||||
|
out.append({"id": b["id"], "page": b["page"], "bbox": b.get("bbox"), "text": text[:4000],
|
||||||
|
"after": after, "_order": (b["page"], b.get("reading_order") or 0)})
|
||||||
|
out.sort(key=lambda a: a["_order"])
|
||||||
|
for a in out:
|
||||||
|
del a["_order"]
|
||||||
|
return out
|
||||||
|
|
||||||
|
|
||||||
def article_text(layout: dict[str, Any], article: dict[str, Any],
|
def article_text(layout: dict[str, Any], article: dict[str, Any],
|
||||||
include_captions: bool = False,
|
include_captions: bool = False,
|
||||||
include_descriptions: bool = False,
|
include_descriptions: bool = False,
|
||||||
|
|||||||
@ -678,7 +678,8 @@ async def magazine_article_text(doc_id: str, article_id: str,
|
|||||||
include_captions=bool(captions),
|
include_captions=bool(captions),
|
||||||
include_descriptions=bool(descriptions),
|
include_descriptions=bool(descriptions),
|
||||||
fuse=True, glossary=target.get("glossary"))
|
fuse=True, glossary=target.get("glossary"))
|
||||||
return {"article": target, "blocks": blocks}
|
layout = mag.load_layout(d)
|
||||||
|
return {"article": target, "blocks": blocks, "asides": mag.article_asides(layout, target, articles)}
|
||||||
|
|
||||||
|
|
||||||
@router.post("/api/magazine/{doc_id}/articles/auto-link")
|
@router.post("/api/magazine/{doc_id}/articles/auto-link")
|
||||||
|
|||||||
@ -21,6 +21,7 @@ const state = {
|
|||||||
article: null, // the article being read or reviewed
|
article: null, // the article being read or reviewed
|
||||||
sentences: [], // [{ text, blockId, page, bbox }]
|
sentences: [], // [{ text, blockId, page, bbox }]
|
||||||
images: [], // current article's figures, in reading order
|
images: [], // current article's figures, in reading order
|
||||||
|
asides: [], // sidebars shown beside the text (not narrated), see article_asides
|
||||||
view: 'library', // library | contents | review | read
|
view: 'library', // library | contents | review | read
|
||||||
contentsView: 'list', // list | pages — how the article list itself is shown
|
contentsView: 'list', // list | pages — how the article list itself is shown
|
||||||
contentsSelectMode: false, // batch-select articles for bulk approve/reject
|
contentsSelectMode: false, // batch-select articles for bulk approve/reject
|
||||||
@ -4233,6 +4234,7 @@ async function magRead(articleId, opts) {
|
|||||||
($('mag-read-captions')?.checked ? 1 : 0) + '&descriptions=' +
|
($('mag-read-captions')?.checked ? 1 : 0) + '&descriptions=' +
|
||||||
($('mag-read-descriptions')?.checked ? 1 : 0));
|
($('mag-read-descriptions')?.checked ? 1 : 0));
|
||||||
state.sentences = splitSentences(data.blocks || []);
|
state.sentences = splitSentences(data.blocks || []);
|
||||||
|
state.asides = Array.isArray(data.asides) ? data.asides : [];
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
return say('Could not load article text: ' + e.message, 'err');
|
return say('Could not load article text: ' + e.message, 'err');
|
||||||
}
|
}
|
||||||
@ -4254,7 +4256,22 @@ function renderArticle() {
|
|||||||
$('mag-read-by').textContent = a.author ? 'By ' + a.author : '';
|
$('mag-read-by').textContent = a.author ? 'By ' + a.author : '';
|
||||||
$('mag-read-pages').textContent = fmtPages(a.pages);
|
$('mag-read-pages').textContent = fmtPages(a.pages);
|
||||||
|
|
||||||
host.innerHTML = state.sentences.map((s, i) => {
|
// Sidebars are shown where they sit in the article but are not sentences: never narrated, never highlighted.
|
||||||
|
const asideHtml = (a2) => '<aside class="mag-aside" aria-label="Sidebar, not read aloud">' +
|
||||||
|
'<span class="mag-aside-label"><span class="mdi mdi-page-layout-sidebar-right" aria-hidden="true"></span> Sidebar · not read aloud · p. ' +
|
||||||
|
Number(a2.page) + '</span><span class="mag-aside-text">' + esc(a2.text) + '</span></aside>';
|
||||||
|
const asidesAfter = new Map();
|
||||||
|
let asidesFirst = '';
|
||||||
|
(state.asides || []).forEach((a2) => {
|
||||||
|
let at = a2.after ? state.sentences.map((x) => x.blockId).lastIndexOf(a2.after) : -1;
|
||||||
|
if (at < 0) {
|
||||||
|
// Not after a narrated region on its page: after the last sentence from an earlier page, else first.
|
||||||
|
for (let k = state.sentences.length - 1; k >= 0; k--) if (state.sentences[k].page < a2.page) { at = k; break; }
|
||||||
|
}
|
||||||
|
if (at < 0) asidesFirst += asideHtml(a2);
|
||||||
|
else asidesAfter.set(at, (asidesAfter.get(at) || '') + asideHtml(a2));
|
||||||
|
});
|
||||||
|
host.innerHTML = asidesFirst + state.sentences.map((s, i) => {
|
||||||
const tag = s.type === 'title' ? 'h2' : s.type === 'deck' ? 'p class="mag-deck"'
|
const tag = s.type === 'title' ? 'h2' : s.type === 'deck' ? 'p class="mag-deck"'
|
||||||
: s.type === 'byline' ? 'p class="mag-byline"' : s.type === 'caption'
|
: s.type === 'byline' ? 'p class="mag-byline"' : s.type === 'caption'
|
||||||
? 'p class="mag-caption"' : s.type === 'figure_description'
|
? 'p class="mag-caption"' : s.type === 'figure_description'
|
||||||
@ -4273,7 +4290,7 @@ function renderArticle() {
|
|||||||
return '<span class="mag-w" data-w="' + j + '">' + esc(w) + '</span>' + marker;
|
return '<span class="mag-w" data-w="' + j + '">' + esc(w) + '</span>' + marker;
|
||||||
}).join(' ');
|
}).join(' ');
|
||||||
return '<' + tag + ' class="mag-s' + (open === 'span' ? '' : '') + '" data-i="' + i + '" ' +
|
return '<' + tag + ' class="mag-s' + (open === 'span' ? '' : '') + '" data-i="' + i + '" ' +
|
||||||
'onclick="magSeek(' + i + ')">' + wordsHtml + '</' + open + '> ';
|
'onclick="magSeek(' + i + ')">' + wordsHtml + '</' + open + '> ' + (asidesAfter.get(i) || '');
|
||||||
}).join('');
|
}).join('');
|
||||||
highlight(state.idx);
|
highlight(state.idx);
|
||||||
}
|
}
|
||||||
|
|||||||
@ -5567,3 +5567,9 @@ details.mag-report-group summary { cursor: pointer; margin-bottom: 6px; }
|
|||||||
.mag-diff ins { text-decoration: underline; color: var(--green); }
|
.mag-diff ins { text-decoration: underline; color: var(--green); }
|
||||||
.mag-diff del::before { content: "\2212\00a0"; }
|
.mag-diff del::before { content: "\2212\00a0"; }
|
||||||
.mag-diff ins::before { content: "+\00a0"; }
|
.mag-diff ins::before { content: "+\00a0"; }
|
||||||
|
|
||||||
|
/* Reading view: sidebars shown in place but not narrated */
|
||||||
|
.mag-aside { display: block; margin: 12px 0; padding: 10px 14px; border-left: 4px solid var(--border);
|
||||||
|
background: var(--surface); border-radius: 0 var(--radius) var(--radius) 0; color: var(--subtext); font-size: .92em; }
|
||||||
|
.mag-aside-label { display: block; font-size: 12px; font-weight: 600; margin-bottom: 4px; }
|
||||||
|
.mag-aside-text { display: block; white-space: pre-line; overflow-wrap: anywhere; }
|
||||||
|
|||||||
@ -1505,3 +1505,32 @@ test.describe('Edited text in review', () => {
|
|||||||
expect(r.big).toBe(true);
|
expect(r.big).toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('sidebars appear in the reading view in place, labelled, and are not sentences', async ({ page }) => {
|
||||||
|
await mockBackend(page, { articleStatus: 'approved' });
|
||||||
|
await page.route(`**/api/magazine/${DOC_ID}/articles/article-001/text*`, route => route.fulfill({
|
||||||
|
json: { article: ARTICLE_APPROVED, blocks: LAYOUT.blocks, asides: [
|
||||||
|
{ id: 's1', page: 3, text: 'Box: <b>Did you know?</b> Lithium is light.', after: 'b2' },
|
||||||
|
{ id: 's2', page: 3, text: 'Glossary of terms', after: 'b3' },
|
||||||
|
{ id: 's0', page: 3, text: 'Before everything', after: null },
|
||||||
|
] } }));
|
||||||
|
await page.goto('/', { waitUntil: 'domcontentloaded' });
|
||||||
|
await page.waitForSelector('#app-skeleton', { state: 'detached', timeout: 15_000 });
|
||||||
|
await navTo(page, 's-magazine');
|
||||||
|
await page.locator('#mag-library-list .mag-card').first().click();
|
||||||
|
await page.locator('.mag-toc-row.is-approved').getByRole('button', { name: /Listen|Continue/ }).click();
|
||||||
|
await expect(page.locator('#mag-view-read')).toBeVisible();
|
||||||
|
|
||||||
|
await expect(page.locator('#mag-article [data-i]')).toHaveCount(4); // asides add no sentences
|
||||||
|
const asides = page.locator('#mag-article aside.mag-aside');
|
||||||
|
await expect(asides).toHaveCount(3);
|
||||||
|
await expect(asides.first()).toContainText('Before everything');
|
||||||
|
await expect(asides.first()).toHaveAttribute('aria-label', 'Sidebar, not read aloud');
|
||||||
|
await expect(asides.nth(1)).toContainText('Did you know?');
|
||||||
|
await expect(page.locator('#mag-article aside b')).toHaveCount(0); // text, never markup
|
||||||
|
// Order in the DOM: before-all aside, title/byline sentences, aside after the byline, body sentences, aside last.
|
||||||
|
const order = await page.locator('#mag-article').evaluate(el => Array.from(el.children).map(c => c.matches('aside') ? 'A' : 'S').join(''));
|
||||||
|
expect(order).toBe('ASSASSA');
|
||||||
|
await asides.nth(1).click();
|
||||||
|
await expect(page.locator('#mag-article [data-i].is-current')).toHaveCount(1);
|
||||||
|
});
|
||||||
|
|||||||
@ -1051,6 +1051,50 @@ class CleanUpBoxesTests(unittest.TestCase):
|
|||||||
shutil.rmtree(d, ignore_errors=True)
|
shutil.rmtree(d, ignore_errors=True)
|
||||||
|
|
||||||
|
|
||||||
|
class ArticleAsidesTests(unittest.TestCase):
|
||||||
|
"""Sidebars are shown beside an article's text but are never part of what is narrated."""
|
||||||
|
|
||||||
|
def layout(self):
|
||||||
|
def blk(i, page, typ, text, order):
|
||||||
|
return {"id": i, "page": page, "bbox": [0, 0, 10, 10], "type": typ, "text": text, "reading_order": order}
|
||||||
|
return {"pages": [{"page": 5}, {"page": 6}, {"page": 7}], "images": [], "blocks": [
|
||||||
|
blk("t", 5, "title", "Headline", 0), blk("p1", 5, "body", "First paragraph.", 1),
|
||||||
|
blk("s1", 5, "sidebar", "Box: did you know?", 2), blk("p2", 5, "body", "Second paragraph.", 3),
|
||||||
|
blk("s2", 6, "sidebar", "Glossary (continued on page 9)", 0),
|
||||||
|
blk("ad", 7, "sidebar", "Shared page box", 0),
|
||||||
|
]}
|
||||||
|
|
||||||
|
def article(self, aid="a1", pages=(5, 6), blocks=("t", "p1", "p2")):
|
||||||
|
return {"id": aid, "pages": list(pages), "blocks": list(blocks)}
|
||||||
|
|
||||||
|
def test_sidebars_on_the_articles_own_pages_follow_the_right_block(self):
|
||||||
|
from core.magazine import article_asides
|
||||||
|
a = self.article()
|
||||||
|
out = article_asides(self.layout(), a, [a])
|
||||||
|
self.assertEqual([(x["id"], x["after"]) for x in out], [("s1", "p1"), ("s2", None)])
|
||||||
|
self.assertEqual(out[1]["text"], "Glossary") # the jump cue is print furniture, not content
|
||||||
|
|
||||||
|
def test_a_page_shared_with_another_article_shows_no_sidebar(self):
|
||||||
|
from core.magazine import article_asides
|
||||||
|
a, b = self.article(), self.article("a2", pages=(5,), blocks=())
|
||||||
|
self.assertEqual([x["id"] for x in article_asides(self.layout(), a, [a, b])], ["s2"])
|
||||||
|
|
||||||
|
def test_a_sidebar_the_reader_ticked_in_is_narrated_not_an_aside(self):
|
||||||
|
from core.magazine import article_asides
|
||||||
|
a = self.article(blocks=("t", "p1", "s1", "p2"))
|
||||||
|
self.assertEqual([x["id"] for x in article_asides(self.layout(), a, [a])], ["s2"])
|
||||||
|
|
||||||
|
def test_advertisement_pages_and_empty_sidebars_are_skipped(self):
|
||||||
|
from core.magazine import article_asides
|
||||||
|
layout = self.layout()
|
||||||
|
layout["pages"][1]["advertisement"] = True
|
||||||
|
a = self.article()
|
||||||
|
self.assertEqual([x["id"] for x in article_asides(layout, a, [a])], ["s1"])
|
||||||
|
layout = self.layout()
|
||||||
|
layout["blocks"][2]["text"] = " "
|
||||||
|
self.assertEqual([x["id"] for x in article_asides(layout, a, [a])], ["s2"])
|
||||||
|
|
||||||
|
|
||||||
class QuickCorrectionTests(unittest.TestCase):
|
class QuickCorrectionTests(unittest.TestCase):
|
||||||
"""Edit text, merge with the next region, split a region -- from the review list."""
|
"""Edit text, merge with the next region, split a region -- from the review list."""
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user