Skip to content

PT-4585: Realign zoom to UX: text only, pop-ups unscaled, Settings % - #2849

Merged
rolfheij-sil merged 141 commits into
mainfrom
pt-4585-zoom-integration-e2e
Sep 25, 2026
Merged

rolfheij-sil merged 141 commits into
mainfrom
pt-4585-zoom-integration-e2e

Conversation

@rolfheij-sil

@rolfheij-sil rolfheij-sil commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Content zoom now scales only project text. Following the UX review of 2026-09-22 on the content-zoom epic (PT-4575), this PR narrows the feature so it does one job: make small project text easier to read. Views that show no project data (Home, New Tab and similar) are no longer zoomed and no longer offer zoom. Inside the views that do zoom, only the text grows; buttons, inputs, filters, headers, card frames and every pop-up keep interface scale. Find, the four inventories, Checks, the Markers Checklist and the Lexical Tools dictionary join the zoomable views. The editor's hamburger keeps its zoom items in Simple mode only, in their own section. In Settings, both zoom settings use a percentage stepper that wraps when narrow and accepts a typed value. The PR's original scope (cross-kind e2e specs, shared zoom maths, the stale-area decision) is unchanged and kept below.

What changed for users

One line per UX point, in the order UX raised them:

  1. Content zoom no longer scales the application's UI. Only project text grows or shrinks; Interface scaling remains the setting for the whole UI.
  2. Only views that show project data zoom, and only their text. Editor, resources, Comments, Find, inventories, Checks, Markers Checklist and the Lexical Tools dictionary zoom their text; Home, New Tab, Get resources, Manage books, Registration, Marketplace, the Send/Receive dialog and URL views render at 100 % content zoom.
  3. Pop-ups keep their size. Context menus, popovers, tooltips, the \ / Enter / footnote palettes and the inline footnote and comment editors are the same size at 200 % as at 100 %, and still open beside the zoomed text and follow scrolling.
  4. Wherever the tab menu offers zoom, Ctrl/⌘ + + / - / 0 and Ctrl/⌘ + wheel work too, including in a zoomable view that is still empty (Find before a search, Checks before a run).
  5. Views that do not zoom have no zoom items in their tab menu (removed, not greyed). In Simple mode such a tab has no tab menu at all, because Simple mode's tab menu holds only the zoom items.
  6. The editor's hamburger menu no longer shows zoom items in Power mode. In Simple mode, where the editor tab has no tab-header menu, the items stay, in their own "Zoom" section with separators around it.
  7. The Settings zoom stepper wraps instead of clipping (step buttons on top, percentage and reset below), and the percentage is typeable.
  8. Interface scaling is shown and edited as a percentage with the same stepper; the stored value is still the factor.

How it is built

Which panes zoom. Core's declaration map (CONTENT_ZOOM_DECLARATION_BY_WEB_VIEW_TYPE in src/shared/models/content-zoom.model.ts) gives each first-party zoomable web-view type a kind and a default area; it gains Find, the four inventories, Checks, the Markers Checklist and the Lexical Tools dictionary. Word List and Compare Versions are declared when their marker PRs in paratext-bible-extensions and paratext-bible-internal-extensions land, so until then those two views offer no zoom. isContentZoomable(webViewId) in the renderer content-zoom service is true when the pane's type is declared OR the pane currently reports at least one zoom area. A declared pane with no marker rendered yet still acts: chords, wheel and menu target its declared default area, starting from the pane's own level, then the remembered per-project level, then the Settings default. A pane that is not zoomable is never scaled: the whole-iframe fallback is gone, and so is the hidden per-type setting platform.webViewContentZoomTypesWithAreas that existed only to prevent its flash (its label key is deprecated in metadata.json, not deleted). The grace timer now only drops the stale areas of a pane whose bootstrap died. Zoomability changes are published as a renderer-local event, onDidChangeContentZoomable, wrapped by useIsContentZoomable.

Tab menu. The three zoom items are absent, not disabled, on a pane that is not zoomable. The tab title reads zoomability reactively, so a third-party view gains its items when it reports its first area; while a menu is open its zoomability is held for that open, so the menu never changes shape under the pointer.

Editor hamburger. The zoom items move into their own column, platformScriptureEditor.zoomSection, with the heading "Zoom" (one new localization key, en and es), and each item carries hiddenInterfaceModes: ["power"]. In Power mode the column is empty and drops out with no heading or separator; in Simple mode it is its own separator-bounded section.

Only project text is marked. ContentZoomRoot gains as?: 'div' | 'span' for inline text, and library components that render project text mark it only inside a ContentZoomTextProvider (via useContentZoomTextProps), which the hosting view opts into. Region markers are replaced by markers on the text itself:

  • Scripture editor: the text marker wraps only the editor tree; the character-marker bar, the empty-chapter and book-not-available views stay at interface scale. The Simple-mode gutter reservation is divided by the zoom so the fixed-size bar never paints over text.
  • Comments: the scripture snippet, comment and reply bodies, the conflict diff (including a resolved conflict's text) and the inline composer; cards, buttons, badges, avatars, author and date do not.
  • Text Collection grid: a wrapper directly around each cell's text, which content zoom alone sizes (the grid's own per-column zoom is removed; see the review section); headers and grips stay fixed. Each resource's text is its own content-zoom area, so each text in the Text Collection zooms on its own; see "Text Collection: one zoom area per resource" below.
  • Enhanced Resources: main and footnotes unchanged; the entries area now marks lemma, gloss, definition and article text instead of the whole panel.
  • Find: result snippets, expanded verse context and replace previews; reference buttons, inputs, toggles and cards do not.
  • Inventories: the item column (including marker tokens in the Markers inventory) and the occurrence text; counts, other columns, headers and the toolbar do not.
  • Checks: the checked text in each card.
  • Markers Checklist: text spans, character-style spans, verse numbers, link text and the \marker tokens; the edit link, Ref column and indentation do not scale.
  • Lexical Tools dictionary: the lemma in the list, and the entry title, glosses and definition.

Pop-ups never scale. Removed: the library's area context (ContentZoomAreaProvider, useContentZoomArea), the pop-up attribute CONTENT_ZOOM_POPUP_ATTRIBUTE and getContentZoomPopupStyleand the overlay contentScale prop. (The contextMenuContainer wiring for the editor library is removed in #2844 itself.) The six shadcn pop-up files return to their bytes on main before #2825, plus a two-line // CUSTOM: note on each pop-up component saying it stays at interface scale. Kept: live anchors (useLivePopoverAnchor, the editor's anchor sources), the overlays' frameScale and getWebViewIframeZoom (parseIframeZoom is now private to its module), and the footnote editor's narrow-pane fixes. Neither this branch nor #2844 depends on paranext/scripture-editors#17, which is closed unmerged.

Settings zoom steppers (intended visual change). The stepper is now two button groups — [− +] and [percentage ⟲] — inside one wrapping row, so a narrow Settings pane puts the percentage and reset under the step buttons instead of clipping. Each group rounds its own outer corners: the + button's right corners and the percentage field's left corners are now rounded, where the single merged group had square joins. This is deliberate, not a regression. The percentage is typeable (any whole 50–300 %, Enter or blur commits, Escape abandons), and Interface scaling uses the same stepper, shown as a percentage (stored factor unchanged). Settings cards now cap at the pane width and put each label above its control when narrow; screenshots of General, an extension group and a project's settings at both widths are attached.

Settings screenshots: General, an extension group and a project's settings, at default and narrow width
Default width Narrow
general-default.png general-narrow.png
extension-default.png extension-narrow.png
project-default.png project-narrow.png

Interface scaling as a percentage. platform.zoomFactor renders with the same ZoomStepper as "Tab content default zoom"; both use the 50–300 % range and reset returns to 100 %. The stored float and its consumers in the main process are unchanged. Both settings get new description keys (…_description_2) that talk in percent; the replaced keys are deprecated, and new en/es label keys cover the Interface scaling buttons, limits and the percentage field.

Docs. New ADR entries adr-content-zoom-applies-only-to-zoomable-panes, adr-pop-ups-stay-at-interface-scale and adr-zoom-areas-mark-project-text; adr-pop-ups-follow-their-content-zoom-area is superseded; adr-editor-context-menu-follows-its-area-via-a-container is withdrawn to a stub under the log's carve-out; adr-zoom-composition, adr-simple-mode-tab-menu-offers-zoom-only and adr-resource-panes-name-their-zoom-areas carry Amended notes. The Extension Development Guide and Component Builder Patterns describe marking project text, the third-party limitation below, and pop-ups at interface scale.

Callouts for reviewers

  • Hidden panes (cross-view sync rule). Zoomability needs no catch-up: area reports come from the bootstrap's MutationObserver, which needs no layout, so an inactive tab's zoomability is current while it is hidden, and the declaration is static. Zoom levels likewise apply while hidden: the CSS variables and the rules that read them are data-driven, so a hidden pane is already correct when its tab is shown. Only the level indicator needs geometry, and it is shown only for a direct user action, which needs a visible pane. Both decisions are recorded at the emitter and at the push site in web-view-content-zoom.service.ts.
  • Corner rounding in Settings is intended; see the Settings paragraph above.
  • Scripture editor marker placement. The text marker sits one level inside TwoStepDeleteTooltipOverlay, not directly around renderEditor(), because the overlay places its two-step delete hint from rect differences against its own wrapper.
  • Text Collection grid test location. The test that pins the marker directly around the cell text lives in resource-cell-view.component.test.tsx, because the grid component's test mocks ResourceCell.
  • Enhanced Resources marking. ER marks its entry text through ContentZoomTextProvider plus the hook rather than ContentZoomRoot as="span". Its article-viewer is a Dialog, so it counts as a pop-up and is not marked.
  • Input routing after text-only marking. The bootstrap resolves the target area from the nearest marked ancestor and falls back to the last active area, and text-only markers leave more gaps: (a) in the Scripture editor, Ctrl+wheel over the character-marker bar or the blank space below a short chapter targets the last-used area, which may be footnotes; (b) in Enhanced Resources, entries drops out of the reported areas whenever its text unmounts (loading, empty state, Media/Maps tabs), so after moving to another verse the next Ctrl+= zooms main until the user clicks entry text. All of these are accepted as documented behaviour (product ruling 2026-09-23).
  • parseIframeZoom is no longer exported (it was @experimental on papi.d.ts with no consumer outside its module); papi.d.ts is regenerated.
  • Word List and Compare Versions are not declared zoomable yet. Their text markers land in their own repos' PRs (paratext-bible-extensions and paratext-bible-internal-extensions), not started yet; each of those PRs comes with the one-line core declaration, so no view ever offers zoom items that do nothing.
  • Third-party views are zoomable only while at least one element carrying data-platform-content-zoom-root is rendered; this is documented in the Extension Development Guide and the ContentZoomRoot TSDoc.
  • The zoom level badge sits in the web view's top-right corner (top-left for right-to-left text), fixed, whichever area was zoomed; it no longer follows the zoomed text's rectangle (it drifted per step and could leave the pane).
  • useWebViewState behaviour change (public hook). Every zoom step saves the level into the web view's state and fires the web view's update event. Two rules were added so that event no longer churns unrelated slots: a slot still showing its default keeps that same object across unrelated updates (a fresh inline []/{} default no longer re-triggers effects; resetWebViewState() still applies the latest default), and a saved value deeply equal to the current one keeps the current object. The Markers Checklist reloaded its rows on every zoom step because of this; it also now passes stable defaults. Signature unchanged; papi.d.ts doc text regenerated. What this changes for callers of the @papi/core hook: saving a value deeply equal to the current one keeps the same object identity, and a slot showing its default keeps its identity across unrelated updates, so an effect keyed on the value's identity no longer re-runs for an equal value. A caller that saved an equal new object to force such an effect to re-run no longer gets that re-run; the review found no such caller.
  • Zoom declaration lookup is cached per web view (invalidated on the web view's update event and when the pane is forgotten), so tab titles no longer walk the dock layout on every focus change.
  • Tab-menu hold is narrower than the spec. Only the zoomability value is held per open, not the whole item list, because Power mode's "Move tab to window" targets arrive after the menu opens.
  • Removed experimental exports (removals from main). PT-4584: Content zoom for the comment list and the Comments panel #2825 is merged, so nine platform-bible-react exports this PR removes or renames are on main today and leave it with this PR: ContentZoomAreaProvider, ContentZoomAreaProviderProps, useContentZoomArea, CONTENT_ZOOM_CSS_VARIABLE_PREFIX, CONTENT_ZOOM_DEFAULT_CSS_VARIABLE, CONTENT_ZOOM_POPUP_ATTRIBUTE and MAIN_CONTENT_ZOOM_AREA_ID are removed, and measureRange/measureElement become one measureBox(target: Range | Element). All nine are @experimental on main, and no consumer was found in core, paratext-bible-extensions, paratext-bible-internal-extensions or paratext-10-studio, so this is a clean break with no deprecated aliases. The other removals (createContentZoomWheelReader and its two types in platform-bible-utils, the platform.webViewContentZoomTypesWithAreas setting, the overlay-content-zoom.util module in papi.d.ts) exist only in PT-4582/4583/4711-4714: zoom for Resources, pop-ups and editor menu #2844 and never reached main.
  • Extension authors: unmarked views are no longer zoomed. With the whole-iframe fallback gone, "Tab content default zoom" and Ctrl/⌘ + wheel or chords no longer affect a third-party web view that marks no zoom area: it renders at 100 %, is reported not zoomable, and its tab menu has no zoom items. On main today such a view is scaled as a whole iframe (since PT-4575: Per-pane content zoom — chords, opt-in, tab menu, Settings, editor #2821), so this is a behaviour change for extensions. To opt in, an author marks the elements that render project text with ContentZoomRoot (or ContentZoomTextProvider for library components); the Extension Development Guide's "Content Zoom (experimental)" section and the Component Builder Patterns' "Content Zoom Opt-In (experimental)" section describe how.
  • shadcn pop-up files against main. The six pop-up shadcn files match main before PT-4584: Content zoom for the comment list and the Comments panel #2825 plus the two-line // CUSTOM: note on each; the diff against main is the removal of PT-4584: Content zoom for the comment list and the Comments panel #2825's zoom-aware pop-up code.

Verification

Ran green:

Check Result
npm run typecheck clean
npm run lint clean (one pre-existing warning outside this range)
npm run format:check clean
papi.d.ts, PBR dist regenerated and current
Unit, full suites after the text marking core 5842 (plus the known stale rc-dock failure), PBR 1077, legacy-comment-manager 277, platform-scripture-editor 1696, platform-enhanced-resources 215
Unit, focused suites after the new views and Settings core 271, PBR 15, platform-scripture 654, platform-lexical-tools 8
After the 2026-09-23 rulings batch overlays 235, localization 54, models/utils 45, content-zoom service/hook/tab-title 327; typecheck, lint, format:check clean

e2e that ran:

  • content-zoom-tab-menu/tab-menu-zoom-items-presence.spec.ts (new): Simple and Power mode.
  • content-zoom-tools/find-inventory-checks-text-zoom.spec.ts (new).
  • settings-zoom-stepper/settings-zoom-stepper.spec.ts (new): narrow-pane wrap, typing, Escape, Interface scaling.
  • scripture-editor/content-zoom.spec.ts with the pop-up steps inverted, plus the other editor zoom specs.
  • From the original scope: content-zoom-live-sharing/, the resources group, and multi-window/ (including the move-between-windows spec). The content-zoom-restart/ editor test failed once on relaunch (DialogService not registered, an environment issue) and passed on retry.

Manual checks and what did NOT run:

  • Comments e2e (comment-list-content-zoom, comments-panel-content-zoom, the Power-mode twin from the original scope) and the specs that read Comments factors (content-zoom-default-change/, the restart comment-list spec): Comments cannot open in dev builds because of PT-4554: Rework the comments tab filters and make drafts durable #2840 on main, so these died at openCommentList before any zoom assertion. Written, linted and listed.

  • Text Collection grid e2e ran on 2026-09-24 on this branch with two installed resources, after the helper fixes described in the 2026-09-24 section below: scripture-text-grid-zoom.spec.ts 3/3 pass; scripture-text-grid.spec.ts 7 pass, 8 fail, 2 skip (the 8 are pre-existing stale assertions unrelated to zoom); cell-reorder.spec.ts 3 skip (needs resources). That 3-test spec was then rewritten for per-resource zoom; its later 4/4 run is under "Text Collection: one zoom area per resource" below.

  • Enhanced Resources e2e: attach-mode specs that need a running app with Marble resources. Written, linted and listed.

  • Windows touchpad and wheel gestures, verified live by Rolf in the Windows dev app on 2026-09-24:

    Gesture Result
    Touchpad pinch works fine
    Ctrl + touchpad pinch works fine (before the fix: a full step per frame)
    Ctrl + two-finger touchpad scroll works fine
    Ctrl + mouse wheel works fine
  • Editor gutter check done 2026-09-23: LTR (Genesis 1) and RTL (zzz7) at 50/100/200/300 % with the character-marker bar fixed and no text under it; the Backspace/Delete two-step hint sits on the caret line at 150–200 %.

  • Visual spot check of the new views was done on 2026-09-23 in the Windows dev app: Find after a search, the Markers inventory and the Markers Checklist at 50 % and 300 % pass; it surfaced the badge drift and the checklist reload fixed above, both re-checked in the app afterwards.

  • Storybook: the new inventory ZoomedText, CommentList ZoomedText and ContentZoomRoot stories were eyeballed on 2026-09-23 (the CommentList story needed the Default story's permission callbacks to show the comment menu; fixture only).

  • Settings label row spacing (a small CSS change from the self-review) has not yet been eyeballed in the app.

Final whole-branch reviews of the two last sub-plans led to one fix wave each:

  • Settings stepper: a focused but unedited percentage field used to write its stale value back on blur, undoing a change made elsewhere (for example the zoom-in command from another window). The field now commits only text the user typed; Enter and Escape clear the edit; the screen-reader announcement clears when the value changes elsewhere. Two red-first tests pin the stale-blur cases, and the eight stepper tests that printed React's act() warning are wrapped in act.
  • New zoomable views: the Lexical Tools title glosses now zoom like the sense-card glosses (the same word no longer shows at two sizes in one pane); the dictionary entry rendered in the narrow-pane Drawer keeps its markers, with a comment recording that the Drawer is the pane's narrow layout rather than a pop-up; the Find contract test covers all three preview layouts and its loading case has a positive control; the dictionary test's structural assertion was replaced by a marker check; closeDockTab is one shared e2e helper instead of three copies.

Review of #2825 and #2844 (2026-09-23): fixes that land here

Matt's second review of #2825 and #2844 found issues in code this PR carries forward unchanged. The fixes land here, not on those PRs, so they reach main in the same merge sequence; each reply on #2825/#2844 names its commit. Findings about code this PR removes (zoom-aware pop-ups, the whole-iframe fallback and its per-type setting) are answered there as superseded.

  • Per-pane zoom memory (web-view-content-zoom.service.ts): a memory key added and removed while a sibling sync is unconfirmed now reaches every pane that missed it (each pane is walked against the keys it missed until its own write is confirmed); a stamped pane fills the areas it has no level for from memory; a level chosen before a pane shows any project is stamped with its kind alone (notes:) and gives way to the first project's remembered level; one helper (hasStaleStamp) decides whether a pane's levels belong to the project it shows, which also stops a re-pointed panel showing or saving the previous project's levels while memory is unreadable and stops a pane whose project is gone flashing the default; an all-invalid level set is no longer stored as an empty map plus a stamp; seedFromMemory simplified and documented; unstamped panes skip identity work on every definition update; the named subscription helpers restored.
  • Comments list: a verse move made while the tab is hidden now supersedes a thread selection that was deferred while hidden, so the list opens on the verse the editor shows (hidden-tab callout: the deferred thread scroll records its thread's verse; a scroll-group move to another verse while hidden drops it; a move to the thread's own verse keeps it). The scroll-padding cleanup that could never run is removed.
  • Editor: the pending-comment popover stays at its caret after a zoom change or a pane resize, including inside a long wrapped run of unspaced text (Thai, CJK). measureRange/measureElement are one measureBox, and the live-anchor exports carry full TSDoc.
  • Text Collection: its own per-column zoom (the right-click menu's zoom items, the chapter view's zoom menu and Ctrl+wheel over a column) is removed, so content zoom alone sizes its text and the two no longer multiply. Levels saved per column are no longer read (not migrated); the four menu strings are kept and marked deprecated; createContentZoomWheelReader and the wheel-parity test are deleted from platform-bible-utils, leaving the bootstrap's reader as the only one. PT-4184: Add the verse-aligned Grid view to the Text Collection #2781 (the Grid view) needs a rebase over this. Later on 2026-09-24 the per-resource zoom came back on the platform's mechanism (see "Text Collection: one zoom area per resource" below): each resource is its own content-zoom area, the right-click zoom items and the chapter view's "⋮" return and send the platform's zoom commands, the grid's own Ctrl+wheel listener stays gone, and the four menu strings are live again (their deprecation entries removed). The old per-column levels are still not read or migrated, and the wheel reader stays deleted.
  • Wheel and chords: a slow Ctrl+two-finger scroll whose wheelDeltaY rounds to 0 now zooms; typing in the text ends a footnote click's hold on the zoom chords (lone modifiers and chords do not), and the shortcut catalog says so.
  • Tests and docs: the Enhanced Resources zoom e2e restores every area on the failure path; the Bible Texts spec waits for its baseline; the Enhanced Resources area sweep finds area= in any attribute position; the Comments e2e helper retries its whole open sequence; comment, ADR and spelling corrections.

Declined, with the reason given on each thread: a batched write for the one stamp-only write per pane after upgrading, and moving @experimental exports to ./experimental. Nothing from this review is deferred.

Since the review (2026-09-24): Windows pinch fix, Text Collection e2e helper, self-review, restack

Commits are named by subject and short SHA; both #2825 and #2844 have since merged, and the SHAs below are the branch's final ones after its rebase onto main (see Restack, below).

  • Windows Ctrl+pinch. fix(zoom): read a Windows touchpad pinch as a pinch, not a notch per frame (13b62ac318b): the bootstrap treated a physically held Ctrl or ⌘ as proof that a ctrl+wheel frame was a mouse notch, so a pinch made with Ctrl held (the usual Windows gesture) zoomed a full step per frame. Held-key evidence is now consulted only on macOS, where a notch can be as small as a pinch frame; on Windows and Linux the frame size alone separates a pinch from a notch. Verified live by Rolf on Windows (pinch, Ctrl+pinch, Ctrl+trackpad scroll, Ctrl+mouse wheel; see Verification). Follow-up docs: docs(zoom): say precisely when a wheel frame opens a pinch, and cross-link the ADR entries (87c471ae774).
  • Text Collection e2e helper. test(e2e): open the Text Collection grid by its current title and project (2198071c650) and test(e2e): seed the setting the Text Collection grid actually reads (1642ba6e4b6): the shared helper looked for the tab title "Scripture text" (renamed "Text Collection" in July) and seeded platformScripture.modelTexts, which the grid stopped reading when platformScripture.referencedProjectsAndResources took over, so every spec using it opened an empty grid. Both problems are also on main's copy of the helper; these commits fix them there when this PR merges. Live result on this branch with two installed resources: scripture-text-grid-zoom.spec.ts 3/3 pass; scripture-text-grid.spec.ts 7 pass, 8 fail, 2 skip; cell-reorder.spec.ts 3 skip (needs resources). The 8 failures are pre-existing stale assertions unrelated to zoom (single-resource full-width layout, a renamed loading string, tab shape, a focus-ring check); they are not fixed here and are tracked as PT-4779 (sub-task of PT-4203). The helper's restore writes the default value back where the setting was absent before the spec; the specs are local-only.
  • Self-review round. /review-paratext and the OCR delegate review ran on the restacked branch. Findings are fixed in four commits: test(zoom): cover the slow Ctrl+scroll fallback when wheelDeltaY rounds to 0 (ab1809430d7), docs(zoom): state the current iframe zoom rule and consolidate this change's ADR amendments (d11b1cbdc0f), style(react): annotate the pop-up components that stay at interface scale (374de6a0cdd) and chore(zoom): self-review minors (e6dec6bec43). Declined with evidence: the theoretical concern that a Ctrl + two-finger touchpad scroll on Windows would be read as a pinch and zoom too fast (the live test was negative); and, by decision with no ticket, the review's rare memory-seeding timing gap where a pane reads its levels from memory during an unconfirmed write.
  • Settings hint text. fix(settings): shorten the tab content default zoom hint (2eb45764d22): the new webViewContentZoom_description_2 hover tooltip was far too long for a settings label hint; it now reads "Default size of project text in panes you haven't zoomed. Interface scaling applies on top." in both en and es.
  • Restack. After PT-4584: Content zoom for the comment list and the Comments panel #2825 merged (8276cb3bdfa), the branch was rebased onto PT-4582/4583/4711-4714: zoom for Resources, pop-ups and editor menu #2844's restacked head: 111 picks and one generated-output rebuild, build: regenerate papi.d.ts and library output after restack onto main (b3e8bd1a0f8). PT-4582/4583/4711-4714: zoom for Resources, pop-ups and editor menu #2844 has since merged too (squash 310bc516da8); the branch was rebased onto main a second time — 117 picks and one generated-output rebuild — and now sits directly on main.

Text Collection: one zoom area per resource (2026-09-24)

What the user gets. Each text in the Text Collection has its own zoom level again. Ctrl+wheel, a pinch, the right-click menu (Copy, then Zoom in, Zoom out, Reset zoom) and the "⋮" in the chapter view's header zoom one text; the others stay as they are. A text's verse row and the chapter panel or chapter column opened for it always show the same size. The zoom keys and the tab menu act on the text last clicked (anywhere in its row or column), and Ctrl+0 resets only that text. The level badge names the text: "HSV · 120 %". Each text's level is remembered per project, follows "Tab content default zoom" until it is zoomed on its own, and comes back after the tab is closed and reopened or the app restarts. Zoom in is disabled at 300 % and Zoom out at 50 %. Reset zoom is disabled while the text has no level of its own, because that is when it would change nothing (the platform's default is a setting, not 100 %).

How it is built.

  • Area ids: resource-<id>, the resource id lower-cased with every character outside [a-z0-9-] replaced by - (toResourceZoomAreaId). An id with nothing left falls back to the pane-wide text-collection area. Markers, scopes and menu commands all get the id from one helper, resourceZoomAreaOf, in scripture-text-grid/resource-zoom-area.utils.ts.
  • Each resource's text is marked with ContentZoomRoot area={zoomArea} label={resourceName}, inside the scroll box, in the verse row and in the chapter view. Resource names, grips, the View Options row and the "⋮" stay at interface scale.
  • The menus send the existing platform.webViewContentZoomIn / Out / Reset commands with the web view id and the resource's area (useResourceContentZoom). They read the level from the platform.contentZoomLevels web-view state and the "Tab content default zoom" setting, and ignore a stored level that is not a map or is outside 50–300 %. web-view-content-zoom.service.ts is unchanged.
  • The four shipped menu strings (%webView_scriptureTextGrid_cell_zoomIn%, …_zoomOut%, …_resetZoom%, …_zoomOptions%) are used again with their shipped values; their deprecationInfo entries are removed and no string is added. The keyboard catalog's content-zoom-wheel context sentence now includes the zoom scope the pointer is in.

Framework additions (experimental, additive). data-platform-content-zoom-scope (CONTENT_ZOOM_SCOPE_ATTRIBUTE) marks an unscaled row, column or card as belonging to one area, so a click, focus or Ctrl+wheel anywhere in it targets that area; a marker inside the scope still wins, and the scope scales nothing. data-platform-content-zoom-label (CONTENT_ZOOM_LABEL_ATTRIBUTE, written by ContentZoomRoot's new label prop) names an area in the badge. Both constants are tagged @experimental in web-view.model.ts (so in papi.d.ts) and in their platform-bible-react mirrors, and so is the label prop. Nothing changes for a view that does not write them. No new command, network object, provider or menu contribution, so nothing new goes on the wire.

Not migrated. Levels saved by the shipped per-column zoom (scriptureTextGrid.zoomByResourceId) are not read or migrated: they were per tab and the new ones are per project, and writing platform.contentZoomLevels from the view would race the platform's seeding. Users re-zoom once. A contract test pins that no grid file reads the old key.

Known limit. A Text Collection opened from the default layout, before it is pointed at a project, has no project to remember levels under. Levels set there are dropped when it is pointed at one, as for the pane-wide level before.

Hidden tabs (for the reviewer to check). No sync code was added. A level change reaches a hidden Text Collection tab as a CSS variable, which applies without layout, so the tab shows the right sizes when it is shown. The menus' enabled states are data-driven (useWebViewState, useSetting). The badge's placement waits for an animation frame, as it does everywhere else.

Observed during the live e2e run, not caused by this PR and not fixed here:

  • After View Options is closed with Escape, focus returns to its button and its tooltip stays open while that button has focus. The tooltip can cover a neighbouring control, such as the last column's "⋮", until focus moves (the first click then lands on the tooltip). This is existing behaviour of every tooltip-wrapped pop-up button in the app. The e2e helper moves focus away before it clicks.
  • At high zoom in a narrow column, poetry lines (\q1 etc.) collapse to a few pixels of text. The editor stylesheet _usj-nodes.scss indents them with 15vw/-10vw, and viewport units grow with CSS zoom while the column does not. This predates this PR (placing the marker on the scroll box instead gives the same result) and affects the platform editor too.
  • The menus' enabled states can trail the visible level by up to about 250 ms, because the platform writes platform.contentZoomLevels after the zoom is already applied. A menu opened in that window may briefly show Zoom in enabled at 300 %; it settles without further input.

PR #2781 (verse-aligned Grid view). The markers wrap each resource's text, never the column box, and the scope sits on the column container that #2781 keeps. Marking each .verse-block with the resource's area works only if the cell does not also mark its text: ResourceCellView wraps the whole editor in one ContentZoomRoot, and a marker nested inside another marker is ignored. So the aligned view must either skip the cell's own text marker, or keep it and read var(--platform-content-zoom-resource-<id>, …) with the wrapper's zoom neutralised. The ADR entry adr-text-collection-resources-are-zoom-areas records this. A follow-up comment for #2781 is drafted and not posted.

Docs. New ADR entry adr-text-collection-resources-are-zoom-areas, with amendments to adr-resource-panes-name-their-zoom-areas and adr-zoom-areas-mark-project-text. Component Builder Patterns ("Content Zoom Opt-In") and the Extension Development Guide ("Content Zoom") describe the scope and the label.

Tests.

  • Unit and contract tests run in CI: the bootstrap's scope and label handling, the constants and their library mirrors, ContentZoomRoot's label, the area-id helper, the menu hook, the cell view and cell (menu items, order and disabled states, the "⋮" in the header layout only), the grid (same area in a resource's verse row and chapter panel, a different area per resource, the scope on every resource container in all four layouts, the fallback with one warning), the Text Collection contract test and the localized-strings test.
  • e2e: scripture-text-grid-zoom.spec.ts was rewritten and is a local-only CDP-attach spec; CI does not run it. It ran headless in WSL in Power mode with two installed resources (NBV21 and WEB) and passed 4/4 on the final content: Ctrl+wheel zooms one resource and its chapter panel shows and moves the same level; the right-click menu and the chapter view's "⋮" zoom one resource and Zoom in stops at 300 %; the zoom keys act on the resource last clicked and Ctrl+0 resets only that one; each resource keeps its level when the tab is closed and reopened. The e2e covers persistence by closing and reopening the tab, not by restarting the app, because an attach-mode spec cannot restart the app.
  • Rolf's live check on Windows (pinch per text, menus, badge with long and RTL names, restart persistence, the 250 ms lag): pending.

Original scope

The PR's original description, kept as written. Its "nothing here changes user-visible behaviour" held for that scope; the realignment above changes behaviour throughout, and the Verification section above supersedes the note that the six original specs had not run. The contextMenuContainer typecheck failure it mentions no longer applies: #2844 dropped that wiring, and neither PR depends on paranext/scripture-editors#17. "Stacked on #2844" no longer applies either: #2844 has merged, and this branch is rebased directly onto main; see Dependencies.

Adds the cross-kind end-to-end checks PT-4585 lists for the per-pane content zoom epic (PT-4575), promotes the duplicated zoom maths into platform-bible-utils (PT-4725), and records the decision on stale zoom areas (PT-4715 A). Stacked on #2844; nothing here changes user-visible behaviour.

What this adds

Cross-kind e2e specs (PT-4585), under e2e-tests/tests/isolated/:

  • content-zoom-restart/: the Scripture editor's text and footnotes areas, and the comment list, keep their zoom across an app relaunch, with no level indicator on arrival (two-phase relaunch, same pattern as the layout-persistence spec).
  • multi-window/web-view-move-between-windows.spec.ts: a moved tab keeps its text-area zoom in the window it arrives in.
  • content-zoom-default-change/: changing "Tab content default zoom" moves only the pane still at default; a pane with its own level stays put, and the follower shows no indicator.
  • content-zoom-live-sharing/: two open editor tabs of one project follow each other live; reset returns both.
  • notes-content-zoom/comments-panel-content-zoom-power-mode.spec.ts: the Power-mode twin of the Simple-mode tab-menu spec.
  • core-settings-info.data.test.ts: pins that platform.webViewContentZoom and platform.webViewContentZoomMemory are core settings, not project settings, which is the only Send/Receive-relevant invariant (S/R sync itself is a stub in this repo).

Not covered here, by design: restart persistence for the Text Collection grid and Enhanced Resources (they need real resource fixtures the isolated suite does not have); the Windows precision-touchpad pinch check (a person's job, PT-4585).

One implementation of the zoom maths (PT-4725). clampZoom, roundZoom, adjustZoomFactor and the MIN_ZOOM_FACTOR / MAX_ZOOM_FACTOR / ZOOM_STEP constants now live once in lib/platform-bible-utils/src/content-zoom.util.ts (each tagged @experimental), next to the wheel reader that already lived there. The core copy and the Text Collection grid's copy are deleted; consumers import the shared version; the wheel reader imports the constants instead of hardcoding them, so the wheel-parity guard that pinned two hardcodings against each other is retired. The core model and platform data modules re-export the constants so existing imports keep working. platform-bible-utils dist and papi.d.ts are regenerated; platform-bible-react externalizes the utils package and needed no rebuild. The wheel reader itself is later deleted from the utilities package with the Text Collection's per-column zoom (see the review section); the platform's inlined reader is the only one.

Stale zoom areas on content replacement (PT-4715 A): decided, not built. A comment at the iframe load hook in web-view-content-zoom.service.ts records why a replacement never clears a pane's area list: the bootstrap's observer re-reports the moment a marker is added or removed, an unchanged list is one the new content still owns, and a realm swap is settled by the grace timer's liveness probe. A "clear on load" flag would regress the reload case, because the load event fires after the new content has already reported.

Verification

  • Typecheck: core, erb and e2e projects clean. The platform-bible-react workspace typecheck fails only on contextMenuContainer, PT-4582/4583/4711-4714: zoom for Resources, pop-ups and editor menu #2844's documented dependency on Let the host choose where the context menu renders scripture-editors#17.
  • Unit tests: root src 4751 passed (the 2 failures in rc-dock-tab-cache-patch.test.ts are a stale node_modules/rc-dock in the worktree, file identical to the base); extensions 3006 passed; platform-bible-utils 644 passed.
  • Lint and prettier clean on every file this branch touches.
  • The six new e2e specs have not run yet: the shared e2e ports were held by another session's app all afternoon. They typecheck and lint; running them is the next step before this leaves draft.

Dependencies

#2821, #2825 and #2844 are all merged; #2825 is on main as 8276cb3bdfa and #2844 as 310bc516da8. This PR's base is now main: once both had merged, the branch was rebased onto main directly — 117 picks and one generated-output rebuild — replacing its earlier basis on #2844's restacked head.

The merge order is unchanged by the realignment. #2844 changed only to drop the editor's context-menu container and reflow one file for prettier. This PR reverses the parts of #2825 and #2844 that UX rejected (pop-ups following their area, region markers, the whole-iframe fallback). Both #2825's and #2844's parts are on main now, so main carries that behaviour until this PR merges. The Word List and Compare Versions marker PRs in their own repos merge after this one.

AI-assisted — session 1, session 2, session 3

🤖 Generated with Claude Code

https://claude.ai/code/session_01MddYES5CZ1wg3CjnyakeXw


This change is Reviewable

@rolfheij-sil rolfheij-sil changed the title PT-4585/4725/4715: cross-kind zoom e2e, shared zoom maths, stale-areas decision PT-4585: Realign zoom to UX: text only, pop-ups unscaled, Settings % Sep 23, 2026
@rolfheij-sil
rolfheij-sil force-pushed the pt-4582-content-zoom-resources-and-popups branch from 280e276 to 57cdcc4 Compare September 23, 2026 12:53
@rolfheij-sil
rolfheij-sil force-pushed the pt-4585-zoom-integration-e2e branch 2 times, most recently from 51bcb2d to 4e1b1af Compare September 23, 2026 16:57
@rolfheij-sil
rolfheij-sil force-pushed the pt-4582-content-zoom-resources-and-popups branch from b0352df to 66943b4 Compare September 24, 2026 00:01
@rolfheij-sil
rolfheij-sil force-pushed the pt-4585-zoom-integration-e2e branch from f818b3c to bff2bc0 Compare September 24, 2026 01:55
@rolfheij-sil
rolfheij-sil force-pushed the pt-4582-content-zoom-resources-and-popups branch from 66943b4 to 489ee7b Compare September 24, 2026 08:27
@rolfheij-sil
rolfheij-sil force-pushed the pt-4585-zoom-integration-e2e branch from 15895c2 to 833f8a1 Compare September 24, 2026 08:40
Base automatically changed from pt-4582-content-zoom-resources-and-popups to main September 24, 2026 09:13
@rolfheij-sil
rolfheij-sil force-pushed the pt-4585-zoom-integration-e2e branch from 833f8a1 to 30db0b8 Compare September 24, 2026 09:25
@rolfheij-sil
rolfheij-sil marked this pull request as ready for review September 24, 2026 10:19
@rolfheij-sil
rolfheij-sil marked this pull request as draft September 24, 2026 12:04
@captaincrazybro

Copy link
Copy Markdown
Contributor

Measurement that bears on where the ContentZoomRoot area="text-collection" marker is placed, from
the Text Collection side (PT-4543 / #2856, stacked on #2781).

Short version: CSS zoom must land strictly below [data-cell-content]. That element is the
scroll port the Text Collection's chapter surfaces scroll to a verse. zoom on it — or on anything
above it — breaks the scroll, silently and only when zoomed.

Why

scrollPortToBlock computes its delta from getBoundingClientRect() (viewport pixels, which include
any CSS zoom in scope) and applies it to scrollTop (the port's own local pixels). Moving
scrollTop by s moves content by s × Z viewport px, so the scroll overshoots by the zoom factor.
The residual after each pass is e × (1 − Z): at Z = 2 that is −e, an oscillation that never
settles.

Measured, headless Chromium, real geometry

Same DOM shape as a chapter cell (300px port, 20px sticky header, 80 verses), navigating to verse 40.
gap is how far the verse still is from the top of the pane after the scroll; 0 is correct.

                         Z      pass1   pass2   pass3
zoom on none            1.5         0       0       0
zoom on none            2           0       0       0
zoom on ancestor(root)  1.5     -1352     676    -338
zoom on ancestor(root)  2       -3310    3310   -3310   ← never converges
zoom on port            1.5     -1352     676    -338
zoom on port            2       -5107    5107   -5107   ← never converges
zoom on child(pad)      1.5         0       0       0
zoom on child(pad)      2           0       0       0

The ancestor row is the one I would not have predicted: it is not enough to keep zoom off the port
itself. A descendant's scrollTop stays in unzoomed layout units while its rect is scaled, so the
same mismatch appears for zoom applied anywhere above the port.

What this means here

On main today the factor arrives through the zoom / zoomTarget prop chain, and #2856 moves it
off [data-cell-content] onto [data-cell-pad] — the "child(pad)" row, which measures clean. This
PR deletes that chain, so the placement decision moves entirely to where <ContentZoomRoot area="text-collection"> renders its marker.

If that marker lands on the cell's content box or any wrapper above it, the chapter-surface scroll
regresses to the numbers above, and the fix in #2856 goes away with the props it edited. It is
invisible at 100 %, which is why the original instance went unnoticed.

@rolfheij-sil your note on #2781 — "that marker probably belongs on the verse blocks rather than on
the column's subgrid wrapper" — matches this. The measurements extend it slightly: blocks are safe,
the subgrid wrapper is not, and neither is anything above the port.

Caveat: this measures the generic DOM shape, not this PR's actual markup, which I have not read.
Worth checking where the marker ends up relative to [data-cell-content].


AI-assisted (Claude Code); the table is measured in headless Chromium, not inferred.

rolfheij-sil added a commit that referenced this pull request Sep 24, 2026
… not the branch's history

PR #2849 will squash-merge into main, so its ADR amendments and guide
changelog rows must describe the final state of the Text Collection's
per-resource zoom, not the removal-then-restoration that only existed
inside the branch (2026-09-23 removed it, 2026-09-24 restored it; neither
step ever reached main).

- adr-resource-panes-name-their-zoom-areas: merged the 2026-09-23 and
  2026-09-24 amendments into one, dated 2026-09-24, describing only the
  final architecture (per-resource resource-<id> areas, text-collection
  as fallback, menus running the platform's zoom commands, no grid wheel
  listener).
- adr-zoom-areas-mark-project-text (new in this PR): edited its
  Consequences bullet directly and dropped the self-amendment.
- adr-text-collection-resources-are-zoom-areas: rewrote Context and
  Alternatives to describe main's actual prior state (the grid's own
  per-resource CSS `zoom`, PT-4155, multiplying with the pane-wide
  text-collection area) instead of the branch-only pane-wide-only state.
- Component-Builder-Patterns.md / Extension-Development-Guide.md:
  collapsed the changelog row pairs that undid each other, renumbered
  contiguously from main's last row, and updated the front-matter
  version.
- Applied the review's M8/M9/M12 minor items in these three files, and
  fixed one further stale removal claim in adr-zoom-composition's
  amendment (same root cause, different entry) found via the grep
  safety net.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
@rolfheij-sil
rolfheij-sil marked this pull request as ready for review September 24, 2026 17:35
@rolfheij-sil
rolfheij-sil force-pushed the pt-4585-zoom-integration-e2e branch from 1b05e45 to 681501a Compare September 24, 2026 17:46
rolfheij-sil added a commit that referenced this pull request Sep 24, 2026
… not the branch's history

PR #2849 will squash-merge into main, so its ADR amendments and guide
changelog rows must describe the final state of the Text Collection's
per-resource zoom, not the removal-then-restoration that only existed
inside the branch (2026-09-23 removed it, 2026-09-24 restored it; neither
step ever reached main).

- adr-resource-panes-name-their-zoom-areas: merged the 2026-09-23 and
  2026-09-24 amendments into one, dated 2026-09-24, describing only the
  final architecture (per-resource resource-<id> areas, text-collection
  as fallback, menus running the platform's zoom commands, no grid wheel
  listener).
- adr-zoom-areas-mark-project-text (new in this PR): edited its
  Consequences bullet directly and dropped the self-amendment.
- adr-text-collection-resources-are-zoom-areas: rewrote Context and
  Alternatives to describe main's actual prior state (the grid's own
  per-resource CSS `zoom`, PT-4155, multiplying with the pane-wide
  text-collection area) instead of the branch-only pane-wide-only state.
- Component-Builder-Patterns.md / Extension-Development-Guide.md:
  collapsed the changelog row pairs that undid each other, renumbered
  contiguously from main's last row, and updated the front-matter
  version.
- Applied the review's M8/M9/M12 minor items in these three files, and
  fixed one further stale removal claim in adr-zoom-composition's
  amendment (same root cause, different entry) found via the grep
  safety net.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
@rolfheij-sil

Copy link
Copy Markdown
Contributor Author

🤖 Claude: Thanks — the measurement matches our reading of the unit mismatch, and #2849's placement is your "child(pad)" row.

In #2849 the cell's only zoom comes from the platform rule on the ContentZoomRoot marker, and in the header (chapter) layout that marker sits two levels inside the scroll box (resource-cell-view.component.tsx @ #2849 head):

The inline layout is the same with the marker as the scroll box's direct child (:318/:319). The zoomFactor/zoomTarget/contentStyle chain is gone, so nothing else in the cell sets zoom, and the Text Collection has no marker above the column or grid (the column carries only data-platform-content-zoom-scope, which has no CSS rule). The area is now per resource, resource-<id>, not text-collection.

It is pinned by resource-cell-view.component.test.tsx ("keeps the scroll box and its padding outside the marker", both layouts): the marker must be strictly inside the overflow-auto box. We wrote that for scrollbar and padding size, but it fails on exactly the "on port" and "ancestor" placements.

For #2856's rebase onto #2849: the contentStyle move is no longer needed. Keep ref={contentRef} and data-cell-content on the :395 box, and consider changing the pad.style.zoom assertions to "the zoom marker is inside [data-cell-content] and not on it". That lets the scroll-math reason live next to useReferenceScroll, which isn't in #2849.

For #2781's aligned view, the ADR note in #2849 (adr-text-collection-resources-are-zoom-areas, Consequences, in .context/standards/Architecture-Decisions.md) still applies. Either put the marker on each .verse-block and skip the cell's own marker, or keep the cell marker with its zoom neutralised and feed --aligned-zoom from var(--platform-content-zoom-resource-<id>, var(--platform-content-zoom-default, 1)). Never mark the subgrid box. Both options keep zoom below the grid-root port. The subgrid box is excluded because of track rescaling rather than scroll math, so your rule and ours agree.

@lyonsil lyonsil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the content-zoom realignment in this PR: 8 findings. Each comment's severity is the one the reviewer settled on after checking it; checked and confirmed means the finding was verified against the code at this head, and needs a human call means it could not be settled either way and is yours to judge.

Findings that could not be attached to a line in this diff

#3 (low · checked and confirmed) extensions/src/platform-scripture-editor/src/platform-scripture-editor-footnotes.component.tsx:285 - Zooming the editor's footnotes pane shows its level in the Scripture text's top-right corner with nothing saying it belongs to the footnotes.

What happens: placeBadge (web-view-content-zoom.bootstrap-script.ts, around line 906) pins the badge 12px/16px from the iframe's corner rather than the zoomed area's own corner. labelOf (line 889) names an area only from data-platform-content-zoom-label, which only Text Collection cells set. The footnotes ContentZoomRoot here passes no label.

Why it matters: With the footnotes pane open, the user cannot tell which pane changed; Enhanced Resources' footnotes and entries areas share the gap.

Fix: Pass label to this ContentZoomRoot from a new localized key in the extension's localizedStrings.json (en and es), and add a test that the marker carries the label.

How this was checked: Confirmed at extensions/src/platform-scripture-editor/src/platform-scripture-editor-footnotes.component.tsx:285, which wraps the footnote list in <ContentZoomRoot area="footnotes" ...> with no label prop. ContentZoomRoot (lib/platform-bible-react/src/components/advanced/content-zoom-root.component.tsx:96) only writes the data-platform-content-zoom-label attribute when label is passed. In web-view-content-zoom.bootstrap-script.ts, placeBadge (line ~867) always pins the zoom badge to the iframe's own top-right corner via INDICATOR_INSET_TOP/INDICATOR_INSET_INLINE (12px/16px, lines 799-800), and labelOf (line 887) only finds a name if some marker for that area carries the label attribute. This PR (diff against 70e1fdc) newly introduces the label prop on ContentZoomRoot and the badge's label-reading logic, and uses it only at extensions/src/platform-scripture-editor/src/scripture-text-grid/resource-cell-view.component.tsx:319/397 (Text Collection cells) - it does not touch the footnotes file. So zooming the footnotes pane shows a bare "N%" badge at the editor iframe's corner, indistinguishable from zooming the Scripture text itself.

Not attached to a line: not on a changed line; cannot be posted inline

#6 (low · checked and confirmed) src/renderer/components/overlay-host.component.tsx:56 - The overlays still thread an iframe frame scale that is now always 1, so the popover and palette keep logic that can never change their output.

What happens: Nothing sets iframe.style.zoom (pushContentZoom no longer writes it), so getWebViewIframeZoom (overlay-coordinates.ts:52) always returns 1, and frameScale on OverlayPopover and OverlayCommandPalette, plus the scaling in translateCoordinates (line 64), have no effect.

Why it matters: It is maintenance weight that reads as live behavior; the TSDoc has to explain that it is always 1.

Fix: Remove the frameScale prop, its multiplication of anchor?.width/anchor?.height, and its TSDoc from both OverlayPopoverPresentationalProps/OverlayPopover (overlay-popover.component.tsx:59,172,213-214,337-342,392) and OverlayCommandPalettePresentationalProps/OverlayCommandPalette (overlay-command-palette.component.tsx:83,356,664-665,771,776,893). Remove the getWebViewIframeZoom import and the two frameScale={getWebViewIframeZoom(...)} call sites in overlay-host.component.tsx:11,56,65. In overlay-coordinates.ts, delete getWebViewIframeZoom, drop the zoom multiplication in translateCoordinates (return {x: rect.left + position.x, y: rect.top + position.y}), and delete parseIframeZoom once both call sites are gone, since nothing else in the file uses it. Update overlay-coordinates.test.ts (drop the getWebViewIframeZoom describe block and the zoom-multiplication cases for translateCoordinates), overlay-popover.component.test.tsx and overlay-command-palette.component.test.tsx (drop the "forwards frameScale" tests), and overlay-host.component.test.tsx (drop the frameScale/getWebViewIframeZoom mocks and assertions). Because getWebViewIframeZoom is declared in the generated lib/papi-dts/papi.d.ts (module renderer/services/overlays/overlay-coordinates), finish by running npm run build:types to regenerate it rather than hand-editing the removal.

How this was checked: Confirmed. This PR's own diff removes the only writer of the whole-iframe zoom: the merge-base version of web-view-content-zoom.service.ts had iframe.style.zoom = mayScaleWholeIframe(webViewId) ? String(defaultZoom) : '', and HEAD's pushContentZoom (web-view-content-zoom.service.ts:867-897) no longer sets it at all, with a comment stating "The iframe element itself is never scaled." Because of that, parseIframeZoom (overlay-coordinates.ts:27-29) always sees an empty style.zoom and falls back to 1, so getWebViewIframeZoom (overlay-coordinates.ts:52) always returns 1. overlay-host.component.tsx:56 and :65 (both lines this PR edited, per the diff removing the sibling contentScale prop) still pass that value as frameScale into OverlayPopover and OverlayCommandPalette, which multiply anchor?.width/anchor?.height by it (overlay-popover.component.tsx:213-214, overlay-command-palette.component.tsx:664-665) - always a no-op multiply-by-1. translateCoordinates's zoom multiplication (overlay-coordinates.ts:64-74) is equally inert. The function's own TSDoc already concedes this ("currently sets no whole-iframe zoom ... so this returns 1"), which is exactly the maintenance-weight the finding describes.

Not attached to a line: not on a changed line; cannot be posted inline

(AI-assisted, with my guidance)

Comment thread extensions/src/platform-scripture/src/checklist.web-view.tsx
Comment thread lib/platform-bible-utils/src/content-zoom.util.ts
Comment thread src/renderer/services/web-view-content-zoom.bootstrap-script.ts
Comment thread src/renderer/services/web-view-content-zoom.service.ts Outdated
Comment thread src/shared/models/content-zoom.model.ts Outdated
@rolfheij-sil
rolfheij-sil force-pushed the pt-4585-zoom-integration-e2e branch from 681501a to e05360f Compare September 25, 2026 09:01
rolfheij-sil added a commit that referenced this pull request Sep 25, 2026
… not the branch's history

PR #2849 will squash-merge into main, so its ADR amendments and guide
changelog rows must describe the final state of the Text Collection's
per-resource zoom, not the removal-then-restoration that only existed
inside the branch (2026-09-23 removed it, 2026-09-24 restored it; neither
step ever reached main).

- adr-resource-panes-name-their-zoom-areas: merged the 2026-09-23 and
  2026-09-24 amendments into one, dated 2026-09-24, describing only the
  final architecture (per-resource resource-<id> areas, text-collection
  as fallback, menus running the platform's zoom commands, no grid wheel
  listener).
- adr-zoom-areas-mark-project-text (new in this PR): edited its
  Consequences bullet directly and dropped the self-amendment.
- adr-text-collection-resources-are-zoom-areas: rewrote Context and
  Alternatives to describe main's actual prior state (the grid's own
  per-resource CSS `zoom`, PT-4155, multiplying with the pane-wide
  text-collection area) instead of the branch-only pane-wide-only state.
- Component-Builder-Patterns.md / Extension-Development-Guide.md:
  collapsed the changelog row pairs that undid each other, renumbered
  contiguously from main's last row, and updated the front-matter
  version.
- Applied the review's M8/M9/M12 minor items in these three files, and
  fixed one further stale removal claim in adr-zoom-composition's
  amendment (same root cause, different entry) found via the grep
  safety net.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
@rolfheij-sil

Copy link
Copy Markdown
Contributor Author

🤖 Claude: Replies to the two findings from the review body above that could not be attached to a line:

#3 (low) … Zooming the editor's footnotes pane shows its level in the Scripture text's top-right corner with nothing saying it belongs to the footnotes. … Enhanced Resources' footnotes and entries areas share the gap.

Agreed, and fixed in 1ee52df. All three areas now name themselves on the badge:

  • Scripture editor footnotes: "Footnotes · 120 %", using the existing %webView_footnoteList_header% string (Spanish "Notas al pie").
  • Enhanced Resources footnotes: a new %enhancedResources_footnotesPane_zoomAreaLabel% string, in English and Spanish.
  • Enhanced Resources lower panel: named after the tab on screen ("Dictionary · 120 %", "Encyclopedia · 120 %"), reusing the tab bar's own strings.

The entries text is marked through ContentZoomTextProvider, not a ContentZoomRoot, so the provider gains an optional label (TSDoc @experimental) that every text element it marks carries, including the inventories' occurrences table (492e882). The platform-bible-react output is rebuilt. Tests: each marker carries its label, and the provider writes a label only when it has a non-empty one.

#6 (low) … The overlays still thread an iframe frame scale that is now always 1 …

Agreed, and removed in a2b5764:

  • frameScale from OverlayPopover and OverlayCommandPalette (both layers), and the two OverlayHost call sites
  • getWebViewIframeZoom and parseIframeZoom
  • the zoom multiplication in translateCoordinates

The tests now pin the anchor at the size the pane measured, and the host test keeps a render case for a popover and a palette. papi.d.ts is regenerated; the only type change is that getWebViewIframeZoom disappears. adr-pop-ups-stay-at-interface-scale is updated to match.

rolfheij-sil and others added 17 commits September 25, 2026 15:54
The per-resource zoom spec aimed its second notch at the center of the
chapter panel's zoom marker. The panel holds the whole chapter, so that
marker is about 2000 px tall and its center lay below the 1079 px
window: the wheel reached no element, the in-frame listener saw no
event, and resource A stayed at 1.1. Aimed at the part of the marker
inside the web view's frame, the same notch raises A to 1.2.

Factor the frame clipping out of `areaBox` into `onScreenBox`, which
takes any locator, and use it for the panel text.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
`switchToChapterView` closes the View Options popover with Escape, which
returns focus to the View Options button, and a focused trigger keeps
its tooltip open (`data-state="instant-open"`) until focus moves. That
tooltip hangs over the header end of the last column, where its "⋮"
sits, so every click on resource B's "Zoom options" button was
intercepted until the test timed out. Hovering the column does not move
focus, so it did not help.

`openChapterViewZoomOptions` now blurs the focused element and waits for
no tooltip to be open before it hovers the column and clicks.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
…word the area label TSDoc

`useResourceContentZoom` treated any finite number under
`platform.contentZoomLevels` as a resource's own level, and read the
stored value as a map without checking it. The platform ignores a
non-map value and any level outside MIN_ZOOM_FACTOR..MAX_ZOOM_FACTOR
when it applies the levels, so the menus could report a level (and
enable Reset) the text did not have, and a `null` map threw. The hook
now applies the same checks.

The `CONTENT_ZOOM_LABEL_ATTRIBUTE` TSDoc said areas without a label are
"shown as before"; it now says an area without a label shows the level
alone. papi.d.ts regenerated with `npm run build:types`; the library's
mirrored constant does not carry that sentence, so its build output is
unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
…text marker

The `adr-text-collection-resources-are-zoom-areas` consequence for the
verse-aligned Grid view said it can mark each verse block with its
resource's area. That holds only if the cell does not also mark its
text: the cell's `ContentZoomRoot` would enclose those markers, nested
markers are ignored, and the wrapper is one more level in the aligned
view's subgrid/`display:contents` chain. Name the two ways out.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
… not the branch's history

PR #2849 will squash-merge into main, so its ADR amendments and guide
changelog rows must describe the final state of the Text Collection's
per-resource zoom, not the removal-then-restoration that only existed
inside the branch (2026-09-23 removed it, 2026-09-24 restored it; neither
step ever reached main).

- adr-resource-panes-name-their-zoom-areas: merged the 2026-09-23 and
  2026-09-24 amendments into one, dated 2026-09-24, describing only the
  final architecture (per-resource resource-<id> areas, text-collection
  as fallback, menus running the platform's zoom commands, no grid wheel
  listener).
- adr-zoom-areas-mark-project-text (new in this PR): edited its
  Consequences bullet directly and dropped the self-amendment.
- adr-text-collection-resources-are-zoom-areas: rewrote Context and
  Alternatives to describe main's actual prior state (the grid's own
  per-resource CSS `zoom`, PT-4155, multiplying with the pane-wide
  text-collection area) instead of the branch-only pane-wide-only state.
- Component-Builder-Patterns.md / Extension-Development-Guide.md:
  collapsed the changelog row pairs that undid each other, renumbered
  contiguously from main's last row, and updated the front-matter
  version.
- Applied the review's M8/M9/M12 minor items in these three files, and
  fixed one further stale removal claim in adr-zoom-composition's
  amendment (same root cause, different entry) found via the grep
  safety net.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
Bible Texts, Commentaries and Model Text wrapped their zoom marker around
the editor's scroll box, so scrollToVerse added a zoomed-pixel rect
distance to the box's unzoomed scrollTop and overshot by the zoom factor.
The marker now sits inside the scroll box, around Editorial, as the Text
Collection cell and the Scripture editor already do; the messages and
spinner stay at interface size. Component tests pin the placement, and
adr-zoom-areas-mark-project-text records the rule.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
Only the entry text in the lower panel is marked, so Ctrl/Cmd+wheel over a
card's padding, the gaps between entries or the tab bar reached no marker
and zoomed the area used last, usually the Bible text. The lower
ResizablePanel now carries data-platform-content-zoom-scope="entries", the
platform's remedy for unmarked space that belongs to one area. A contract
test pins the attribute; the bootstrap's "zoom scope" tests cover how a
scope resolves.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
The grid declared `text-collection` as its default area although no
resource's text is marked with it: an empty grid showed a badge for a
level nothing read, and a grid-wide level remembered from the shipped
build was seeded into state where no text used it. ContentZoomDeclaration's
defaultArea is now optional and the grid omits it, so a grid showing no
resource is not zoomable (no items, chords, wheel, level or badge) and
becomes zoomable when its first resource renders. `text-collection`
remains only the shared fallback for a resource id that yields no area.
The Text Collection ADR entries record the final state and that a
grid-wide level from the shipped build is not carried over.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
…badge

Zooming a footnotes pane showed a bare level in the tab's corner, the same
badge the Scripture text shows, so the user could not tell which pane had
changed. The Scripture editor's footnotes pane now names its area with the
existing "Footnotes" string, the Enhanced Resources footnotes pane with a
new "Footnotes" / "Notas al pie" string, and the Enhanced Resources lower
panel after the tab on screen ("Dictionary · 120 %"), reusing the tab
bar's strings.

The entries text is marked through ContentZoomTextProvider, so the
provider gains an optional experimental `label` that every text element it
marks carries; platform-bible-react dist rebuilt.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
…entity

The comment above NO_COMPARATIVE_TEXTS described useWebViewState swapping
in the caller's latest default on every unrelated state write, which the
hook no longer does. The constants stay; the comment now gives the reason
that still holds: the values sit in effect dependency lists, as
useWebViewState's documentation advises.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
…tion

adjustZoomFactor added a step and rounded to the nearest tenth, so after a
typed 137 % the + button, the pane zoom keys and the Interface-scaling
keys jumped to 150 %, skipping 140 %. An off-grid factor now first moves
to the grid mark it has passed in the direction of travel (floor for +,
ceil for -, with a 1e-9 tolerance for float noise in the tenths), then
steps, clamps and rounds: 137 % goes to 140 % on + and 130 % on -.
Utility, stepper and Settings e2e expectations, the stepper TSDoc and the
platform.zoomIn/zoomOut docs updated; platform-bible-utils dist rebuilt
(platform-bible-react's rebuilt dist is byte-identical) and papi.d.ts
regenerated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
Content zoom scales marked areas inside a pane and never the iframe
element, so nothing writes iframe.style.zoom any more and the overlay
frame scale was always 1. Removed: the frameScale prop from
OverlayPopover and OverlayCommandPalette (presentational and store-
connected), the OverlayHost calls that supplied it, getWebViewIframeZoom
and parseIframeZoom, and the zoom multiplication in translateCoordinates.
Tests now pin the anchor at the size the pane measured; the host test
keeps a render case for popovers and palettes. papi.d.ts regenerated
(one experimental function removed); adr-pop-ups-stay-at-interface-scale
updated to match.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
…t area scan

Every zoom step ran labelOf, and every placed frame ran isRtlArea, each a
document-wide querySelectorAll over every marker; with text-level
marking a book-length checklist, Find or inventory carries thousands of
markers, at up to 120 wheel notches a second. collectAreas now records
each area's first accepted marker and first non-empty label during the
scan it already runs on marker-changing mutations, labelOf and isRtlArea
read that record, and the observer also watches the label attribute so a
rename refreshes it. Tests count querySelectorAll calls: none on a zoom
step, one for a rename.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
…n updates

The onDidUpdateWebView handler dropped the pane's cached declaration on
every update, and updates arrive for every zoom step and every
useWebViewState write, so the next tab-title render walked the dock
layout again just to re-read the pane's type. That type cannot change
while the pane is open (webViewType is not an updatable definition
property, and a new view always gets a new id), so the cache is now
dropped only when the pane is forgotten. The test pins that an update
keeps the declaration.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
Follow-ups from reviewing this round's own changes:
- OccurrencesTable picked only the marker attribute out of
  useContentZoomTextProps, so a labelled provider's name never reached
  its cells; it now passes the label attribute too (test added,
  platform-bible-react dist rebuilt).
- The Enhanced Resources footnotes label falls back to no name rather
  than the raw string key, matching the entries label.
- The tab bar reads its captions from RESEARCH_TAB_LABEL_KEYS, so the tab
  captions and the entries area's name share one source; the test that
  only compared the constant with a copy of itself is dropped.
- The bootstrap observer comment names both watched attributes, and a
  service test fixture comment says what it is for.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
@rolfheij-sil
rolfheij-sil force-pushed the pt-4585-zoom-integration-e2e branch from 492e882 to c04243b Compare September 25, 2026 13:59
@rolfheij-sil
rolfheij-sil merged commit d54baad into main Sep 25, 2026
6 of 7 checks passed
@rolfheij-sil
rolfheij-sil deleted the pt-4585-zoom-integration-e2e branch September 25, 2026 15:39
rolfheij-sil added a commit that referenced this pull request Sep 25, 2026
Matt's PR #2849 follow-up (issue comment 5833335163), round-1 verified.

1. Add a contract test to each web view's `content-zoom-markers.contract.test.ts`
   pinning that the footnotes pane's zoom-area label is wired from a loaded
   localize key: `platform-scripture-editor.web-view.tsx`'s `FootnotesLayout`
   `zoomAreaLabel` prop and its `EDITOR_LOCALIZED_STRINGS` entry, and
   `enhanced-resource.web-view.tsx`'s `EnhancedResourceFootnotesPane`
   `zoomAreaLabel` prop and its `ENHANCED_RESOURCE_WEB_VIEW_STRING_KEYS` entry.
   Without either test, dropping the prop or the key still passed every test —
   the component tests cover the pane, not the view that feeds it. Verified
   each test fails when the prop is removed and when the key is dropped from
   its loaded list (both reverted after confirming red); Matt's regexes
   matched the source as written, so neither needed adjusting.

2. Fix `getContentZoomBootstrapScript`'s `declaredArea` param doc: it reads
   `undefined` both for a web view type core does not declare and for one
   declared with no default area (e.g. the Text Collection grid), not only
   the former.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
tjcouch-sil added a commit that referenced this pull request Sep 25, 2026
…-editing

Main reworked content zoom (#2849). The footnotes pane keeps its list wrapper
as the footnotes zoom area and now passes main's zoomAreaLabel through to it;
the web view passes both the pane's editing props and the label. The pbr
dist is rebuilt rather than taken from either side.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/060fba48-2dc4-4718-9172-4a835abd6dc0
rolfheij-sil added a commit that referenced this pull request Sep 25, 2026
Matt's PR #2849 follow-up (issue comment 5833335163), round-1 verified.

1. Add a contract test to each web view's `content-zoom-markers.contract.test.ts`
   pinning that the footnotes pane's zoom-area label is wired from a loaded
   localize key: `platform-scripture-editor.web-view.tsx`'s `FootnotesLayout`
   `zoomAreaLabel` prop and its `EDITOR_LOCALIZED_STRINGS` entry, and
   `enhanced-resource.web-view.tsx`'s `EnhancedResourceFootnotesPane`
   `zoomAreaLabel` prop and its `ENHANCED_RESOURCE_WEB_VIEW_STRING_KEYS` entry.
   Without either test, dropping the prop or the key still passed every test —
   the component tests cover the pane, not the view that feeds it. Verified
   each test fails when the prop is removed and when the key is dropped from
   its loaded list (both reverted after confirming red); Matt's regexes
   matched the source as written, so neither needed adjusting.

2. Fix `getContentZoomBootstrapScript`'s `declaredArea` param doc: it reads
   `undefined` both for a web view type core does not declare and for one
   declared with no default area (e.g. the Text Collection grid), not only
   the former.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
katherinejensen00 added a commit that referenced this pull request Sep 26, 2026
Rebased onto main after PT-4585 (#2849) moved the zoom markers inside the
editor scroll boxes and reworked the resource cell's right-click menu:

- Portalled right-click test and copyright stories use the new cell props
  (required zoomArea, menuStrings fixture, Copy-only menu)
- Grid test's verse-text ids no longer match the /^cell-/ cell count
- Rebuilt platform-bible-react and platform-bible-utils dist

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
katherinejensen00 added a commit that referenced this pull request Sep 28, 2026
Rebased onto main after PT-4585 (#2849) moved the zoom markers inside the
editor scroll boxes and reworked the resource cell's right-click menu:

- Portalled right-click test and copyright stories use the new cell props
  (required zoomArea, menuStrings fixture, Copy-only menu)
- Grid test's verse-text ids no longer match the /^cell-/ cell count
- Rebuilt platform-bible-react and platform-bible-utils dist

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
katherinejensen00 added a commit that referenced this pull request Sep 29, 2026
Rebased onto main after PT-4585 (#2849) moved the zoom markers inside the
editor scroll boxes and reworked the resource cell's right-click menu:

- Portalled right-click test and copyright stories use the new cell props
  (required zoomArea, menuStrings fixture, Copy-only menu)
- Grid test's verse-text ids no longer match the /^cell-/ cell count
- Rebuilt platform-bible-react and platform-bible-utils dist

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rolfheij-sil added a commit that referenced this pull request Oct 1, 2026
Matt's PR #2849 follow-up (issue comment 5833335163), round-1 verified.

1. Add a contract test to each web view's `content-zoom-markers.contract.test.ts`
   pinning that the footnotes pane's zoom-area label is wired from a loaded
   localize key: `platform-scripture-editor.web-view.tsx`'s `FootnotesLayout`
   `zoomAreaLabel` prop and its `EDITOR_LOCALIZED_STRINGS` entry, and
   `enhanced-resource.web-view.tsx`'s `EnhancedResourceFootnotesPane`
   `zoomAreaLabel` prop and its `ENHANCED_RESOURCE_WEB_VIEW_STRING_KEYS` entry.
   Without either test, dropping the prop or the key still passed every test —
   the component tests cover the pane, not the view that feeds it. Verified
   each test fails when the prop is removed and when the key is dropped from
   its loaded list (both reverted after confirming red); Matt's regexes
   matched the source as written, so neither needed adjusting.

2. Fix `getContentZoomBootstrapScript`'s `declaredArea` param doc: it reads
   `undefined` both for a web view type core does not declare and for one
   declared with no default area (e.g. the Text Collection grid), not only
   the former.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KTCXDyYLo9NMf8wGoDdWSc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants