schaefera/utStudyBIble

Watch

Fork 0

(#91) #102

Merged

claude-bot merged 1 commit from refactor/shared-controls into main

2026-10-28 13:15:42 +00:00

claude-bot commented

2026-10-28 12:36:46 +00:00

Collaborator

Closes #91.

What

A refactor with no visible change. PageBar moves to qml/controls/, and the new IconMenuItem, RowButton, TaskRow and NoteSwitch replace the Controls 2 tweaks that were repeated across the pages. CLAUDE.md's Code layout section gets the rule.

Deviations from the design:

TaskRow has the Starter page's layout. Modules keeps its look through detailBelow=false, rowSpacing, titleElide and spacing.
RowButton is flat by default. importGo stays a plain Button.
NoteSwitch uses the root's enabled and exposes switchItem for the e2e report.

Blast radius

Tier: trunk
Trunk paths touched: qml/pages/ReadingPane.qml
Testing tier: standard
Gate / rollback: none (no flag). Revert this merge.

Proof

CI on f5dd50a: rust, audit, app, e2e and click all passed.
Shots of 8 e2e cases (32 PNGs) before and after: 30 are pixel-equal. In the other 2 (the import dialog), only the test server's random port in the URL differs.
e2e passed locally: module-manager, import-url, modules-offline, starter-install, starter-fail, install-queue-fail, install-queue-cancel, display-settings, red-letters, red-letters-greyed, add-bible.
i18n refreshed; app-docker lint and build passed.
Resource budgets: no effect. CI mem-check peak PSS was 155,837 kB on the normal profile and 164,704 kB on the slow one, against main's 164,084 and 157,691 kB, so it is within the run-to-run noise (#96).
On the phone: build f5dd50a is installed on the Plinius and restored the saved chapter, with no QML errors or AppArmor denials in the journal. The owner ran the checklist (comment 2171) and everything passed (comment 2176). Their two layout remarks were already true on main; filed as #104 and #105.

Review

Reviewer: fresh-context code-review at high: 0 findings, 0 fixed, 0 declined, 0 deferred, 0 fixed after the delta review, 0 left open
Review-Confidence: 85

Confidence by part

New controls (TaskRow, NoteSwitch, RowButton, IconMenuItem): high. The shots are identical and the phone check passed.
Page rewiring (ModulesPage, StarterPage, DisplaySettings): high, for the same reasons.
ReadingPane.qml: high, a small change.
po/ files: high, generated by cargo make i18n.

🤖 Generated with Claude Code

Closes #91. ## What A refactor with no visible change. PageBar moves to qml/controls/, and the new IconMenuItem, RowButton, TaskRow and NoteSwitch replace the Controls 2 tweaks that were repeated across the pages. CLAUDE.md's Code layout section gets the rule. Deviations from the design: - TaskRow has the Starter page's layout. Modules keeps its look through detailBelow=false, rowSpacing, titleElide and spacing. - RowButton is flat by default. importGo stays a plain Button. - NoteSwitch uses the root's `enabled` and exposes switchItem for the e2e report. ## Blast radius **Tier:** trunk **Trunk paths touched:** qml/pages/ReadingPane.qml **Testing tier:** standard **Gate / rollback:** none (no flag). Revert this merge. ## Proof - CI on f5dd50a: rust, audit, app, e2e and click all passed. - Shots of 8 e2e cases (32 PNGs) before and after: 30 are pixel-equal. In the other 2 (the import dialog), only the test server's random port in the URL differs. - e2e passed locally: module-manager, import-url, modules-offline, starter-install, starter-fail, install-queue-fail, install-queue-cancel, display-settings, red-letters, red-letters-greyed, add-bible. - i18n refreshed; app-docker lint and build passed. - Resource budgets: no effect. CI mem-check peak PSS was 155,837 kB on the normal profile and 164,704 kB on the slow one, against main's 164,084 and 157,691 kB, so it is within the run-to-run noise (#96). - On the phone: build f5dd50a is installed on the Plinius and restored the saved chapter, with no QML errors or AppArmor denials in the journal. The owner ran the checklist (comment 2171) and everything passed (comment 2176). Their two layout remarks were already true on main; filed as #104 and #105. ## Review **Reviewer:** fresh-context `code-review` at high: 0 findings, 0 fixed, 0 declined, 0 deferred, 0 fixed after the delta review, 0 left open **Review-Confidence:** 85 ## Confidence by part - New controls (TaskRow, NoteSwitch, RowButton, IconMenuItem): high. The shots are identical and the phone check passed. - Page rewiring (ModulesPage, StarterPage, DisplaySettings): high, for the same reasons. - ReadingPane.qml: high, a small change. - po/ files: high, generated by `cargo make i18n`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-10-28 12:36:46 +00:00

Shared QML controls in qml/controls/ (#91 )

CI / rust (pull_request) Successful in 6m17s

Details

CI / audit (pull_request) Successful in 14s

Details

CI / app (pull_request) Successful in 3m29s

Details

CI / e2e (pull_request) Successful in 7m57s

Details

CI / click (pull_request) Successful in 2m38s

Details

f5dd50a470

PageBar moves there; IconMenuItem, RowButton, TaskRow and NoteSwitch
replace the repeated tweaks of Controls 2 in the pages. No visible change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-review reviewed

2026-10-28 12:37:28 +00:00

claude-review left a comment

Review level: high. Blast radius: trunk (qml/pages/ReadingPane.qml). Reviewed commit: f5dd50a470. Range: origin/main...HEAD.

Files reviewed: CLAUDE.md, po/de.po, po/utstudybible.schaefera.pot, qml/controls/{IconMenuItem,NoteSwitch,PageBar,RowButton,TaskRow}.qml, qml/controls/qmldir, qml/pages/{DisplaySettings,ModulesPage,ReadingPane,SettingsPage,StarterPage}.qml, qml/qml.qrc.

No defects found. Checked: ReadingPane's two IconMenuItems keep their objectNames, text, icon sources (relative to the declaring file) and handlers, and the icon size and touch height moved into IconMenuItem unchanged. The PageBar move is consistent across qmldir and qml.qrc, and no stale reference to pages/PageBar remains. The qrc lists every new control. The TaskRow aliases (cancelButton, retryButton) and NoteSwitch.switchItem keep the StarterPage and DisplaySettings e2e/report hooks working. The enabled state of NoteSwitch propagates to the inner Switch, and the note shows when it is disabled, as before. The ModulesPage and StarterPage TaskRow bindings preserve the old visibility, spacing, objectNames and detail placement. No findings outside the diff.

Review level: high. Blast radius: trunk (qml/pages/ReadingPane.qml). Reviewed commit: f5dd50a47081af3a7d600b0f2ec68ec306fd7eb6. Range: origin/main...HEAD. Files reviewed: CLAUDE.md, po/de.po, po/utstudybible.schaefera.pot, qml/controls/{IconMenuItem,NoteSwitch,PageBar,RowButton,TaskRow}.qml, qml/controls/qmldir, qml/pages/{DisplaySettings,ModulesPage,ReadingPane,SettingsPage,StarterPage}.qml, qml/qml.qrc. No defects found. Checked: ReadingPane's two IconMenuItems keep their objectNames, text, icon sources (relative to the declaring file) and handlers, and the icon size and touch height moved into IconMenuItem unchanged. The PageBar move is consistent across qmldir and qml.qrc, and no stale reference to pages/PageBar remains. The qrc lists every new control. The TaskRow aliases (cancelButton, retryButton) and NoteSwitch.switchItem keep the StarterPage and DisplaySettings e2e/report hooks working. The enabled state of NoteSwitch propagates to the inner Switch, and the note shows when it is disabled, as before. The ModulesPage and StarterPage TaskRow bindings preserve the old visibility, spacing, objectNames and detail placement. No findings outside the diff.

claude-bot commented

2026-10-28 13:09:15 +00:00

Author

Collaborator

On the phone: build refactor/shared-controls@f5dd50a47 (built 14:01, installed 14:02) is now on the Plinius and running. It restored Gen 1 (ASV). The journal shows no QML errors and no AppArmor denials.

Already checked by touch: the ⋮ menu (Modules and Settings with their icons, on Suru) and the Modules page (PageBar, Install buttons) both look as before.

On the phone, try: this PR should change nothing you can see, so look for anything that differs from yesterday's build.

Reading view ⋮ → Modules → ⋮ → Import / Refresh: icons and row heights as before.
Modules: Install a small module (e.g. AB), then watch the row's progress, Cancel, and Remove (Keep/Remove dialog).
Modules ⋮ → Import → paste a bad URL → Import: the import row shows the error with Retry / Cancel. Cancel removes it.
Reading view ⋮ → Settings → Display, in a Bible that doesn't mark the words of Jesus: "Words of Jesus in red" is greyed out with the reason below it. In KJV it's enabled, with no note.
Footer Bible button → Add (the starter page): install rows show the title, a bar, Retry/Dismiss on failure, and Cancel all.
Back from each page still works.

CI: green on f5dd50a. mem-check peak PSS was 155,837 kB (normal profile) and 164,704 kB (slow profile), against main's 164,084 / 157,691 kB, so it is within the run-to-run noise tracked in #96.

Shots: 30 of 32 are pixel-equal to main. The 2 import-dialog shots differ only in the test server's random port in the URL.

Unrelated, seen on Modules: Aleppo's detail line ("עברית · 2.0.1 · 1.0 MB") is laid out right to left and its numbers are reordered. That label is not touched here. I'll file it separately.

**On the phone:** build `refactor/shared-controls@f5dd50a47` (built 14:01, installed 14:02) is now on the Plinius and running. It restored Gen 1 (ASV). The journal shows no QML errors and no AppArmor denials. **Already checked by touch:** the ⋮ menu (Modules and Settings with their icons, on Suru) and the Modules page (PageBar, Install buttons) both look as before. **On the phone, try:** this PR should change nothing you can see, so look for anything that differs from yesterday's build. 1. Reading view ⋮ → **Modules** → ⋮ → **Import** / **Refresh**: icons and row heights as before. 2. Modules: **Install** a small module (e.g. AB), then watch the row's progress, **Cancel**, and **Remove** (Keep/Remove dialog). 3. Modules ⋮ → Import → paste a bad URL → **Import**: the import row shows the error with **Retry** / **Cancel**. Cancel removes it. 4. Reading view ⋮ → **Settings** → Display, in a Bible that doesn't mark the words of Jesus: "Words of Jesus in red" is greyed out with the reason below it. In KJV it's enabled, with no note. 5. Footer Bible button → **Add** (the starter page): install rows show the title, a bar, Retry/Dismiss on failure, and **Cancel all**. 6. Back from each page still works. **CI:** green on f5dd50a. mem-check peak PSS was 155,837 kB (normal profile) and 164,704 kB (slow profile), against main's 164,084 / 157,691 kB, so it is within the run-to-run noise tracked in #96. **Shots:** 30 of 32 are pixel-equal to main. The 2 import-dialog shots differ only in the test server's random port in the URL. **Unrelated, seen on Modules:** Aleppo's detail line ("עברית · 2.0.1 · 1.0 MB") is laid out right to left and its numbers are reordered. That label is not touched here. I'll file it separately.

claude-bot referenced this pull request

2026-10-28 13:09:32 +00:00

Modules: a right-to-left language name flips the whole detail line #103

schaefera commented

2026-10-28 13:13:52 +00:00

Owner

Remarks:
ℹ️ When the download of a module occurs, the progress bar and the actual numerical data amounts should be on the same row. Additionally, a little bit of filler space could be added. Put into a separate could issue.
⚠️ When downloading a module, the cancel button seems to shift alignment compared to the install button in a notable way. This may be caused by the addition of the new row of the download bar shifting percentage or spacing calculations in the layout. Unsure on my end if this is a new regression.

All other checklist items pass.

Remarks: ℹ️ When the download of a module occurs, the progress bar and the actual numerical data amounts should be on the same row. Additionally, a little bit of filler space could be added. Put into a separate could issue. ⚠️ When downloading a module, the cancel button seems to shift alignment compared to the install button in a notable way. This may be caused by the addition of the new row of the download bar shifting percentage or spacing calculations in the layout. Unsure on my end if this is a new regression. All other checklist items pass.

claude-bot referenced this pull request

2026-10-28 13:14:23 +00:00

Modules: a download's progress bar and its amounts on one row #104

claude-bot referenced this pull request

2026-10-28 13:14:23 +00:00

Modules: the row's action button jumps when Install turns into Cancel #105

claude-bot commented

2026-10-28 13:14:43 +00:00

Author

Collaborator

Thanks, phone check passed.

ℹ️ Progress bar and amounts on one row: filed as #104 (Could).
⚠️ Cancel shifting against Install: not a regression from this PR. The row's action button has exactly the same properties as on main (flat, touch height, dimmed when disabled); RowButton only moves them into one file. The jump comes from two things already on main. The button is as wide as its label ("Cancel" is narrower than "Install"), and the row grows by the progress lines, so the vertically centred button moves down. Filed as #105 (Could, raise it if you'd like it sooner) with a fixed-width, top-aligned fix.

🤖 Generated with Claude Code

Thanks, phone check passed. - ℹ️ Progress bar and amounts on one row: filed as #104 (Could). - ⚠️ Cancel shifting against Install: **not a regression from this PR.** The row's action button has exactly the same properties as on main (flat, touch height, dimmed when disabled); `RowButton` only moves them into one file. The jump comes from two things already on main. The button is as wide as its label ("Cancel" is narrower than "Install"), and the row grows by the progress lines, so the vertically centred button moves down. Filed as #105 (Could, raise it if you'd like it sooner) with a fixed-width, top-aligned fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-review approved these changes

2026-10-28 13:15:06 +00:00

claude-review left a comment

Gate approval on f5dd50a470.

Review round: one high-level review on this commit, zero findings, so no high or medium finding is open. No commits after it (head is the single commit f5dd50a over origin/main).
CI: commit status is success on head.
Proof: testing tier is standard. There is no elevated, maximum or AI-behavior gate to run. Phone try confirmed by the owner.
Audit: Review-Confidence 85. The deduction was for the report not showing caller tracing. The auditor traced the call sites itself and found no defect. Two minor behaviour notes (red-letters note label in a disabled NoteSwitch, Retry dimming on Starter) are not defects.
Not merged. Remaining before merge: take the PR out of draft and remove the WIP prefix (it is still draft and the API shows mergeable=false).

Gate approval on f5dd50a47081af3a7d600b0f2ec68ec306fd7eb6. - Review round: one high-level review on this commit, zero findings, so no high or medium finding is open. No commits after it (head is the single commit f5dd50a over origin/main). - CI: commit status is success on head. - Proof: testing tier is standard. There is no elevated, maximum or AI-behavior gate to run. Phone try confirmed by the owner. - Audit: Review-Confidence 85. The deduction was for the report not showing caller tracing. The auditor traced the call sites itself and found no defect. Two minor behaviour notes (red-letters note label in a disabled NoteSwitch, Retry dimming on Starter) are not defects. - Not merged. Remaining before merge: take the PR out of draft and remove the WIP prefix (it is still draft and the API shows mergeable=false).

claude-bot changed title from ~~WIP: Shared QML controls in qml/controls/ (#91)~~ to Shared QML controls in qml/controls/ (#91)

2026-10-28 13:15:31 +00:00

claude-bot merged commit b645c80cb4 into main

2026-10-28 13:15:42 +00:00

claude-bot referenced this pull request from a commit

2026-10-28 13:15:42 +00:00

Merge pull request 'Shared QML controls in qml/controls/ (#91)' (#102) from refactor/shared-controls into main