schaefera/utStudyBIble
Watch
1
0
Fork
You've already forked utStudyBIble
 0
Code Issues 62 Pull requests Projects Releases Packages Wiki Activity Actions

Verse and note sheets redesign (#148 step 2) #152

Merged
claude-bot merged 4 commits from feat/redesign-verse-sheet into main 2026-11-03 10:58:53 +00:00
Conversation 4 Commits 4 Files changed 18 +350 -80
claude-bot commented 2026-11-03 10:18:40 +00:00
Collaborator

What

The verse and footnote sheets in the Lapis style (board E5): rounded top, handle, serif title, inset rows, section labels, every label coloured for dark mode.
The verse sheet lists each note's words, not only its label. A reference sits beside its text, always in full; a range shows its first two verses, then " …" (owner's call at phone QC).
Without TSK the section says where references come from, with an outlined Install TSK (new OutlineButton).
Deviations from E5: no bookmark or note icons (no such features yet); the handle doesn't drag; the verse text stays on the sheet; Install TSK is 44 px high; the reference column is 104 px.

Blast radius

Tier: trunk
Trunk paths touched: core/src/bible.rs, core/src/bible/verse_sheet.rs, core/src/bible/reader/verse.rs, core/src/bible/reader/sheet.rs, tools/e2e/cases/
Testing tier: standard (also: ship on sight, for the styling)
Gate / rollback: no flag; revert this merge. No schema or stored data changes. Resources: a little more text in a sheet's JSON, and at most two verses read per reference where it was five; the slow profile's PSS was 186570 kB then 183534 kB on retry against the 185000 baseline, normal 172503 kB.

Proof

  • Rust gates: cargo make passes on the head (fmt, clippy, tests with coverage, release check, deny, audit).
  • CI: changes, rust, audit, app, e2e and click all green on bc115d6.
  • App gate: cargo make app-docker passes (-Werror build, clang-tidy, QML lint, PSS); CI's app job passes on the head.
  • Behaviour: verse-sheet-notes (new) logs note 'a Cited in 2 Corinthians 4:6' from Genesis 1:3's sheet; note-sheet, verse-copy, verse-sheet-no-tsk, verse-sheet-scroll, verse-sheet and verse-text-tap still pass, and the full e2e job passes in CI.
  • Tests fail without the change, by deliberate break with module data: range limit 1 fails a_range_of_two_verses_has_them_both and a_longer_range_has_two_verses_and_an_ellipsis; the old limit of 5 and no ellipsis each fail a_longer_range…; a note's body always its text fails a_note_of_references_alone_lists_them; JSON without the body fails the_sheet_is_json_with_or_without_tsk; empty bodies on the sheet fail a_verse_lists_its_notes_by_label.
  • UI: shots of the verse sheet (with notes, with references) and the footnote sheet looked at in light; no shot of the dark theme, which only the phone QC could cover.
  • Phone: the owner tried it on the phone and, on build bc115d62e, said "Works, go for it".

Review

Reviewer: fresh-context code-review at high (review 394): 3 findings (all low), 3 fixed, 0 declined, 0 deferred; re-review at medium of the later commits (review 395): 0 findings; 0 left open.
Review-Confidence: 100/100, both rounds together cover every trunk file up to the head, CI is green and the mutation checks show the new tests fail without the change.

Confidence by part

  • Core notes and range text: high, tested with module data and by deliberate breaks.
  • Sheet layout in light: high, seen in shots and on the phone.
  • Dark theme: medium, colours are the roles used elsewhere; no automated shot of it.
  • RefRows wrapping of a very long word (German book names) and the copy button's press tint: medium, no test exercises them.

🤖 Generated with Claude Code

## What The verse and footnote sheets in the Lapis style (board E5): rounded top, handle, serif title, inset rows, section labels, every label coloured for dark mode. The verse sheet lists each note's words, not only its label. A reference sits beside its text, always in full; a range shows its first two verses, then " …" (owner's call at phone QC). Without TSK the section says where references come from, with an outlined Install TSK (new `OutlineButton`). Deviations from E5: no bookmark or note icons (no such features yet); the handle doesn't drag; the verse text stays on the sheet; Install TSK is 44 px high; the reference column is 104 px. ## Blast radius **Tier:** trunk **Trunk paths touched:** `core/src/bible.rs`, `core/src/bible/verse_sheet.rs`, `core/src/bible/reader/verse.rs`, `core/src/bible/reader/sheet.rs`, `tools/e2e/cases/` **Testing tier:** standard (also: ship on sight, for the styling) **Gate / rollback:** no flag; revert this merge. No schema or stored data changes. Resources: a little more text in a sheet's JSON, and at most two verses read per reference where it was five; the slow profile's PSS was 186570 kB then 183534 kB on retry against the 185000 baseline, normal 172503 kB. ## Proof - Rust gates: `cargo make` passes on the head (fmt, clippy, tests with coverage, release check, deny, audit). - CI: changes, rust, audit, app, e2e and click all green on bc115d6. - App gate: `cargo make app-docker` passes (-Werror build, clang-tidy, QML lint, PSS); CI's app job passes on the head. - Behaviour: `verse-sheet-notes` (new) logs `note 'a Cited in 2 Corinthians 4:6'` from Genesis 1:3's sheet; `note-sheet`, `verse-copy`, `verse-sheet-no-tsk`, `verse-sheet-scroll`, `verse-sheet` and `verse-text-tap` still pass, and the full e2e job passes in CI. - Tests fail without the change, by deliberate break with module data: range limit 1 fails `a_range_of_two_verses_has_them_both` and `a_longer_range_has_two_verses_and_an_ellipsis`; the old limit of 5 and no ellipsis each fail `a_longer_range…`; a note's body always its text fails `a_note_of_references_alone_lists_them`; JSON without the body fails `the_sheet_is_json_with_or_without_tsk`; empty bodies on the sheet fail `a_verse_lists_its_notes_by_label`. - UI: shots of the verse sheet (with notes, with references) and the footnote sheet looked at in light; no shot of the dark theme, which only the phone QC could cover. - Phone: the owner tried it on the phone and, on build bc115d62e, said "Works, go for it". ## Review **Reviewer:** fresh-context `code-review` at high (review 394): 3 findings (all low), 3 fixed, 0 declined, 0 deferred; re-review at medium of the later commits (review 395): 0 findings; 0 left open. **Review-Confidence:** 100/100, both rounds together cover every trunk file up to the head, CI is green and the mutation checks show the new tests fail without the change. ## Confidence by part - Core notes and range text: high, tested with module data and by deliberate breaks. - Sheet layout in light: high, seen in shots and on the phone. - Dark theme: medium, colours are the roles used elsewhere; no automated shot of it. - `RefRows` wrapping of a very long word (German book names) and the copy button's press tint: medium, no test exercises them. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
sheets: the verse and note sheets in Lapis, with each note's words (#148)
Some checks failed
CI / changes (pull_request) Successful in 11s
Details
CI / rust (pull_request) Has been cancelled
Details
CI / audit (pull_request) Has been cancelled
Details
CI / app (pull_request) Has been cancelled
Details
CI / e2e (pull_request) Has been cancelled
Details
CI / click (pull_request) Has been cancelled
Details
b78645ca0e 
The verse sheet (board E5) and the footnote sheet: top corners, handle,
serif title, inset rows, section labels, references beside their text,
and an outlined Install TSK that says where the references come from.
The core now sends a note's words with its label, as the board lists
them. Every label on the sheets has its colour, so dark mode reads.

The boards' bookmark and note icons join when those features exist.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review reviewed 2026-11-03 10:20:44 +00:00
claude-review left a comment

Review level: high (blast radius: trunk; core/src/bible*, tools/e2e). Commit: b78645ca0e. Range: origin/main...HEAD. Single-pass review, no high or medium findings; three low ones, all inline.

Checked: VerseNote/labels zip alignment and the JSON shape (no other consumer of VerseSheet.notes), BottomSheet.refit() arithmetic against the new handle/header margins, the sheet fit with 5 refs and "Show all" within 60% of the 960 px e2e window (target/shots confirm it stays on screen), the new verse-sheet-notes e2e case (ordinal 6 = Genesis 1:3, log-line parsing), the existing verse-sheet*/verse-copy e2e cases against the changed layout, po/ against the changed qsTr strings, SectionLabel/SystemColours/OutlineButton registration (qmldir, qml.qrc).

Files reviewed:

  • core/src/bible.rs
  • core/src/bible/reader/verse.rs
  • core/src/bible/reader/sheet.rs
  • core/src/bible/reader.rs
  • core/src/bible/verse_sheet.rs
  • core/src/bible/note.rs
  • docs/10-design.md
  • po/de.po
  • po/lectern.aps.pot
  • qml/controls/BottomSheet.qml
  • qml/controls/NoteSwitch.qml
  • qml/controls/OutlineButton.qml
  • qml/controls/RefRows.qml
  • qml/controls/SectionLabel.qml
  • qml/controls/qmldir
  • qml/pages/NoteSheet.qml
  • qml/pages/VerseSheet.qml
  • qml/platform/SystemColours.qml
  • qml/qml.qrc
  • qml/theme/Theme.qml
  • tools/e2e/lib.sh
  • tools/e2e/cases/verse-sheet-no-tsk.sh
  • tools/e2e/cases/verse-sheet-notes.sh
  • tools/e2e/cases/verse-sheet.sh
  • tools/e2e/cases/verse-sheet-scroll.sh
  • tools/e2e/cases/note-sheet.sh
  • sword/src/filter/notes.rs
Review level: high (blast radius: trunk; core/src/bible*, tools/e2e). Commit: b78645ca0eba0437f67c5e132ad440f05bae5316. Range: origin/main...HEAD. Single-pass review, no high or medium findings; three low ones, all inline. Checked: VerseNote/labels zip alignment and the JSON shape (no other consumer of VerseSheet.notes), BottomSheet.refit() arithmetic against the new handle/header margins, the sheet fit with 5 refs and "Show all" within 60% of the 960 px e2e window (target/shots confirm it stays on screen), the new verse-sheet-notes e2e case (ordinal 6 = Genesis 1:3, log-line parsing), the existing verse-sheet*/verse-copy e2e cases against the changed layout, po/ against the changed qsTr strings, SectionLabel/SystemColours/OutlineButton registration (qmldir, qml.qrc). Files reviewed: - core/src/bible.rs - core/src/bible/reader/verse.rs - core/src/bible/reader/sheet.rs - core/src/bible/reader.rs - core/src/bible/verse_sheet.rs - core/src/bible/note.rs - docs/10-design.md - po/de.po - po/lectern.aps.pot - qml/controls/BottomSheet.qml - qml/controls/NoteSwitch.qml - qml/controls/OutlineButton.qml - qml/controls/RefRows.qml - qml/controls/SectionLabel.qml - qml/controls/qmldir - qml/pages/NoteSheet.qml - qml/pages/VerseSheet.qml - qml/platform/SystemColours.qml - qml/qml.qrc - qml/theme/Theme.qml - tools/e2e/lib.sh - tools/e2e/cases/verse-sheet-no-tsk.sh - tools/e2e/cases/verse-sheet-notes.sh - tools/e2e/cases/verse-sheet.sh - tools/e2e/cases/verse-sheet-scroll.sh - tools/e2e/cases/note-sheet.sh - sword/src/filter/notes.rs
po/de.po
@ -320,3 +323,2 @@
msgid "Notes: %1"
msgstr "Anmerkungen: %1"
msgid "NOTES"
claude-review commented 2026-11-03 10:20:44 +00:00
Collaborator

low: the three strings that were translated ("Notes: %1", "Notes: none in this verse", "Needs the TSK module") became NOTES, "None in this verse" and the TSK sentence, plus CROSS-REFERENCES, all with an empty msgstr. A German-locale user sees English in the verse sheet where it was German before; the sibling section labels (READING -> LESEN) are translated. Fill in the msgstr.

low: the three strings that were translated ("Notes: %1", "Notes: none in this verse", "Needs the TSK module") became NOTES, "None in this verse" and the TSK sentence, plus CROSS-REFERENCES, all with an empty msgstr. A German-locale user sees English in the verse sheet where it was German before; the sibling section labels (READING -> LESEN) are translated. Fill in the msgstr.
qml/controls/RefRows.qml
@ -38,2 +47,3 @@
Label {
Layout.fillWidth: true
Layout.alignment: Qt.AlignTop
Layout.preferredWidth: 104
claude-review commented 2026-11-03 10:20:44 +00:00
Collaborator

low: the reference label is fixed at 104 px with wrapMode Text.Wrap, which only breaks at spaces. A single word wider than 104 px (German "Thessalonicher" is about 102 px at 14 px medium; wider with a larger system font) is not clipped or elided, so it paints over the verse text beside it. Consider WrapAnywhere or a minimum width tied to the font.

low: the reference label is fixed at 104 px with wrapMode Text.Wrap, which only breaks at spaces. A single word wider than 104 px (German "Thessalonicher" is about 102 px at 14 px medium; wider with a larger system font) is not clipped or elided, so it paints over the verse text beside it. Consider WrapAnywhere or a minimum width tied to the font.
qml/pages/VerseSheet.qml Outdated
@ -164,8 +205,10 @@ BottomSheet {
objectName: "copyVerse"
width: Theme.touch
height: Theme.touch
background: null
claude-review commented 2026-11-03 10:20:44 +00:00
Collaborator

low: background: null on the copy ToolButton removes all press/focus feedback (Suru drew it in the background), so a tap or keyboard focus on the title's only action shows nothing. A transparent Rectangle with a visualFocus border, as OutlineButton does, would keep the look.

low: `background: null` on the copy ToolButton removes all press/focus feedback (Suru drew it in the background), so a tap or keyboard focus on the title's only action shows nothing. A transparent Rectangle with a visualFocus border, as OutlineButton does, would keep the look.
sheets: German for the new strings, long reference words break, copy shows its press (#148)
Some checks failed
CI / changes (pull_request) Successful in 9s
Details
CI / rust (pull_request) Has been cancelled
Details
CI / audit (pull_request) Has been cancelled
Details
CI / app (pull_request) Has been cancelled
Details
CI / e2e (pull_request) Has been cancelled
Details
CI / click (pull_request) Has been cancelled
Details
11f2f70141 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-bot commented 2026-11-03 10:21:54 +00:00
Author
Collaborator

Answers to review 394, all three fixed in 11f2f70:

  • po/de.po:324: fixed. The four new strings are translated (149 translated, 0 untranslated).
  • qml/controls/RefRows.qml:49: fixed. The reference label wraps with WrapAtWordBoundaryOrAnywhere, so a word wider than the column breaks instead of painting over the text.
  • qml/pages/VerseSheet.qml:208: fixed. The copy button has a round tint on press and on keyboard focus.

None of these touch trunk code, so no re-review round. verse-copy, verse-sheet-notes and verse-sheet pass after the fixes.

Answers to review 394, all three fixed in 11f2f70: - `po/de.po:324`: fixed. The four new strings are translated (149 translated, 0 untranslated). - `qml/controls/RefRows.qml:49`: fixed. The reference label wraps with `WrapAtWordBoundaryOrAnywhere`, so a word wider than the column breaks instead of painting over the text. - `qml/pages/VerseSheet.qml:208`: fixed. The copy button has a round tint on press and on keyboard focus. None of these touch trunk code, so no re-review round. `verse-copy`, `verse-sheet-notes` and `verse-sheet` pass after the fixes.
sheets: a reference's text is shown whole, never cut off (#148)
Some checks failed
CI / changes (pull_request) Successful in 7s
Details
CI / rust (pull_request) Has been cancelled
Details
CI / audit (pull_request) Has been cancelled
Details
CI / app (pull_request) Has been cancelled
Details
CI / e2e (pull_request) Has been cancelled
Details
CI / click (pull_request) Has been cancelled
Details
6bc9d78fe6 
The owner's phone QC: a reader may only want to read the verse, and not
every row has a way to see more. No line limit on a reference's text, and
a range shows all its verses, up to 40 (then an ellipsis says it goes on).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sheets: a reference to a range shows two verses, then an ellipsis (#148)
All checks were successful
CI / changes (pull_request) Successful in 7s
Details
CI / rust (pull_request) Successful in 6m39s
Details
CI / audit (pull_request) Successful in 14s
Details
CI / app (pull_request) Successful in 3m24s
Details
CI / e2e (pull_request) Successful in 14m19s
Details
CI / click (pull_request) Successful in 2m46s
Details
bc115d62ec 
The owner's phone QC.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review reviewed 2026-11-03 10:33:37 +00:00
claude-review left a comment

Review level: medium (trunk). Reviewed commit: bc115d62ec. Range: b78645ca0eba0437f67c5e132ad440f05bae5316..HEAD.

Files reviewed: core/src/bible/reader/sheet.rs, docs/10-design.md, po/de.po, qml/controls/RefRows.qml, qml/pages/VerseSheet.qml (callers/consumers checked by grep: RANGE_VERSES, maximumLineCount, range_text).

Findings: none at high or medium. No defects with a concrete failure scenario found. The ellipsis condition (last - first >= 2) matches the two-verse cap, the tests cover both cases (they skip without SWORD_TEST_PATH), and removing the line cap in RefRows is bounded because the core now sends at most two verses.

Review level: medium (trunk). Reviewed commit: bc115d62ec750b9f07226a1c89b77cf667d16a24. Range: b78645ca0eba0437f67c5e132ad440f05bae5316..HEAD. Files reviewed: core/src/bible/reader/sheet.rs, docs/10-design.md, po/de.po, qml/controls/RefRows.qml, qml/pages/VerseSheet.qml (callers/consumers checked by grep: RANGE_VERSES, maximumLineCount, range_text). Findings: none at high or medium. No defects with a concrete failure scenario found. The ellipsis condition (last - first >= 2) matches the two-verse cap, the tests cover both cases (they skip without SWORD_TEST_PATH), and removing the line cap in RefRows is bounded because the core now sends at most two verses.
claude-bot changed title from WIP: Verse and note sheets redesign (#148 step 2) to Verse and note sheets redesign (#148 step 2) 2026-11-03 10:57:47 +00:00
claude-review approved these changes 2026-11-03 10:57:58 +00:00
claude-review left a comment

Merge gate: approved at bc115d6.

  • Findings: review 394 (high) had three lows, all fixed in 11f2f70 and answered in comment 2908. Review 395 (medium, b78645c..bc115d6) had none. No high or medium findings are open.
  • Head: the PR head is bc115d6, the last commit reviewed. No commits came after review 395.
  • CI: all six statuses (changes, rust, audit, app, e2e, click) are green on bc115d6.
  • Proof: every gate listed in the PR body was run. None is an unrun elevated, maximum or AI-behavior gate. The dark theme has no shot, and the PR body says so; the owner's phone QC on bc115d62e covered it.
Merge gate: approved at bc115d6. - Findings: review 394 (high) had three lows, all fixed in 11f2f70 and answered in comment 2908. Review 395 (medium, b78645c..bc115d6) had none. No high or medium findings are open. - Head: the PR head is bc115d6, the last commit reviewed. No commits came after review 395. - CI: all six statuses (changes, rust, audit, app, e2e, click) are green on bc115d6. - Proof: every gate listed in the PR body was run. None is an unrun elevated, maximum or AI-behavior gate. The dark theme has no shot, and the PR body says so; the owner's phone QC on bc115d62e covered it.
schaefera deleted branch feat/redesign-verse-sheet 2026-11-03 11:07:14 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
claude-review
Labels
Clear labels   
priority
Could
 MoSCoW: nice to have, done when time allows

  
priority
Must
 MoSCoW: the release isn't viable without it

  
priority
Should
 MoSCoW: important, but the release works without it

  
priority
Won't
 MoSCoW: not this release (kept for later)

  
type
Bug

No labels
priority
Could
priority
Must
priority
Should
priority
Won't
type
Bug
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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!152
No description provided.