From e5a055b4947cf2263348dc1fe231efdb050708c9 Mon Sep 17 00:00:00 2001 From: Jiminy Panoz Date: Wed, 22 Jul 2026 17:53:49 +0200 Subject: [PATCH 1/3] Correct id monitoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When navigating id, we place it at the center of the screen, but timeline used other references so it was never properly sync’d in that case. --- .../src/modules/snapper/CJKVerticalSnapper.ts | 14 +++++++++---- .../src/modules/snapper/ScrollSnapper.ts | 7 ++++++- .../src/modules/snapper/Snapper.ts | 21 +++++++++++++++++-- .../src/modules/snapper/WebPubSnapper.ts | 7 ++++++- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/navigator-html-injectables/src/modules/snapper/CJKVerticalSnapper.ts b/navigator-html-injectables/src/modules/snapper/CJKVerticalSnapper.ts index 8a30dcf8..e899cf7d 100644 --- a/navigator-html-injectables/src/modules/snapper/CJKVerticalSnapper.ts +++ b/navigator-html-injectables/src/modules/snapper/CJKVerticalSnapper.ts @@ -69,10 +69,16 @@ export class CJKVerticalSnapper extends Snapper { } protected hasScrolledPast(el: Element): boolean { + // go_id/go_text center the target in the viewport, not scroll it to the edge — + // so "reached" has to mean "crossed the viewport's horizontal center", the same + // line navigation scrolls to, not the leading edge. The tolerance matches + // setupTimelineObserver's rootMargin so both agree even with scrollLeft rounding. const rect = el.getBoundingClientRect(); - // vertical-rl: content flows right→left; leading edge is right, scrolled past when off-screen right. - // vertical-lr: content flows left→right; leading edge is left, scrolled past when off-screen left. - return this.verticalLR ? rect.right <= 0 : rect.left >= this.wnd.innerWidth; + const center = this.wnd.innerWidth / 2; + const tolerance = this.wnd.innerWidth * Snapper.CENTER_TOLERANCE; + // vertical-rl: content flows right→left; leading edge is right, scrolled past when past center. + // vertical-lr: content flows left→right; leading edge is left, scrolled past when past center. + return this.verticalLR ? rect.right <= center + tolerance : rect.left >= center - tolerance; } private reportProgress(forcedFragmentId?: string) { @@ -158,7 +164,7 @@ export class CJKVerticalSnapper extends Snapper { mount(wnd: ReadiumWindow, comms: Comms): boolean { this.wnd = wnd; this.comms = comms; - this.setupTimelineObserver(); + this.setupTimelineObserver("horizontal"); this.initialScrollHandled = false; this.lastScrollLeft = 0; diff --git a/navigator-html-injectables/src/modules/snapper/ScrollSnapper.ts b/navigator-html-injectables/src/modules/snapper/ScrollSnapper.ts index 178d7b6e..e112434e 100644 --- a/navigator-html-injectables/src/modules/snapper/ScrollSnapper.ts +++ b/navigator-html-injectables/src/modules/snapper/ScrollSnapper.ts @@ -38,7 +38,12 @@ export class ScrollSnapper extends Snapper { } protected hasScrolledPast(el: Element): boolean { - return el.getBoundingClientRect().bottom <= 0; + // go_id/go_text center the target in the viewport, not scroll it to the top — + // so "reached" has to mean "crossed the viewport's vertical center", the same + // line navigation scrolls to, not the top/bottom edge. The tolerance matches + // setupTimelineObserver's rootMargin so both agree even with scrollTop rounding. + const center = this.wnd.innerHeight / 2; + return el.getBoundingClientRect().top <= center + this.wnd.innerHeight * Snapper.CENTER_TOLERANCE; } private reportProgress(forcedFragmentId?: string) { diff --git a/navigator-html-injectables/src/modules/snapper/Snapper.ts b/navigator-html-injectables/src/modules/snapper/Snapper.ts index 7dfe102e..8c27d156 100644 --- a/navigator-html-injectables/src/modules/snapper/Snapper.ts +++ b/navigator-html-injectables/src/modules/snapper/Snapper.ts @@ -8,6 +8,11 @@ const SNAPPER_STYLE_ID = "readium-snapper-style"; export abstract class Snapper extends Module { static readonly moduleName: ModuleName = "snapper"; + // Shared by the observer's rootMargin and every hasScrolledPast implementation so both + // stay pinned to the exact same boundary. Sized to comfortably exceed realistic + // scrollTop-rounding error (~1px) while still reading as "the center", not a wide band. + protected static readonly CENTER_TOLERANCE = 0.01; + private protected = false; // Timeline fragment tracking @@ -45,8 +50,17 @@ export abstract class Snapper extends Module { rect.top <= root.bottom && rect.bottom >= root.top; } - protected setupTimelineObserver(): void { + /** + * Collapses the observer's root to a thin band around the same center line + * `hasScrolledPast` tests against (go_id/go_text navigate by centering the target, not by + * aligning it to an edge), so "intersecting" and "scrolled past" can't disagree about + * where "current" is. A literal zero-height root would make every intersection area zero + * — isIntersecting would never fire — so CENTER_TOLERANCE leaves it a real, if thin, band. + * Percentage-based margins track viewport resizes automatically, no re-setup needed. + */ + protected setupTimelineObserver(axis: "vertical" | "horizontal" = "vertical"): void { if (this.timelineObserver) this.timelineObserver.disconnect(); + const inset = `-${50 - Snapper.CENTER_TOLERANCE * 100}%`; this.timelineObserver = new IntersectionObserver( (entries) => { for (const entry of entries) { @@ -56,7 +70,10 @@ export abstract class Snapper extends Module { this.visibleFragmentIds.delete((entry.target as HTMLElement).id); } }, - { threshold: [0.01] } + { + threshold: [0.01], + rootMargin: axis === "vertical" ? `${inset} 0px ${inset} 0px` : `0px ${inset} 0px ${inset}` + } ); } diff --git a/navigator-html-injectables/src/modules/snapper/WebPubSnapper.ts b/navigator-html-injectables/src/modules/snapper/WebPubSnapper.ts index 1c780c99..906dd774 100644 --- a/navigator-html-injectables/src/modules/snapper/WebPubSnapper.ts +++ b/navigator-html-injectables/src/modules/snapper/WebPubSnapper.ts @@ -32,7 +32,12 @@ export class WebPubSnapper extends Snapper { } protected hasScrolledPast(el: Element): boolean { - return el.getBoundingClientRect().bottom <= 0; + // go_id/go_text center the target in the viewport, not scroll it to the top — + // so "reached" has to mean "crossed the viewport's vertical center", the same + // line navigation scrolls to, not the top/bottom edge. The tolerance matches + // setupTimelineObserver's rootMargin so both agree even with scrollTop rounding. + const center = this.wnd.innerHeight / 2; + return el.getBoundingClientRect().top <= center + this.wnd.innerHeight * Snapper.CENTER_TOLERANCE; } private reportProgress(forcedFragmentId?: string) { From 3f476f5d22a4fe0224a8e42d839daf9c55cb11e0 Mon Sep 17 00:00:00 2001 From: Jiminy Panoz Date: Wed, 22 Jul 2026 18:07:03 +0200 Subject: [PATCH 2/3] Fix span handling in timeline candidates --- .../publication/services/timeline/Timeline.ts | 10 ++++-- shared/test/Timeline.test.ts | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/shared/src/publication/services/timeline/Timeline.ts b/shared/src/publication/services/timeline/Timeline.ts index 4ad73d4b..d94dffca 100644 --- a/shared/src/publication/services/timeline/Timeline.ts +++ b/shared/src/publication/services/timeline/Timeline.ts @@ -339,8 +339,11 @@ export class Timeline { for (const link of tocLinks) { const linkBare = Timeline.bareHref(link.href); // Fragment-only hrefs (e.g. "#t=60" in single-track audio) have no bare href, - // so they are associated with the current resource. - const matchesBare = linkBare === bare || linkBare === ''; + // so they are associated with the current resource. A hrefless link (e.g. a + // grouping heading) also bareHrefs to '', but names no resource at all + // and must not match every resource. + const isFragmentOnly = link.href.startsWith('#'); + const matchesBare = linkBare === bare || (isFragmentOnly && linkBare === ''); if (matchesBare && link.title && !Timeline.isStartOfResource(link.href)) { // Prepend the resource href when the link uses a fragment-only reference. const ref = linkBare === '' ? bare + link.href : link.href; @@ -382,7 +385,8 @@ export class Timeline { for (const link of tocLinks) { const linkBare = Timeline.bareHref(link.href); - if ((linkBare === bare || linkBare === '') && link.title) { + const isFragmentOnly = link.href.startsWith('#'); + if ((linkBare === bare || (isFragmentOnly && linkBare === '')) && link.title) { if (Timeline.isStartOfResource(link.href)) { atStart.push(link); } else { diff --git a/shared/test/Timeline.test.ts b/shared/test/Timeline.test.ts index 40ad2639..9ba16951 100644 --- a/shared/test/Timeline.test.ts +++ b/shared/test/Timeline.test.ts @@ -146,6 +146,42 @@ describe('Timeline – title resolution', () => { ); expect(clean(t.items)[0]).toMatchObject({ title: 'Introduction' }); }); + + it('2.7 a hrefless grouping heading (e.g. a ) does not become the title of every resource', () => { + const t = build( + ro( + { href: 'bib.xhtml' }, + { href: 'intro.xhtml' }, + { href: 'isaacs.xhtml' }, + { href: 'andersen.xhtml' }, + ), + toc( + { href: 'bib.xhtml', title: 'Bibliography' }, + { href: 'intro.xhtml', title: 'Introductory' }, + { + href: '', + title: 'Abram S. Isaacs', + children: [ + { href: 'isaacs.xhtml', title: 'The Story' }, + ], + }, + { + href: '', + title: 'Hans Christian Andersen', + children: [ + { href: 'andersen.xhtml#story1', title: 'The Real Princess' }, + { href: 'andersen.xhtml#story2', title: "The Emperor's New Clothes" }, + ], + }, + ), + ); + expect(clean(t.items)).toMatchObject([ + { title: 'Bibliography', references: ['bib.xhtml'] }, + { title: 'Introductory', references: ['intro.xhtml'] }, + { title: 'The Story', references: ['isaacs.xhtml'] }, + { title: undefined, references: ['andersen.xhtml'] }, + ]); + }); }); // --------------------------------------------------------------------------- From 8554979d7024acacb77497bed5ce2dd8ce6c7343 Mon Sep 17 00:00:00 2001 From: Jiminy Panoz Date: Wed, 22 Jul 2026 18:31:04 +0200 Subject: [PATCH 3/3] Prepare packages --- navigator-html-injectables/CHANGELOG.MD | 6 ++++++ navigator-html-injectables/package.json | 2 +- navigator/CHANGELOG.MD | 7 +++++++ navigator/package.json | 2 +- shared/CHANGELOG.MD | 6 ++++++ shared/package.json | 2 +- 6 files changed, 22 insertions(+), 3 deletions(-) diff --git a/navigator-html-injectables/CHANGELOG.MD b/navigator-html-injectables/CHANGELOG.MD index 82aafa7a..53521dc1 100644 --- a/navigator-html-injectables/CHANGELOG.MD +++ b/navigator-html-injectables/CHANGELOG.MD @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.6.2] – 2026-07-22 + +### Fixed + +- Snappers now align id monitoring with the actual centering behaviour of `goId`/`goText`, so Timeline stays in sync when navigating to an id ([#243](https://github.com/readium/ts-toolkit/pull/243)) + ## [2.6.1] – 2026-07-16 ### Fixed diff --git a/navigator-html-injectables/package.json b/navigator-html-injectables/package.json index d3c5feb7..a3fba6ed 100644 --- a/navigator-html-injectables/package.json +++ b/navigator-html-injectables/package.json @@ -1,6 +1,6 @@ { "name": "@readium/navigator-html-injectables", - "version": "2.6.1", + "version": "2.6.2", "type": "module", "description": "An embeddable solution for connecting frames of HTML publications with a Readium Navigator", "author": "readium", diff --git a/navigator/CHANGELOG.MD b/navigator/CHANGELOG.MD index 8c2360d2..3b4bd2eb 100644 --- a/navigator/CHANGELOG.MD +++ b/navigator/CHANGELOG.MD @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.7.3] – 2026-07-22 + +### Fixed + +- Timeline matching no longer matches hrefless toc entries (e.g. `` grouping headings) against every resource (via `@readium/shared`) ([#243](https://github.com/readium/ts-toolkit/pull/243)) +- Snappers now align id monitoring with the actual centering behaviour of `goId`/`goText` (via `@readium/navigator-html-injectables`), so `timeline` stays in sync when navigating to an id ([#243](https://github.com/readium/ts-toolkit/pull/243)) + ## [2.7.2] – 2026-07-16 ### Fixed diff --git a/navigator/package.json b/navigator/package.json index dab8ad1a..d81688e0 100644 --- a/navigator/package.json +++ b/navigator/package.json @@ -1,6 +1,6 @@ { "name": "@readium/navigator", - "version": "2.7.2", + "version": "2.7.3", "type": "module", "description": "Next generation SDK for publications in Web Apps", "author": "readium", diff --git a/shared/CHANGELOG.MD b/shared/CHANGELOG.MD index 97add596..3fecdab5 100644 --- a/shared/CHANGELOG.MD +++ b/shared/CHANGELOG.MD @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.3.2] – 2026-07-22 + +### Fixed + +- `Timeline` no longer matches hrefless toc entries (e.g. `` grouping headings, which carry no `href`) against every resource; they are now correctly excluded as candidates ([#243](https://github.com/readium/ts-toolkit/pull/243)) + ## [2.3.1] – 2026-07-16 ### Fixed diff --git a/shared/package.json b/shared/package.json index 1a6a9a67..87a65465 100644 --- a/shared/package.json +++ b/shared/package.json @@ -1,6 +1,6 @@ { "name": "@readium/shared", - "version": "2.3.1", + "version": "2.3.2", "type": "module", "description": "Shared models to be used across other Readium projects and implementations in Typescript", "author": "readium",