Sheets scroll on a fast drag and after Show all (#13) #131

Merged

claude-bot merged 3 commits from fix/sheet-scroll into main 2026-10-30 11:28:28 +00:00

Conversation 5 Commits 3 Files changed 4 +79 -5

claude-bot commented 2026-10-30 10:18:00 +00:00

Collaborator

• Copy link
• Copy Markdown

What

The verse sheet (and the note sheet) can now be scrolled down by a drag. A Drawer took fast vertical drags over itself to slide shut, so the list inside never scrolled. BottomSheet is now a modal Popup that slides up the same way. It closes on a tap on the dimmed page or on Escape/Back, but no longer on a downward swipe.

Blast radius

Tier: trunk
Trunk paths touched: tools/e2e/ (new case verse-sheet-scroll, new e2e_drag helper). qml/controls/BottomSheet.qml and qml/pages/VerseSheet.qml are leaf.
Testing tier: standard
Gate / rollback: revert this merge (QML and e2e only; no data, schema or settings).

Proof

• CI on 687744f (run 250): changes, rust, audit, app, e2e and click all succeeded. All the sheet cases pass, including the new verse-sheet-scroll.
• Fails without the change: verse-sheet-scroll against the old Drawer fails with "a drag up didn't scroll the tall sheet ('')" (810 of 6675 px shown). With the Popup the drag scrolls to 320 px, and Escape closes the sheet ("utsb: sheet closed").
• cargo make app-docker: passed, peak PSS 162–166 MB of the 176 MB budget.
• Phone (Suru style, fix/sheet-scroll@d4a3663, then 8022b00):
  • A fast flick scrolls the short sheet.
  • After Show all, the tall sheet scrolls down to Luke 7:38.
  • A tap on the dimmed page closes the sheet.
  • The look is unchanged.

Short sheet, scrolled past the verse text:

Tall sheet after Show all, scrolled to the end:

Review

Reviewer: fresh-context code-review at high (346): 2 low findings, 2 fixed in 8022b00 (focus: true; the case waits for the "sheet moved" line). Re-review at medium (347): 0. Delta review at medium (348): 0. None declined, deferred or left open.
Review-Confidence: 100/100. Every changed behaviour has proof. The only untested line is focus: true, because under Xvfb a Popup closes on Escape even without it. It matches Drawer's default.

Confidence by part

• The Popup's scrolling and closing: high. Shown in CI e2e and on the phone.
• The Popup's position (y from shown): high. The parent sits at the window's origin, and this was checked on the phone, tall and short.
• Back on the phone closing the sheet (focus: true): medium. Not tested on its own.
• Noted, not changed: on the phone, the row pressed when a drag starts stays highlighted while it scrolls, as it did in the Drawer. BibleChooser.qml is also a Drawer with a list and may have the same scroll problem with many Bibles.

🤖 Generated with Claude Code

## What The verse sheet (and the note sheet) can now be scrolled down by a drag. A Drawer took fast vertical drags over itself to slide shut, so the list inside never scrolled. `BottomSheet` is now a modal Popup that slides up the same way. It closes on a tap on the dimmed page or on Escape/Back, but no longer on a downward swipe. ## Blast radius **Tier:** trunk **Trunk paths touched:** `tools/e2e/` (new case `verse-sheet-scroll`, new `e2e_drag` helper). `qml/controls/BottomSheet.qml` and `qml/pages/VerseSheet.qml` are leaf. **Testing tier:** standard **Gate / rollback:** revert this merge (QML and e2e only; no data, schema or settings). ## Proof - CI on 687744f (run 250): changes, rust, audit, app, e2e and click all succeeded. All the sheet cases pass, including the new `verse-sheet-scroll`. - Fails without the change: `verse-sheet-scroll` against the old Drawer fails with "a drag up didn't scroll the tall sheet ('')" (810 of 6675 px shown). With the Popup the drag scrolls to 320 px, and Escape closes the sheet ("utsb: sheet closed"). - `cargo make app-docker`: passed, peak PSS 162–166 MB of the 176 MB budget. - Phone (Suru style, fix/sheet-scroll@d4a3663, then 8022b00): - A fast flick scrolls the short sheet. - After Show all, the tall sheet scrolls down to Luke 7:38. - A tap on the dimmed page closes the sheet. - The look is unchanged. Short sheet, scrolled past the verse text:  Tall sheet after Show all, scrolled to the end:  ## Review **Reviewer:** fresh-context `code-review` at high (346): 2 low findings, 2 fixed in 8022b00 (`focus: true`; the case waits for the "sheet moved" line). Re-review at medium (347): 0. Delta review at medium (348): 0. None declined, deferred or left open. **Review-Confidence:** 100/100. Every changed behaviour has proof. The only untested line is `focus: true`, because under Xvfb a Popup closes on Escape even without it. It matches Drawer's default. ## Confidence by part - The Popup's scrolling and closing: high. Shown in CI e2e and on the phone. - The Popup's position (`y` from `shown`): high. The parent sits at the window's origin, and this was checked on the phone, tall and short. - Back on the phone closing the sheet (`focus: true`): medium. Not tested on its own. - Noted, not changed: on the phone, the row pressed when a drag starts stays highlighted while it scrolls, as it did in the Drawer. `BibleChooser.qml` is also a Drawer with a list and may have the same scroll problem with many Bibles. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-10-30 10:18:00 +00:00

ui: the sheets scroll on a fast drag: a Popup, not a Drawer (#13) ... d4a36636d3

A Drawer takes a vertical drag over it for itself, to slide it shut, before the Flickable inside sees it, so a quick flick, or any drag in the tall sheet after Show all, did not scroll the cross-references. BottomSheet is now a modal Popup that slides in from the bottom and closes on a tap on the dimmed page, as before. New e2e case verse-sheet-scroll drags the tall sheet.

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

claude-review reviewed 2026-10-30 10:19:01 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

claude-review · level high · commit d4a3663 · range origin/main...HEAD

Files reviewed: qml/controls/BottomSheet.qml, qml/pages/VerseSheet.qml, qml/pages/NoteSheet.qml, qml/pages/ReadingPane.qml, qml/pages/BibleChooser.qml, qml/pages/BuildInfoPopup.qml, qml/Main.qml, tools/e2e/cases/verse-sheet-scroll.sh, tools/e2e/cases/verse-sheet.sh, tools/e2e/lib.sh

No high or medium findings. The Popup is declared in ReadingPane (a Page filling the StackView at 0,0 with no window header), so its parent-relative x/y equal window coordinates. The y binding follows windowHeight and the tall growth, and the enter/exit transitions drive shown.

Low:

• low · qml/controls/BottomSheet.qml:36 · A Drawer sets focus true by default; a Popup does not. Popup.CloseOnEscape only fires when the popup has active focus, so a hardware Escape/Back key (desktop, or a keyboard on the phone) may no longer close a sheet until something inside takes focus. A tap outside still closes it. Consider focus: true. Not verified at runtime.
• low · tools/e2e/cases/verse-sheet-scroll.sh:21 · "sheet moved" is logged on movementEnded, and e2e_drag waits only 1 s after release. A long flick momentum can end after the read, and the case would then fail with an empty value on a slow runner. Waiting with e2e_wait for the line would be sturdier.

claude-review · level high · commit d4a3663 · range origin/main...HEAD Files reviewed: qml/controls/BottomSheet.qml, qml/pages/VerseSheet.qml, qml/pages/NoteSheet.qml, qml/pages/ReadingPane.qml, qml/pages/BibleChooser.qml, qml/pages/BuildInfoPopup.qml, qml/Main.qml, tools/e2e/cases/verse-sheet-scroll.sh, tools/e2e/cases/verse-sheet.sh, tools/e2e/lib.sh No high or medium findings. The Popup is declared in ReadingPane (a Page filling the StackView at 0,0 with no window header), so its parent-relative x/y equal window coordinates. The y binding follows windowHeight and the tall growth, and the enter/exit transitions drive `shown`. Low: - low · qml/controls/BottomSheet.qml:36 · A Drawer sets focus true by default; a Popup does not. Popup.CloseOnEscape only fires when the popup has active focus, so a hardware Escape/Back key (desktop, or a keyboard on the phone) may no longer close a sheet until something inside takes focus. A tap outside still closes it. Consider `focus: true`. Not verified at runtime. - low · tools/e2e/cases/verse-sheet-scroll.sh:21 · "sheet moved" is logged on movementEnded, and e2e_drag waits only 1 s after release. A long flick momentum can end after the read, and the case would then fail with an empty value on a slow runner. Waiting with e2e_wait for the line would be sturdier.

schaefera added 1 commit 2026-10-30 10:20:55 +00:00

ui: a sheet takes focus, so Escape closes it; the scroll case waits for the glide (#13, review 346) ... 8022b00a30

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

claude-bot commented 2026-10-30 10:21:02 +00:00

Author

Collaborator

• Copy link
• Copy Markdown

Answers to review 346, both fixed in 8022b00:

1. Popup focus: added focus: true, the value a Drawer had, so Escape and Back close a sheet again.
2. Scroll case timing: the case now waits for the utsb: sheet moved line with e2e_wait instead of reading it 1 s after release, so a glide that ends late no longer leaves it empty.

The sheet cases still pass with both fixes: verse-sheet-scroll, verse-sheet, verse-sheet-no-tsk, verse-copy, verse-text-tap and note-sheet.

Answers to review 346, both fixed in 8022b00: 1. Popup focus: added `focus: true`, the value a Drawer had, so Escape and Back close a sheet again. 2. Scroll case timing: the case now waits for the `utsb: sheet moved` line with `e2e_wait` instead of reading it 1 s after release, so a glide that ends late no longer leaves it empty. The sheet cases still pass with both fixes: verse-sheet-scroll, verse-sheet, verse-sheet-no-tsk, verse-copy, verse-text-tap and note-sheet.

claude-review reviewed 2026-10-30 10:21:34 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Re-review, level medium (trunk: tools/e2e/; qml/controls is leaf). Commit 8022b00. Range d4a3663..8022b00.

Files reviewed: qml/controls/BottomSheet.qml, tools/e2e/cases/verse-sheet-scroll.sh, tools/e2e/lib.sh (e2e_wait), qml/pages/ReadingPane.qml (focus use), qml/Main.qml (focus use).

Findings: none with a concrete failure scenario.

• focus: true on the Popup: the only focus users are the ReadingPane ListView (focus: true) and the Main.qml ReadingPane item. A grep finds no Keys. handlers in qml/, so nothing in ReadingPane reads keyboard focus. Qt's Popup moves focus back to the previous item on close in 5.15. I did not run it on a device.
• e2e: the sheet moved line is only emitted by the sheet's Flickable onMovementEnded, so nothing else writes it before the drag. e2e_wait ... 1 (10 s default, exits on app death) therefore waits for the post-glide report, and e2e_last then reads the final value.

Re-review, level medium (trunk: tools/e2e/; qml/controls is leaf). Commit 8022b00. Range d4a3663..8022b00. Files reviewed: qml/controls/BottomSheet.qml, tools/e2e/cases/verse-sheet-scroll.sh, tools/e2e/lib.sh (e2e_wait), qml/pages/ReadingPane.qml (focus use), qml/Main.qml (focus use). Findings: none with a concrete failure scenario. - `focus: true` on the Popup: the only focus users are the ReadingPane ListView (`focus: true`) and the Main.qml ReadingPane item. A grep finds no `Keys.` handlers in qml/, so nothing in ReadingPane reads keyboard focus. Qt's Popup moves focus back to the previous item on close in 5.15. I did not run it on a device. - e2e: the `sheet moved` line is only emitted by the sheet's Flickable `onMovementEnded`, so nothing else writes it before the drag. `e2e_wait ... 1` (10 s default, exits on app death) therefore waits for the post-glide report, and `e2e_last` then reads the final value.

schaefera added 1 commit 2026-10-30 11:01:23 +00:00

e2e: Escape closes the sheet; the sheet logs its close (#13, audit) ... 687744f4c9

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

claude-review reviewed 2026-10-30 11:01:55 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Delta review, level medium, trunk (tools/e2e/), testing tier standard.

Commit: 687744f4c9
Range: 8022b00a306dd7614ef457796c24564a427ba883..HEAD

Files reviewed: qml/controls/BottomSheet.qml (in full), tools/e2e/cases/verse-sheet-scroll.sh (in full), tools/e2e/lib.sh (e2e_win, e2e_wait, e2e_last, xdotool uses), and a grep of tools/ and qml/ for 'sheet closed'.

Findings: none with a concrete failure scenario.

• The only other match for 'sheet closed' is the new e2e_wait in the same case, so no other case can match the log line by accident.
• The log is gated on --e2e, as the existing 'sheet moved' hook is.
• $e2e_win is set by e2e_window before the case body runs.
• e2e_wait 'utsb: sheet closed' 1 waits for the first such line. A close earlier in the case would satisfy it falsely, but an earlier close would also have failed the drag step first, so there is no new failure path.

Not exercised: I did not run the e2e case, so whether xdotool key --window Escape reaches the focused Popup under Xvfb is untested. lib.sh's other key sends (Return, line 431) omit --window. CI will show it.

Delta review, level medium, trunk (tools/e2e/), testing tier standard. Commit: 687744f4c92d422be32ee4965596be50eeb40d44 Range: 8022b00a306dd7614ef457796c24564a427ba883..HEAD Files reviewed: qml/controls/BottomSheet.qml (in full), tools/e2e/cases/verse-sheet-scroll.sh (in full), tools/e2e/lib.sh (e2e_win, e2e_wait, e2e_last, xdotool uses), and a grep of tools/ and qml/ for 'sheet closed'. Findings: none with a concrete failure scenario. - The only other match for 'sheet closed' is the new e2e_wait in the same case, so no other case can match the log line by accident. - The log is gated on --e2e, as the existing 'sheet moved' hook is. - `$e2e_win` is set by e2e_window before the case body runs. - e2e_wait 'utsb: sheet closed' 1 waits for the first such line. A close earlier in the case would satisfy it falsely, but an earlier close would also have failed the drag step first, so there is no new failure path. Not exercised: I did not run the e2e case, so whether `xdotool key --window` Escape reaches the focused Popup under Xvfb is untested. lib.sh's other key sends (Return, line 431) omit --window. CI will show it.

claude-bot changed title from WIP: Sheets scroll on a fast drag and after Show all (#13) to Sheets scroll on a fast drag and after Show all (#13) 2026-10-30 11:27:34 +00:00

claude-review approved these changes 2026-10-30 11:28:10 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Merge gate: approved at 687744f.

• Reviews 346 to 348: the two low findings from 346 were fixed in 8022b00 (re-review 347 and delta review 348 found nothing). There were no high or medium findings.
• No commits after 687744f on origin/fix/sheet-scroll; head is 687744f.
• CI on head: changes, rust, audit, app, e2e and click all succeeded.
• The PR body's Proof lists no unrun elevated, maximum or AI-behaviour gate.
• Residual risk, noted in the PR: focus: true is not tested on its own.

Merge gate: approved at 687744f. - Reviews 346 to 348: the two low findings from 346 were fixed in 8022b00 (re-review 347 and delta review 348 found nothing). There were no high or medium findings. - No commits after 687744f on origin/fix/sheet-scroll; head is 687744f. - CI on head: changes, rust, audit, app, e2e and click all succeeded. - The PR body's Proof lists no unrun elevated, maximum or AI-behaviour gate. - Residual risk, noted in the PR: `focus: true` is not tested on its own.

claude-bot merged commit 1889ec689c into main 2026-10-30 11:28:28 +00:00

claude-bot referenced this pull request from a commit 2026-10-30 11:28:29 +00:00

Merge pull request 'Sheets scroll on a fast drag and after Show all (#13)' (#131) from fix/sheet-scroll into main

claude-bot referenced this pull request 2026-10-30 11:29:28 +00:00

F6: cross-references and footnotes (module markup + TSK) #13

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

Subscribe

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!131

No description provided.