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

Picker: square chapter and verse buttons, taller book buttons #35

Merged
claude-bot merged 1 commit from phase1/picker-height into main 2026-10-22 09:37:41 +00:00
Conversation 2 Commits 1 Files changed 1 +9 -1
claude-bot commented 2026-10-22 09:25:40 +00:00
Collaborator

What

From the phone QC of #11 (#31). In Go to, the chapter and verse buttons were wide but flat, so a thumb hit the row above or below.

  • Chapter and verse buttons (five to a row) are now square: height: Math.max(width, page.touch).
  • Book buttons (three to a row) are at least page.touch high.
  • page.touch is 48, which is 6 grid units. main.cpp sets QT_SCALE_FACTOR from GRID_UNIT_PX, so one grid unit is 8 logical pixels. This is the same scale as the page's existing margin 12 and gap 8. 48 dp is the common minimum touch target.

Closes #31.

Blast radius

Tier: leaf (qml/pages/PickerPage.qml only)
Testing tier: standard. Only layout changes: there's no logic, model, memory or start-up change.
Gate / rollback: none needed. Rollback is a revert.

Proof

  • Local app gates: cargo make app-docker exit 0 on 51f4a6d: qmllint, a -Werror build, clang-tidy, and mem-check in both profiles.
    • mem-check taps the picker by coordinates in a 540×960 window: book 4 at (95,145), chapter 1 and verse 1 at (60,72). Both still land with the new sizes.
    • Normal profile: picker jump and Back to 'BSB 268 0.348', the same after kill -9 with history, and peak PSS 141278 kB.
    • Slow profile: 'BSB 266 0.803' both ways, and peak PSS 140907 kB (budget 165000).
    • The typed field still jumps.
  • CI: rust, audit, app and click all green on 51f4a6d.
  • Budgets (docs/07): no change. Peak PSS is 141 MB, the same as main. There's no start-up work, and the button count is the same (only their height changes).
  • Longer scrolling: square cells make long chapters scroll more. The worst is Ps 119, with 176 verses in 36 rows of about 97 px in the 540 px window.
  • On the phone (Plinius): the owner tried the 51f4a6d build. The journal shows build 0.0.1 phase1/picker-height@51f4a6d0e, and jumps made through the picker (Ps 149 → Exod 1, Exod 2 → Ezek 13). The owner said: "Button size is overall much better, particularly the chapter and verse numbers."

Review

Reviewer: a fresh-context leaf review, review 225 (COMMENT), on 51f4a6d. It found no high or medium issues.

  • The one low: touch: 48 is a page constant, not part of the Theme singleton docs/07 asks for, which doesn't exist yet. This is deferred to #12, where night mode and fonts bring that singleton, along with margin and gap.
  • Bindings: it checked that there's no binding loop and that contentHeight grows with the cells.
  • Taps: it worked out by hand that mem-check's taps still land (book 4 in row 2, y 135–183; chapter and verse 1 in the first cell, y 52–149).
  • Body: it found the body's claims match the code.

Review-Confidence: not audited (leaf). The Review-Tier: leaf trailer goes in the merge commit message rather than an empty commit (agent-workflow#7), which saves a CI run.

🤖 Generated with Claude Code

## What From the phone QC of #11 (#31). In Go to, the chapter and verse buttons were wide but flat, so a thumb hit the row above or below. - **Chapter and verse buttons** (five to a row) are now square: `height: Math.max(width, page.touch)`. - **Book buttons** (three to a row) are at least `page.touch` high. - **`page.touch` is 48**, which is 6 grid units. `main.cpp` sets `QT_SCALE_FACTOR` from `GRID_UNIT_PX`, so one grid unit is 8 logical pixels. This is the same scale as the page's existing `margin` 12 and `gap` 8. 48 dp is the common minimum touch target. Closes #31. ## Blast radius **Tier:** leaf (`qml/pages/PickerPage.qml` only) **Testing tier:** standard. Only layout changes: there's no logic, model, memory or start-up change. **Gate / rollback:** none needed. Rollback is a revert. ## Proof - **Local app gates:** `cargo make app-docker` exit 0 on 51f4a6d: qmllint, a -Werror build, clang-tidy, and mem-check in both profiles. - mem-check taps the picker by coordinates in a 540×960 window: book 4 at (95,145), chapter 1 and verse 1 at (60,72). Both still land with the new sizes. - Normal profile: picker jump and Back to 'BSB 268 0.348', the same after kill -9 with history, and peak PSS 141278 kB. - Slow profile: 'BSB 266 0.803' both ways, and peak PSS 140907 kB (budget 165000). - The typed field still jumps. - **CI:** rust, audit, app and click all green on 51f4a6d. - **Budgets (docs/07):** no change. Peak PSS is 141 MB, the same as main. There's no start-up work, and the button count is the same (only their height changes). - **Longer scrolling:** square cells make long chapters scroll more. The worst is Ps 119, with 176 verses in 36 rows of about 97 px in the 540 px window. - **On the phone (Plinius):** the owner tried the 51f4a6d build. The journal shows `build 0.0.1 phase1/picker-height@51f4a6d0e`, and jumps made through the picker (Ps 149 → Exod 1, Exod 2 → Ezek 13). The owner said: "Button size is overall much better, particularly the chapter and verse numbers." ## Review **Reviewer:** a fresh-context leaf review, review 225 (COMMENT), on 51f4a6d. It found no high or medium issues. - **The one low:** `touch: 48` is a page constant, not part of the `Theme` singleton docs/07 asks for, which doesn't exist yet. This is deferred to #12, where night mode and fonts bring that singleton, along with `margin` and `gap`. - **Bindings:** it checked that there's no binding loop and that `contentHeight` grows with the cells. - **Taps:** it worked out by hand that mem-check's taps still land (book 4 in row 2, y 135–183; chapter and verse 1 in the first cell, y 52–149). - **Body:** it found the body's claims match the code. **Review-Confidence:** not audited (leaf). The `Review-Tier: leaf` trailer goes in the merge commit message rather than an empty commit (agent-workflow#7), which saves a CI run. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
qml: square chapter and verse buttons, books at least 48 high, so a thumb doesn't miss (#31)
All checks were successful
CI / rust (pull_request) Successful in 3m47s
Details
CI / audit (pull_request) Successful in 11s
Details
CI / app (pull_request) Successful in 3m8s
Details
CI / click (pull_request) Successful in 1m9s
Details
51f4a6d0e0 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review reviewed 2026-10-22 09:26:26 +00:00
claude-review left a comment

Fresh-context review (leaf, medium). Commit 51f4a6d0e0, range origin/main...51f4a6d.

Files reviewed:

  • qml/pages/PickerPage.qml
  • tools/mem-check/run.sh (tap coordinates, around lines 280-390)
  • qt/main.cpp (scaleFromGridUnit)
  • docs/07-platform-constraints.md

Findings: none at high or medium severity.

What I checked:

  • Heights. Math.max(width, page.touch) and Math.max(implicitHeight, page.touch) have no binding loop, because width is set from grids.width and does not depend on height. All cells in a Grid get the same height, so rows stay aligned. Column and Grid implicit heights follow the children, so contentHeight: grids.height + margin grows correctly. Hidden Grids and Labels in the Column take no space.
  • Grid units. scaleFromGridUnit sets QT_SCALE_FACTOR = GRID_UNIT_PX/8, so 48 logical px is 6 grid units, as the PR says. The chapter and verse buttons are at least 48 even on a narrow screen: a 320 dp screen gives about 53 px.
  • mem-check taps, 540x960 window, default style. Everything below assumes a 40 px header and a Button implicit height of 40, which I inferred from the Qt 5.15 default style and did not run.
    • Book 4 at (95,145): the Old Testament grid starts at about y=79. Row 2 is now y 135-183 (it was 127-167), column 0 is x 12-178, so the tap still lands on the 4th book (Numbers).
    • Chapter 1 and verse 1 at (60,72): the grid starts at y=52 because the typed field has height 0 and no margins at that step. Cell 1 is x 12-109, y 52-149, so the tap lands on it.
    • Header taps at (27,940), (300,940), (480,23) and (270,77) are unchanged. The typed field is at y 52-92 and is not touched by this diff. The filter still narrows the grids through books.names(..., typed.text).
  • Longer scrolling. Ps 119 at 36 rows is about 3.8k px in the test window and about 2.5k px on a 360 dp phone. This is acceptable and as the PR states.
  • PR claims. They match the code. The PR also says it is not yet tried on the phone and is still a draft, which is consistent with docs/09.

Low, no action needed:

  • qml/pages/PickerPage.qml:27 hardcodes touch: 48. docs/07 line 21 asks for sizes through one Theme singleton, but none exists yet. Move it there when one lands.
Fresh-context review (leaf, medium). Commit 51f4a6d0e00f23d5f5d4535726ad63bf42645172, range origin/main...51f4a6d. Files reviewed: - qml/pages/PickerPage.qml - tools/mem-check/run.sh (tap coordinates, around lines 280-390) - qt/main.cpp (scaleFromGridUnit) - docs/07-platform-constraints.md Findings: none at high or medium severity. What I checked: - **Heights.** `Math.max(width, page.touch)` and `Math.max(implicitHeight, page.touch)` have no binding loop, because `width` is set from `grids.width` and does not depend on height. All cells in a Grid get the same height, so rows stay aligned. Column and Grid implicit heights follow the children, so `contentHeight: grids.height + margin` grows correctly. Hidden Grids and Labels in the Column take no space. - **Grid units.** `scaleFromGridUnit` sets `QT_SCALE_FACTOR = GRID_UNIT_PX/8`, so 48 logical px is 6 grid units, as the PR says. The chapter and verse buttons are at least 48 even on a narrow screen: a 320 dp screen gives about 53 px. - **mem-check taps, 540x960 window, default style.** Everything below assumes a 40 px header and a Button implicit height of 40, which I inferred from the Qt 5.15 default style and did not run. - Book 4 at (95,145): the Old Testament grid starts at about y=79. Row 2 is now y 135-183 (it was 127-167), column 0 is x 12-178, so the tap still lands on the 4th book (Numbers). - Chapter 1 and verse 1 at (60,72): the grid starts at y=52 because the typed field has height 0 and no margins at that step. Cell 1 is x 12-109, y 52-149, so the tap lands on it. - Header taps at (27,940), (300,940), (480,23) and (270,77) are unchanged. The typed field is at y 52-92 and is not touched by this diff. The filter still narrows the grids through `books.names(..., typed.text)`. - **Longer scrolling.** Ps 119 at 36 rows is about 3.8k px in the test window and about 2.5k px on a 360 dp phone. This is acceptable and as the PR states. - **PR claims.** They match the code. The PR also says it is not yet tried on the phone and is still a draft, which is consistent with docs/09. Low, no action needed: - qml/pages/PickerPage.qml:27 hardcodes `touch: 48`. docs/07 line 21 asks for sizes through one Theme singleton, but none exists yet. Move it there when one lands.
claude-bot changed title from WIP: Picker: square chapter and verse buttons, taller book buttons to Picker: square chapter and verse buttons, taller book buttons 2026-10-22 09:36:38 +00:00
claude-review approved these changes 2026-10-22 09:37:21 +00:00
claude-review left a comment

Merge gate (leaf, standard) on 51f4a6d0e0.

Checked:

  • Diff is only qml/pages/PickerPage.qml: a touch constant and height bindings. It matches the PR body.
  • Review 225 (COMMENT) found no high or medium issues. Its one low (touch: 48 as a page constant instead of a Theme singleton) is deferred to #12.
  • Head equals the commit review 225 covered, so nothing after it is unreviewed.
  • CI on head: rust, audit, app and click are all success (commit status API).
  • Proof has no unrun elevated, maximum or AI-behavior gate. The local app gates passed, and the owner tried the build on the phone and said the button size is much better.
  • Leaf PR, so no audit is required.
Merge gate (leaf, standard) on 51f4a6d0e00f23d5f5d4535726ad63bf42645172. Checked: - Diff is only qml/pages/PickerPage.qml: a touch constant and height bindings. It matches the PR body. - Review 225 (COMMENT) found no high or medium issues. Its one low (touch: 48 as a page constant instead of a Theme singleton) is deferred to #12. - Head equals the commit review 225 covered, so nothing after it is unreviewed. - CI on head: rust, audit, app and click are all success (commit status API). - Proof has no unrun elevated, maximum or AI-behavior gate. The local app gates passed, and the owner tried the build on the phone and said the button size is much better. - Leaf PR, so no audit is required.
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
2 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!35
No description provided.