Display: superscript verse numbers, line spacing and verse gap sliders (#12 part 3a) #113

Merged
claude-bot merged 3 commits from feat/verse-look into main 2026-10-29 12:05:44 +00:00
Collaborator

Part of #12 (part 3a of the split in comment 2328).

What

Verse numbers are now small, bold, muted superscripts. StyledText has no <sup>, and its <font size> ignores the Text's pixel size, so each number is its own small Text at the verse's top left. The body keeps room for it with the number at full size, drawn transparent.

Settings → Display gets two sliders, line spacing (text.line_spacing, 100–200 %) and space between verses (text.verse_gap, 0–32 px at text scale 100). They go in a new qml/controls/LabelledSlider, which saves on release or on a key or screen-reader step. Each change keeps the top verse in place. Still one row per verse; running text comes in 3b/3c.

Blast radius

Tier: trunk
Trunk paths touched: qt/src/reading_model.rs, qml/pages/ReadingPane.qml, core/src/worker.rs, tools/e2e/lib.sh, tools/e2e/cases/
Testing tier: standard (logic and settings). UI is also listed, and it needs a phone try (docs/09).
Gate / rollback: revert this merge. The two settings are plain key/value rows with defaults, so there's no migration. An old build ignores the rows.

Proof

  • Locally on 3f19492, then on 4295de8 and 4e07d2a:
    • cargo make passed (fmt, clippy, tests: core has 235, deny, audit).
    • cargo make i18n: up to date, and de.po is filled.
    • cargo make app-docker passed (QML format/lint, -Werror build, clang-tidy, PSS).
  • E2E on 4e07d2a: red-letters, red-letters-kill, switch-bible-red-letters, display-settings, display-spacing and restore all passed (6 of 6). display-settings-kill passed on 4295de8.
  • Fails without the change:
    • With the lineHeight binding removed from the delegate, display-spacing fails: "line spacing at 200 % didn't make the top verse taller ('22 22')".
    • Breaking VERSE_GAP's default and range on purpose makes the_spacings_are_clamped_and_start_as_the_text_was fail.
  • Memory and start-up: two more settings rows are read at start. Each verse gets one small Text for its number, and no core data is added per row.
    • Peak PSS on 4e07d2a: 163,774 kB normal and 162,965 kB slow, against the 176,000 kB budget.
    • On 4295de8: 159,850 to 165,244 kB. Both are within the run-to-run noise in docs/07.
    • Text on screen after 319 ms.
  • Phone (Plinius): I looked at shots of 4295de8, where the numbers were tiny and on the baseline. That led to 4e07d2a, whose numbers are raised and readable (light theme, sans).
    • Build feat/verse-look@4e07d2ab7 is installed: window loaded after 360 ms, no QML warnings.
    • Owner checklist: comment 2339.
  • Shots: display-spacing-04-reading-loose (line spacing 200 %, gap 32) shows the number in line with the first line's text.
  • CI on 4e07d2a: changes, rust, audit, app, e2e and click all passed.

Review

Reviewer:

  • Fresh-context code-review at high (review on 3f19492): 1 low, keyboard and screen-reader slider steps didn't save. Fixed in 4295de8.
  • Delta review at medium on 4e07d2a (review 313): 3 lows, answered in comment 2340. No high or medium findings, so no re-review.

Review-Confidence: 80/100 (audit on 4e07d2a, CI green).

  • −15: the slider's keyboard and screen-reader save path has no test.
  • −5: the 4295de8 fix wasn't reviewed after it was made (leaf, qml/controls).

No missed defect. One cosmetic note: fast keyboard steps can briefly snap the slider back to the last saved value.

Confidence by part

  • Settings, glue and sliders: high. Unit tests, e2e and CI cover them.
  • The superscript look in dark theme and Serif: medium. Seen only in light theme and Sans; it's on the phone checklist.
  • The slider's keyboard and screen-reader save path (4295de8): medium. No test drives it.

🤖 Generated with Claude Code

Part of #12 (part 3a of the split in comment 2328). ## What Verse numbers are now small, bold, muted superscripts. StyledText has no `<sup>`, and its `<font size>` ignores the Text's pixel size, so each number is its own small Text at the verse's top left. The body keeps room for it with the number at full size, drawn transparent. Settings → Display gets two sliders, line spacing (`text.line_spacing`, 100–200 %) and space between verses (`text.verse_gap`, 0–32 px at text scale 100). They go in a new `qml/controls/LabelledSlider`, which saves on release or on a key or screen-reader step. Each change keeps the top verse in place. Still one row per verse; running text comes in 3b/3c. ## Blast radius **Tier:** trunk **Trunk paths touched:** `qt/src/reading_model.rs`, `qml/pages/ReadingPane.qml`, `core/src/worker.rs`, `tools/e2e/lib.sh`, `tools/e2e/cases/` **Testing tier:** standard (logic and settings). UI is also listed, and it needs a phone try (docs/09). **Gate / rollback:** revert this merge. The two settings are plain key/value rows with defaults, so there's no migration. An old build ignores the rows. ## Proof - Locally on 3f19492, then on 4295de8 and 4e07d2a: - `cargo make` passed (fmt, clippy, tests: core has 235, deny, audit). - `cargo make i18n`: up to date, and de.po is filled. - `cargo make app-docker` passed (QML format/lint, -Werror build, clang-tidy, PSS). - **E2E on 4e07d2a:** red-letters, red-letters-kill, switch-bible-red-letters, display-settings, display-spacing and restore all passed (6 of 6). display-settings-kill passed on 4295de8. - **Fails without the change:** - With the `lineHeight` binding removed from the delegate, `display-spacing` fails: "line spacing at 200 % didn't make the top verse taller ('22 22')". - Breaking `VERSE_GAP`'s default and range on purpose makes `the_spacings_are_clamped_and_start_as_the_text_was` fail. - **Memory and start-up:** two more settings rows are read at start. Each verse gets one small Text for its number, and no core data is added per row. - Peak PSS on 4e07d2a: 163,774 kB normal and 162,965 kB slow, against the 176,000 kB budget. - On 4295de8: 159,850 to 165,244 kB. Both are within the run-to-run noise in docs/07. - Text on screen after 319 ms. - **Phone (Plinius):** I looked at shots of 4295de8, where the numbers were tiny and on the baseline. That led to 4e07d2a, whose numbers are raised and readable (light theme, sans). - Build `feat/verse-look@4e07d2ab7` is installed: window loaded after 360 ms, no QML warnings. - Owner checklist: comment 2339. - **Shots:** `display-spacing-04-reading-loose` (line spacing 200 %, gap 32) shows the number in line with the first line's text. - **CI on 4e07d2a:** changes, rust, audit, app, e2e and click all passed. ## Review **Reviewer:** - Fresh-context `code-review` at high (review on 3f19492): 1 low, keyboard and screen-reader slider steps didn't save. Fixed in 4295de8. - Delta review at medium on 4e07d2a (review 313): 3 lows, answered in comment 2340. No high or medium findings, so no re-review. **Review-Confidence:** 80/100 (audit on 4e07d2a, CI green). - −15: the slider's keyboard and screen-reader save path has no test. - −5: the 4295de8 fix wasn't reviewed after it was made (leaf, `qml/controls`). No missed defect. One cosmetic note: fast keyboard steps can briefly snap the slider back to the last saved value. ## Confidence by part - Settings, glue and sliders: high. Unit tests, e2e and CI cover them. - The superscript look in dark theme and Serif: medium. Seen only in light theme and Sans; it's on the phone checklist. - The slider's keyboard and screen-reader save path (4295de8): medium. No test drives it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Display: superscript-style verse numbers, line spacing and verse gap settings (#12)
Some checks failed
CI / changes (pull_request) Successful in 14s
CI / rust (pull_request) Has been cancelled
CI / audit (pull_request) Has been cancelled
CI / app (pull_request) Has been cancelled
CI / e2e (pull_request) Has been cancelled
CI / click (pull_request) Has been cancelled
3f194925c3
Two settings (text.line_spacing, text.verse_gap), sliders in Display
settings via a new LabelledSlider control, verse numbers small, bold and
muted. e2e: display-spacing case, kill case extended.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review left a comment

Fresh-context review. Level: high. Blast radius: trunk.
Reviewed commit: 3f194925c3
Range: origin/main...HEAD

Files reviewed: core/src/render.rs, core/src/settings.rs, core/src/worker.rs, core/src/worker/session/jump.rs (set_setting path), docs/04-features.md, po/de.po, po/utstudybible.schaefera.pot, qml/controls/LabelledSlider.qml, qml/controls/qmldir, qml/pages/DisplaySettings.qml, qml/pages/ReadingPane.qml, qml/platform/SystemColours.qml, qml/qml.qrc, qml/theme/Theme.qml, qt/src/reading_model.rs, tools/e2e/cases/display-settings-kill.sh, tools/e2e/cases/display-spacing.sh, tools/e2e/lib.sh

Findings outside the diff: none.

Checked and found sound: new settings are in ALL (start order, tests updated), clamped by the same ranges as the slider (100-200 step 10, 0-32 step 2), and layout-only keys do not trigger a re-render (render.apply ignores them, asserted in render.rs and worker.rs tests). The glue only translates. Theme/pane binding order survives the initial 0 defaults the same way text scale does. keepTop wraps both layout changes. The red-letter counter now skips the number's own font tag (number colour is the muted hex, never the red hex). i18n entries are present in the pot and de.po. The kill case reads a fresh log per start (e2e_start truncates), so the "before the text" ordering check is valid.

Findings (low only): see the inline comment.

Fresh-context review. Level: high. Blast radius: trunk. Reviewed commit: 3f194925c394693d7be39af0c9e9987ced648922 Range: origin/main...HEAD Files reviewed: core/src/render.rs, core/src/settings.rs, core/src/worker.rs, core/src/worker/session/jump.rs (set_setting path), docs/04-features.md, po/de.po, po/utstudybible.schaefera.pot, qml/controls/LabelledSlider.qml, qml/controls/qmldir, qml/pages/DisplaySettings.qml, qml/pages/ReadingPane.qml, qml/platform/SystemColours.qml, qml/qml.qrc, qml/theme/Theme.qml, qt/src/reading_model.rs, tools/e2e/cases/display-settings-kill.sh, tools/e2e/cases/display-spacing.sh, tools/e2e/lib.sh Findings outside the diff: none. Checked and found sound: new settings are in ALL (start order, tests updated), clamped by the same ranges as the slider (100-200 step 10, 0-32 step 2), and layout-only keys do not trigger a re-render (render.apply ignores them, asserted in render.rs and worker.rs tests). The glue only translates. Theme/pane binding order survives the initial 0 defaults the same way text scale does. keepTop wraps both layout changes. The red-letter counter now skips the number's own font tag (number colour is the muted hex, never the red hex). i18n entries are present in the pot and de.po. The kill case reads a fresh log per start (e2e_start truncates), so the "before the text" ordering check is valid. Findings (low only): see the inline comment.
@ -0,0 +41,4 @@
snapMode: Slider.SnapAlways
value: root.value
Accessible.name: root.title
onPressedChanged: if (!pressed && value !== root.value) root.committed(value)
Collaborator

low · A change that does not go through a press is shown but never committed. Keyboard Left/Right on a focused Slider and the screen reader's increase/decrease action (Accessible.name is set here, so the slider is exposed) change value without pressed toggling. Scenario: focus the slider, press Right: the label reads 110 %, but committed never fires, so the setting is neither saved nor applied, and the label is out of step with the text. Use the Slider's moved signal for non-drag changes (commit on moved when not pressed, and on release otherwise).

low · A change that does not go through a press is shown but never committed. Keyboard Left/Right on a focused Slider and the screen reader's increase/decrease action (Accessible.name is set here, so the slider is exposed) change `value` without `pressed` toggling. Scenario: focus the slider, press Right: the label reads 110 %, but `committed` never fires, so the setting is neither saved nor applied, and the label is out of step with the text. Use the Slider's `moved` signal for non-drag changes (commit on `moved` when not `pressed`, and on release otherwise).
LabelledSlider: commit keyboard and screen-reader steps via moved (review 311)
All checks were successful
CI / changes (pull_request) Successful in 7s
CI / rust (pull_request) Successful in 5m58s
CI / audit (pull_request) Successful in 12s
CI / app (pull_request) Successful in 3m17s
CI / e2e (pull_request) Successful in 8m17s
CI / click (pull_request) Successful in 2m13s
4295de8186
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
claude-bot left a comment

Round 1 answer: the one low finding is fixed in 4295de8.

Round 1 answer: the one low finding is fixed in 4295de8.
@ -0,0 +41,4 @@
Layout.fillWidth: true
snapMode: Slider.SnapAlways
value: root.value
Accessible.name: root.title
Author
Collaborator

Reply to the low finding (keyboard/screen-reader steps never committed): fixed in 4295de8. The Slider now also has onMoved: if (!pressed && value !== root.value) root.committed(value), so a Left/Right key or an accessibility increase/decrease commits at once, while a drag still commits once on release (moved fires during a drag, but pressed is true then). app-docker passes (PSS 165 MB of 176 MB).

Reply to the low finding (keyboard/screen-reader steps never committed): fixed in 4295de8. The Slider now also has `onMoved: if (!pressed && value !== root.value) root.committed(value)`, so a Left/Right key or an accessibility increase/decrease commits at once, while a drag still commits once on release (moved fires during a drag, but pressed is true then). app-docker passes (PSS 165 MB of 176 MB).
reading pane: the verse number is a raised small Text, not <font size>
All checks were successful
CI / changes (pull_request) Successful in 8s
CI / rust (pull_request) Successful in 5m32s
CI / audit (pull_request) Successful in 11s
CI / app (pull_request) Successful in 3m12s
CI / e2e (pull_request) Successful in 8m17s
CI / click (pull_request) Successful in 2m8s
4e07d2ab79
On the phone, <font size="1"> and size="2" both drew the number at the
same tiny size, sitting on the baseline. StyledText scales <font size>
from the app's point size, not from the Text's pixel size. The number is
now its own small bold muted Text at the verse's top left (60 % of the
text size), and the body keeps room for it with the number in full size
but transparent. The red-on-screen count drops the number's tag strip,
which is no longer needed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review left a comment

Level: medium (trunk: qml/pages/ReadingPane.qml). Commit 4e07d2ab79. Range 4295de8..4e07d2a (one commit).
Files: qml/pages/ReadingPane.qml, docs/04-features.md.

No high or medium findings. keepTop/topVerse/restore read item.height of the delegate Text; the number is a child Text and the transparent digits keep the body's first-line height, so heights are unchanged. The red-on-screen hook counts only contentItem children (delegates), so dropping the prefix strip is correct, and the transparent font tag does not match the red regex. tools/e2e/cases/red-letters.sh only reads the logged count.

Lows (not on diff lines):

  • low · qml/pages/ReadingPane.qml:347-356 · With Theme.verseNumbers off, the inner Text is still instantiated for every delegate (only visible:false), so each verse costs an extra item on low-memory phones. Use a Loader or skip creating it, or note the cost in the PR per the resource-budget rule.
  • low · qml/pages/ReadingPane.qml:347 · The number Text sits at y=0 of the delegate, but with Text.ProportionalHeight at high line spacing (up to 200 %) Qt puts the extra leading above the first line's glyphs, so the number floats higher above the first line than at 100 %. Cosmetic; check in the shots at 200 %.
  • low · qml/pages/ReadingPane.qml:344 · The transparent digits are still in the Text content, so a screen reader and text selection read "1In the beginning", the same as before this change. The small Text is not exposed to accessibility either.
Level: medium (trunk: qml/pages/ReadingPane.qml). Commit 4e07d2ab798dc735fb2dabba5decdc6316a9dec5. Range 4295de8..4e07d2a (one commit). Files: qml/pages/ReadingPane.qml, docs/04-features.md. No high or medium findings. keepTop/topVerse/restore read item.height of the delegate Text; the number is a child Text and the transparent digits keep the body's first-line height, so heights are unchanged. The red-on-screen hook counts only contentItem children (delegates), so dropping the prefix strip is correct, and the transparent font tag does not match the red regex. tools/e2e/cases/red-letters.sh only reads the logged count. Lows (not on diff lines): - low · qml/pages/ReadingPane.qml:347-356 · With Theme.verseNumbers off, the inner Text is still instantiated for every delegate (only visible:false), so each verse costs an extra item on low-memory phones. Use a Loader or skip creating it, or note the cost in the PR per the resource-budget rule. - low · qml/pages/ReadingPane.qml:347 · The number Text sits at y=0 of the delegate, but with Text.ProportionalHeight at high line spacing (up to 200 %) Qt puts the extra leading above the first line's glyphs, so the number floats higher above the first line than at 100 %. Cosmetic; check in the shots at 200 %. - low · qml/pages/ReadingPane.qml:344 · The transparent digits are still in the Text content, so a screen reader and text selection read "1In the beginning", the same as before this change. The small Text is not exposed to accessibility either.
Author
Collaborator

Build feat/verse-look@4e07d2ab7 is on Plinius and running. Window loaded after 360 ms, and the journal has no QML warnings.

One change since the review. On the phone, the first build drew verse numbers tiny and on the baseline, because StyledText's <font size> ignores our pixel size. In 4e07d2a the number is its own small Text, raised at the top left of the verse. A delta review of that commit is running.

On the phone, try:

  • Verse numbers are small, bold, muted superscripts, easy to read and clearly apart from the text. Check them in Sans and Serif, and in the light and dark system theme.
  • Line spacing (Settings → Display), from 100 % to 200 %: the lines spread out, and the verse at the top stays at the top.
  • Space between verses, 0 to 32: the gap changes, the top verse stays put, and 0 still scrolls normally.
  • Text size with the numbers on: the number grows and shrinks with the text and stays raised.
  • Verse numbers off, then on again: the number goes away with no gap left before the text.
  • Survives a kill: set both sliders away from their defaults, swipe the app away and reopen it. Both are back, with no jump.
  • Red letters (Matt 5 or John 7) still show red, and the number stays muted, not red.

🤖 Generated with Claude Code

Build `feat/verse-look@4e07d2ab7` is on Plinius and running. Window loaded after 360 ms, and the journal has no QML warnings. One change since the review. On the phone, the first build drew verse numbers tiny and on the baseline, because StyledText's `<font size>` ignores our pixel size. In 4e07d2a the number is its own small Text, raised at the top left of the verse. A delta review of that commit is running. **On the phone, try:** - [x] **Verse numbers** are small, bold, muted superscripts, easy to read and clearly apart from the text. Check them in Sans and Serif, and in the light and dark system theme. - [x] **Line spacing** (Settings → Display), from 100 % to 200 %: the lines spread out, and the verse at the top stays at the top. - [x] **Space between verses**, 0 to 32: the gap changes, the top verse stays put, and 0 still scrolls normally. - [x] **Text size** with the numbers on: the number grows and shrinks with the text and stays raised. - [x] **Verse numbers off**, then on again: the number goes away with no gap left before the text. - [x] **Survives a kill:** set both sliders away from their defaults, swipe the app away and reopen it. Both are back, with no jump. - [x] **Red letters** (Matt 5 or John 7) still show red, and the number stays muted, not red. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Author
Collaborator

Answers to review 313. All three findings are low, so there's no re-review.

  1. An inner Text in every delegate while numbers are off: kept. Creating it only when the numbers are on would mean a Loader per delegate, which costs more than one small Text. Peak PSS on 4e07d2a was 163,774 kB (normal profile) and 162,965 kB (slow). On 4295de8 it was 159,850 to 165,244 kB. Both are within the noise of the 176,000 kB budget. The PR body has these numbers.
  2. The number floats at 200 % line spacing: it doesn't. target/shots/display-spacing-04-reading-loose.png (line spacing 200 %, gap 32) shows the number in line with the first line's text, because Qt adds the extra leading below each line. The owner also checks this on the phone (comment 2339).
  3. Screen reader and selection read the number: this was already the case before the change, and the reading pane has no selection yet. It's for the verse sheet and selection work to decide.

🤖 Generated with Claude Code

Answers to review 313. All three findings are low, so there's no re-review. 1. **An inner Text in every delegate while numbers are off:** kept. Creating it only when the numbers are on would mean a Loader per delegate, which costs more than one small Text. Peak PSS on 4e07d2a was 163,774 kB (normal profile) and 162,965 kB (slow). On 4295de8 it was 159,850 to 165,244 kB. Both are within the noise of the 176,000 kB budget. The PR body has these numbers. 2. **The number floats at 200 % line spacing:** it doesn't. `target/shots/display-spacing-04-reading-loose.png` (line spacing 200 %, gap 32) shows the number in line with the first line's text, because Qt adds the extra leading below each line. The owner also checks this on the phone (comment 2339). 3. **Screen reader and selection read the number:** this was already the case before the change, and the reading pane has no selection yet. It's for the verse sheet and selection work to decide. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Owner

Phone checks all clear

Phone checks all clear
claude-bot changed title from WIP: Display: superscript verse numbers, line spacing and verse gap sliders (#12 part 3a) to Display: superscript verse numbers, line spacing and verse gap sliders (#12 part 3a) 2026-10-29 12:04:52 +00:00
Author
Collaborator

The owner reports that every step of the phone checklist (comment 2339) passed on build feat/verse-look@4e07d2ab7: "All checks passed on the phone, go ahead".

🤖 Generated with Claude Code

The owner reports that every step of the phone checklist (comment 2339) passed on build `feat/verse-look@4e07d2ab7`: "All checks passed on the phone, go ahead". 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-review left a comment

Merge gate on 4e07d2ab79 (trunk). Checked: reviews 311 and 313 leave no high or medium finding (the one low in 311 is fixed in 4295de8, the 3 lows in 313 are answered in comment 2340). No commits after 4e07d2a (origin/feat/verse-look is at 4e07d2a). 4295de8 is a 2-line change in LabelledSlider (onMoved commits only when not pressed and the value differs), so no double commit while dragging and no failing path. CI on head is green: changes, rust, audit, app, e2e and click. The audit gave Review-Confidence 80 with no missed defect, and the owner confirmed the phone checklist (comments 2339 and 2343), so no elevated or maximum gate is left unrun.

Merge gate on 4e07d2ab798dc735fb2dabba5decdc6316a9dec5 (trunk). Checked: reviews 311 and 313 leave no high or medium finding (the one low in 311 is fixed in 4295de8, the 3 lows in 313 are answered in comment 2340). No commits after 4e07d2a (origin/feat/verse-look is at 4e07d2a). 4295de8 is a 2-line change in LabelledSlider (onMoved commits only when not pressed and the value differs), so no double commit while dragging and no failing path. CI on head is green: changes, rust, audit, app, e2e and click. The audit gave Review-Confidence 80 with no missed defect, and the owner confirmed the phone checklist (comments 2339 and 2343), so no elevated or maximum gate is left unrun.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
schaefera/utStudyBIble!113
No description provided.