From 1137b0d90dee5f503e787c9d64aecf69f608fb8b Mon Sep 17 00:00:00 2001 From: hectorqin Date: Tue, 27 Oct 2026 09:24:49 +0000 Subject: [PATCH] fix(web): paged mode lands on page boundaries; highlights draw and follow reflow; bottom bar stays visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Paged mode - Restoring the position after the viewport height changes (address bar, keyboard, rotation) wrote max * fraction, which is almost never a column boundary: the screen showed half of one page and half of the next. Every paged scrollLeft write (restore, TTS follow, revealBlock, search) now goes through one page-snapping helper, and a scroll the browser makes on its own (focus, selection autoscroll) is snapped back. Highlights - localSectionId prefixed `xhtml:` from the format alone, so a windowed book whose sections have no prefix saved highlights that matched no chapter and were never drawn (and its progress failed to restore). The loaded document's own section ids are now the authority. - Highlight boxes are measured once; font size, margins, mode switch, resize or a late image moved the text under them. They are repainted after each. Mobile bottom bar - `.reader-route-host` had min-height: 100vh, which on a phone browser is taller than the visible viewport while the address bar shows, pushing the reader's bottom toolbar under it. Height is 100dvh with a 100vh fallback. - Notices are bottom-centre, exactly over 朗读/外观; in the reader they now stack above the bottom bar instead of swallowing the next tap. Tap to turn hides the toolbar - A tap on the outer thirds turns the page and puts a showing toolbar away; the middle third still toggles it. UI review scripts: page-turn-regression waits for login to finish rather than for the shelf (the app now opens on media); reader-regression shows the toolbar before pressing 下一章 after tap turns. Co-Authored-By: Claude Opus 5.5 --- web/src/app/app.css | 6 +- web/src/styles/notifications.css | 10 ++ web/src/ui/reader-screen.tsx | 19 +++- web/src/ui/reader-view.ts | 102 +++++++++++++++++-- web/tools/ui-review/page-turn-regression.mjs | 2 +- web/tools/ui-review/reader-regression.mjs | 4 + 6 files changed, 128 insertions(+), 15 deletions(-) diff --git a/web/src/app/app.css b/web/src/app/app.css index 45317e4..d11b1fc 100644 --- a/web/src/app/app.css +++ b/web/src/app/app.css @@ -1,2 +1,6 @@ .media-page { max-width: 1440px; margin-inline: auto; padding-bottom: 7rem; } -.reader-route-host { display: flex; flex-direction: column; height: 100dvh; min-height: 100vh; overflow: hidden; } +/* The *visible* viewport, never the largest one. A phone browser's 100vh is the height + with its address bar hidden, so `min-height: 100vh` made the reader taller than the + screen whenever the bar was showing and put the reader's own bottom toolbar under it. + 100vh stays only as the fallback for a WebView that predates dvh. */ +.reader-route-host { display: flex; flex-direction: column; height: 100vh; height: 100dvh; overflow: hidden; } diff --git a/web/src/styles/notifications.css b/web/src/styles/notifications.css index ada187d..1e024ed 100644 --- a/web/src/styles/notifications.css +++ b/web/src/styles/notifications.css @@ -4,6 +4,16 @@ body .notyf { gap: .5rem; font: inherit; } +/* In the reader, notices stack above its bottom bar, not on it. Bottom-centre is + exactly where the 朗读/外观 buttons are, so a "已恢复到上次阅读位置" notice and its + dismiss button sat over them and swallowed the next tap. The heights are the + footer's own (`--reader-footer-height` in reader.css), and the 1.8rem strip + when the chrome is hidden. */ +body:has(.reader-screen) .notyf { padding-bottom: calc(1.8rem + env(safe-area-inset-bottom) + .5rem); } +body:has(.reader-screen[data-chrome='visible']) .notyf { padding-bottom: calc(8.5rem + env(safe-area-inset-bottom) + .5rem); } +@media (min-width: 64rem) { + body:has(.reader-screen[data-chrome='visible']) .notyf { padding-bottom: calc(5.5rem + .5rem); } +} .notyf .global-notice { width: fit-content; max-width: min(28rem, calc(100vw - 2rem)); diff --git a/web/src/ui/reader-screen.tsx b/web/src/ui/reader-screen.tsx index 3f7a384..fff65bf 100644 --- a/web/src/ui/reader-screen.tsx +++ b/web/src/ui/reader-screen.tsx @@ -647,6 +647,14 @@ export class ReaderScreen { } } private localSectionId(id: string): string { + // The loaded document is the authority on its own section ids. Guessing the + // `xhtml:` prefix from the format alone is wrong for a windowed book whose ids + // never had one, and a highlight whose id gains a prefix the chapter does not + // have matches no chapter: saved, synced, and never drawn. + const bare = id.replace(/^xhtml:/, ''); + const sections = this.doc?.sections ?? []; + const known = sections.find(section => section.id === id) ?? sections.find(section => section.id.replace(/^xhtml:/, '') === bare); + if (known) return known.id; if (!this.content && this.book?.format === 'epub') return id.replace(/^xhtml:/, ''); if (this.content && this.book?.format === 'epub' && !id.startsWith('xhtml:')) return 'xhtml:' + id; return id; @@ -1187,6 +1195,12 @@ export class ReaderScreen { * So the two gestures are now disjoint, one meaning each, which is the property * the old version lacked: 左/右 = 翻页, 中 = 显隐工具栏. Nothing is revealed by a * page turn, and nothing is turned by a tap that asked for the controls. + * + * One step further, by request ("点击翻页时自动隐藏工具栏"): a page turn *hides* + * a toolbar that is showing. A reader who turns the page has gone back to reading, + * and the bars would otherwise sit over the page they just turned to until they + * found the middle third. Hiding never moves the text — the bars float over a + * constant page (see `.reader-screen` in reader.css) — so this costs no reflow. */ private onTapZone(zone: 'previous' | 'toggle-chrome' | 'next'): void { if (this.chrome.tocOpen || this.chrome.settingsOpen) return; @@ -1196,12 +1210,13 @@ export class ReaderScreen { return; } - // The page turn, and nothing else: the chrome's visibility is the middle - // third's business alone (see the doc comment above). `tapZone` decides which + // The page turn, which also puts a showing toolbar away (see the doc comment + // above); only the middle third brings it back. `tapZone` decides which // physical side is *forward*, so a left-handed reader can put next-page under // the thumb that holds the phone. const reversed = this.settings.tapZone === 'reversed'; const forward = reversed ? zone === 'previous' : zone === 'next'; + if (this.chromeVisible) this.setChromeVisible(false); void this.turnPage(forward ? 'next' : 'previous'); } diff --git a/web/src/ui/reader-view.ts b/web/src/ui/reader-view.ts index 3f1bc22..86ae8a8 100644 --- a/web/src/ui/reader-view.ts +++ b/web/src/ui/reader-view.ts @@ -206,6 +206,18 @@ export class ReaderView { * value it would write is the one already there. */ private trimApplied: number | null = null; + /** + * What the highlight boxes were painted from, kept so they can be painted again. + * + * A highlight is a set of absolutely positioned boxes measured from the text's + * rects at one moment. Anything that re-flows the text — font size, margins, + * line height, switching between scroll and paged, a rotation, a picture + * finishing loading — moves the text and leaves the boxes where they were, over + * the wrong words. So the anchors are kept and re-measured after every reflow + * (`repaintMarks`). + */ + private noteAnchors: Array<{ id?: string; anchor: TextAnchor; color: string }> = []; + private searchAnchor: TextAnchor | null = null; constructor(options: ReaderViewOptions) { this.options = options; @@ -221,6 +233,12 @@ export class ReaderView { this.resolver = new ResourceResolver(options.doc.resources); this.container.append(this.host); this.bindChapterLinks(); + this.host.flow.addEventListener('scroll', () => this.snapStrayScroll(), { passive: true }); + // `load` does not bubble, so it is caught on the way down. A picture that + // arrives after the highlights were painted pushes the text below it. + this.host.flow.addEventListener('load', (event) => { + if (event.target instanceof HTMLImageElement) this.repaintMarks(); + }, true); let timer: ReturnType | undefined; const changed = () => { clearTimeout(timer); @@ -421,7 +439,10 @@ export class ReaderView { this.host.style.direction = this.settings.direction; // Toggling pagination changes the column count, so the stored offset has to // be re-applied after the browser has laid out the new column box. - requestAnimationFrame(() => this.restoreOffset()); + requestAnimationFrame(() => { + this.restoreOffset(); + this.repaintMarks(); + }); // A page turn is the only place an animation is wanted, and only the two // settings that change where a page *is* can start one. @@ -523,6 +544,9 @@ export class ReaderView { if (this.staged) await this.staged.loadSection(index); this.sectionIndex = index; this.sectionOffset = Math.min(1, Math.max(0, offset)); + // The search box belongs to the chapter it was found in; a new chapter's + // content has already dropped the box itself. + this.searchAnchor = null; this.releaseObjectUrl(); if (this.doc.layout === 'fixed') { @@ -585,6 +609,7 @@ export class ReaderView { const range = anchorRange(this.host.flow, anchor); if (!range) return false; this.revealRange(range); + this.searchAnchor = anchor; this.paintRange(range, '#f5a623', 'search'); return true; } @@ -595,6 +620,7 @@ export class ReaderView { } paintAnnotations(anchors: Array<{ id?: string; anchor: TextAnchor; color: string }>): void { + this.noteAnchors = anchors; for (const mark of this.host.flow.querySelectorAll('[data-reader-mark="note"]')) mark.remove(); const sectionId = this.doc.sections[this.sectionIndex]?.id; for (const { id, anchor, color } of anchors) { @@ -604,6 +630,16 @@ export class ReaderView { } } + /** Re-measures every highlight box against the text as it is laid out now. */ + private repaintMarks(): void { + if (this.doc.layout === 'fixed') return; + this.paintAnnotations(this.noteAnchors); + const search = this.searchAnchor; + if (!search || this.doc.sections[this.sectionIndex]?.id !== search.sectionId) return; + const range = anchorRange(this.host.flow, search); + if (range) this.paintRange(range, '#f5a623', 'search'); + } + private paintRange(range: Range, color: string, kind: string, id?: string): void { if (kind === 'search') for (const node of this.host.flow.querySelectorAll('[data-reader-mark="search"]')) node.remove(); const box = this.host.flow.getBoundingClientRect(); @@ -623,10 +659,7 @@ export class ReaderView { const rect = rangeRect(range); if (rect) { const scroller = this.scroller, box = scroller.getBoundingClientRect(); - if (this.settings.mode === 'paged') { - const stride = this.columnStride(this.columnCount()); - scroller.scrollLeft = Math.floor((scroller.scrollLeft + rect.left - box.left) / stride) * stride; - } + if (this.settings.mode === 'paged') this.scrollToColumnAt(scroller.scrollLeft + rect.left - box.left); else scroller.scrollTop += rect.top - box.top - box.height / 3; } this.emitPosition(); @@ -1033,8 +1066,12 @@ export class ReaderView { // exist: a trim applied afterwards would move the reader by the trim. this.syncPageTrim(); if (this.settings.mode === 'paged') { - const max = Math.max(0, scroller.scrollWidth - scroller.clientWidth); - scroller.scrollLeft = max * this.sectionOffset; + // The fraction becomes a page *index* before it becomes an offset: the column + // count may have changed since the fraction was measured, and `max * fraction` + // is then almost never a column boundary (see scrollToColumnAt). + const columns = this.columnCount(); + const page = Math.round(this.sectionOffset * (columns - 1)); + scroller.scrollLeft = page * this.columnStride(columns); return; } const limit = Math.max(0, scroller.scrollHeight - scroller.clientHeight); @@ -1065,6 +1102,7 @@ export class ReaderView { // A rotation changes the viewport, and the page's line grid with it: the trim is // recomputed before the offset is restored for the same reason as everywhere else. this.restoreOffset(); + this.repaintMarks(); this.emitPosition(); }); this.resizeObserver.observe(this.host); @@ -1443,6 +1481,47 @@ export class ReaderView { return Math.max(1, this.scroller.clientWidth); } + /** + * Lands paged mode on the page that contains `x`, a horizontal offset into the + * chapter's column strip. + * + * Every paged-mode write of `scrollLeft` goes through here, because an offset + * that is not a column boundary is a screen showing the right half of one page + * and the left half of the next. The writes that skipped this — restoring a + * fraction after the viewport changed height, centring a spoken sentence — + * are what produced the half-and-half pages: a phone changes the viewport's + * height every time its address bar or keyboard moves, and a different height + * is a different column count, so `max * fraction` was almost never a boundary. + */ + private scrollToColumnAt(x: number): void { + const columns = this.columnCount(); + const stride = this.columnStride(columns); + // A hair of tolerance so an offset measured a sub-pixel short of a boundary + // still lands on the page that starts there, not on the one before it. + const page = Math.min(columns - 1, Math.max(0, Math.floor((x + 1) / stride))); + this.scroller.scrollLeft = page * stride; + } + + /** + * Puts paged mode back on a column boundary after the browser scrolled it. + * + * The column strip is `overflow: hidden`, which stops the reader from scrolling it + * but not the browser: focusing a highlight's button, a find-in-page match, or + * selection autoscroll all move it to wherever the element happens to be. The + * page turns would then step from that offset, keeping the reader between two + * pages for the rest of the chapter. + */ + private snapStrayScroll(): void { + if (this.settings.mode !== 'paged' || this.doc.layout === 'fixed') return; + const scroller = this.scroller; + const stride = this.columnStride(this.columnCount()); + const offset = scroller.scrollLeft; + const off = offset - Math.round(offset / stride) * stride; + if (Math.abs(off) <= 1) return; + this.scrollToColumnAt(Math.round(offset / stride) * stride); + this.emitPosition(); + } + // ---- read aloud ---- /** @@ -1557,9 +1636,10 @@ export class ReaderView { if (this.settings.mode === 'paged') { // In paged mode the page is found by measurement, not by `scrollIntoView`, // because the browser's idea of "into view" is a horizontal jump to a - // column boundary that ignores the chapter's own padding. - const target = flow.scrollLeft + rect.left - flow.getBoundingClientRect().left - flow.clientWidth / 2; - flow.scrollLeft = Math.max(0, target); + // column boundary that ignores the chapter's own padding. The sentence's + // own page, not the sentence centred: centring is an offset between pages. + const x = flow.scrollLeft + rect.left - flow.getBoundingClientRect().left; + if (x < flow.scrollLeft || x >= flow.scrollLeft + flow.clientWidth) this.scrollToColumnAt(x); return; } const hostRect = flow.getBoundingClientRect(); @@ -1627,7 +1707,7 @@ export class ReaderView { if (!(block instanceof HTMLElement)) return; if (this.settings.mode === 'paged') { const flow = this.host.flow; - flow.scrollLeft = Math.max(0, block.offsetLeft - flow.clientWidth / 2); + this.scrollToColumnAt(flow.scrollLeft + block.getBoundingClientRect().left - flow.getBoundingClientRect().left); return; } const flow = this.host.flow; diff --git a/web/tools/ui-review/page-turn-regression.mjs b/web/tools/ui-review/page-turn-regression.mjs index 6a83cb6..1114407 100644 --- a/web/tools/ui-review/page-turn-regression.mjs +++ b/web/tools/ui-review/page-turn-regression.mjs @@ -11,7 +11,7 @@ try { await page.locator('input[autocomplete=username]').fill('review'); await page.locator('input[type=password]').fill('password12'); await page.locator('button[type=submit]').click(); - await page.locator('.shelf-screen').waitFor(); + await page.locator('input[type=password]').waitFor({ state: 'detached' }); for (const width of [1366, 390]) { await page.setViewportSize({width,height:768}); await page.goto(base + '/#/shelf'); diff --git a/web/tools/ui-review/reader-regression.mjs b/web/tools/ui-review/reader-regression.mjs index 89656be..a44bffa 100644 --- a/web/tools/ui-review/reader-regression.mjs +++ b/web/tools/ui-review/reader-regression.mjs @@ -120,6 +120,10 @@ try { await cdp.sleep(300); assert.equal((await read()).page,'2','reverse tap returns to same page'); await cdp.screenshot(join(shots, 'paged.png')); + // A page-turn tap puts the toolbar away; the middle third brings it back. + assert.equal((await read()).chrome, 'hidden', 'tap page turn hides the toolbar'); + await cdp.tapMiddle(); + await cdp.sleep(200); await cdp.clickText('.footer button', '下一章'); await cdp.sleep(350); assert.equal((await read()).page,'1','next chapter starts at page one');