Skip to content

PT-4579: Percent stepper for the content-zoom default; Settings layout - #2818

Closed
rolfheij-sil wants to merge 7 commits into
pt-4576-content-zoom-platform-corefrom
pt-4579-content-zoom-settings-stepper
Closed

rolfheij-sil wants to merge 7 commits into
pt-4576-content-zoom-platform-corefrom
pt-4579-content-zoom-settings-stepper

Conversation

@rolfheij-sil

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

Copy link
Copy Markdown
Contributor

Stacked on #2803 (pt-4576-content-zoom-platform-core). Review only the commits above that branch. Stacked PRs get no automatic CI here; the full local battery (typecheck, lint, format:check, vitest run src: 263 files / 3833 tests) is green. After #2803 squash-merges this branch is rebased with git rebase --onto main <old parent tip>.

Summary

Work item 4 of the per-pane content-zoom epic (PT-4575): the Settings side of content zoom, plus the General / Supporter layout the Epic Lead agreed with UX on 2026-09-14.

  • Percent stepper. platform.webViewContentZoom renders as − · 100 % · + · reset instead of a text box holding a decimal factor. A new renderer-internal PercentStepper (Button + lucide icons, role="group", localized aria-labels, aria-live readout, buttons disabled at the bounds and reset disabled at the default). It writes through the existing validate → setSetting tail without the text box's 500 ms debounce, so a press reaches following panes immediately (PT-3236 is sidestepped for this control only). Bounds, step and default come from the shared content-zoom constants; rounding follows the caller's step.
  • Relabels. "Zoom factor" → Interface Scaling through a new key (%settings_platform_zoomFactor_label_2%, with a deprecation entry for the shipped key in metadata.json, per the Localization Guide's immutable-strings rule); "Zoom" → Tab content default zoom in place (never shipped); the content-zoom description is rewritten in percent terms and names Interface Scaling. Spanish keeps the two distinguishable ("Escalado de la interfaz" / "Zoom predeterminado del contenido de la pestaña"). platform.zoomFactor's control and behaviour are untouched (PT-3458 stays a no-go).
  • Layout. General now reads Interface Language, Interface Scaling, Tab content default zoom (third, not first). A new Supporter settings group holds Request timeout and Show re-registration reminder at startup (keys, defaults and validators unchanged). Interface mode is hidden from Settings: its switch is the Simple/Power toggle in the profile popover, which the toolbar renders in both modes (user-profile-popover.component.tsx, platform-bible-toolbar.tsx), so a Settings entry would be a second switch for the same value.

Nothing zooms yet on its own: the default this control edits is applied by #2803 to panes that mark a zoom area, and the first such pane is PT-4581.

Why review this

Part of the current epic (PT-4575). This is the user-facing default for content zoom and the Settings relabel the PRD's terminology rule depends on.

Decisions worth a look

  • The stepper does not call adjustZoomFactor. That helper hard-codes the shared step and bounds, which would make the component's min/max/step props decorative. It clamps to the props and rounds to the step's precision; the one call site passes the shared constants, so behaviour is identical.
  • No optimistic readout. The stored value round-trips through the extension host before the prop updates, so two quick presses would both read the pre-press value. The readout always shows the stored value; only the arithmetic baseline for a rapid next press uses the last emitted factor, within a 1.5 s window and until the prop moves. Two earlier attempts at an optimistic readout each opened a stuck-or-flicker case (caught by the per-commit reviews), which is why the display is deliberately not optimistic.
  • handleChangeSetting accepts a raw number for the stepper, next to the existing change-event, boolean and string-array cases, and is now a useCallback (it joined generateComponent's dependency list).
  • "Interface Scaling" is title case as specified by UX, while the Localization Guide prescribes sentence case and the neighbouring labels are mixed. Left as specified; raise with UX if the house rule should win.
  • Spanish strings added, not just relabelled: the Supporter group label ("Configuración de soporte", a translator may prefer another word for the supporter role) and the two registration-reminder strings that es.json had been missing, which the new group would otherwise show half in English.
  • Studio's repo-patches/paranext-core.patch overrides the two relabelled keys and the General group metadata; that refresh belongs to PT-4585.
  • No e2e changes needed. A broad grep of e2e-tests and the settings components found no assertion on the old labels or the General-group order; the zoom hits there are the Enhanced Resources media viewer, a different feature.

Testing

  • Self-review: /review-paratext (four analyzer passes) and an OpenCodeReview delegate pass ran on the branch. One Critical (the in-place relabel of a shipped string, fixed via the new key) and the Important items are fixed here: tooltips on the three icon buttons and ButtonGroup per the Storybook guidelines, the glossary page, a story, the description in percent terms, and the stepper's readout following the stored value. Defaults applied without a ruling: "Interface Scaling" keeps UX's capital S although the capitalization guideline prefers sentence case; "Tab content default zoom" stays and its description explains it as the default for every pane in a tab; the Supporter group gained a description.
  • New percent-stepper.component.test.tsx (19 cases: display, step up/down/reset, bounds, disabled, keyboard reachability, group label, optimistic double-press, prop catch-up, quarter and five-hundredths steps, tenth-step exactness).
  • setting.component.test.tsx: the stepper renders for platform.webViewContentZoom and no text box; a press writes the stepped factor through setSetting; platform.zoomFactor still renders a text box.
  • core-settings-info.data.test.ts: visible General order; Supporter group membership; moved settings keep keys, labels, defaults and validators; Interface mode hidden; every setting declared exactly once across groups.
  • Root battery green (see the stacked note); shipped-locale-assets.test.ts green.
  • roborev per-commit reviews triaged (one refuted: the group label key lands in the next commit of the same branch; one fixed: step-precision rounding).
  • Manual check in the running app pending (Settings order, live stepper, both modes, Spanish).

Risk Level

Low–Medium. Relabels two visible settings and regroups two more; the new control is confined to one setting key.

AI-assisted — Claude Code (Fable 5.1 coordinating; Opus design pass; per-task Sonnet agents under TDD; Sonnet verification pass).

🤖 Generated with Claude Code


This change is Reviewable

rolfheij-sil and others added 7 commits September 15, 2026 10:48
…pporter settings get their own group

Reorders the General settings group so visible settings read
interface language, zoom factor, content zoom, then the hidden
book-keeping entries. Interface mode is now hidden, since the
Simple/Power toggle lives in the profile popover and a Settings
entry would be a redundant second switch. Request timeout and the
registration reminder move into a new Supporter settings group,
for settings a support person adjusts rather than day-to-day
translation settings.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…a decimal

Add PercentStepper, a +/-/reset control that displays a factor as a
percentage and clamps every step to the caller's own min/max/step
rather than the shared zoom constants, so callers with different
bounds don't get a control that silently ignores them. Optimistic
display keeps two quick presses from racing the stored value's
round-trip through the platform's subscription.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ecimal factor

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…o a tenth

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ent wording for the zoom stepper

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ses are confirmed

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…presses step from the last emitted factor

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@rolfheij-sil

Copy link
Copy Markdown
Contributor Author

🤖 Claude: Superseded by #2821, which combines this PR with the other content-zoom work items into one PR stacked on #2803. All commits are preserved there unchanged.

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.

1 participant