Note markers blue, italic and easy to tap; dark sheets; a mockup rule in docs/09 #134
No reviewers
Labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
schaefera/utStudyBIble!134
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/marker-colour"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
Note markers are link blue and italic, as in the #13 mockup ("Letter markers"), not grey like the verse numbers. A tap within half a touch size of a marker opens its note, where a near miss used to open the verse's sheet. The verse and note sheets take the theme's window colour, so they are dark in the dark theme; before, they kept a light background under light text. docs/09 gains a "Mockups" rule: when a change needs a mockup, and a fresh check of the shots against the chosen artboard.
Blast radius
Tier: trunk
Trunk paths touched:
qml/pages/ReadingPane.qml,tools/e2e/cases/note-sheet.shTesting tier: standard (also: ship on sight:
docs/09-quality-gates.md)Gate / rollback: revert this merge. Markers go back to grey, taps to exact hits, sheets to the palette colour.
Proof
b6b7400(changes, rust, audit, app, e2e, click).cargo make app-dockerexit 0 on4abed19: mem-check peak 169709 kB of 176000 kB, so the budget isn't affected.note-sheetnow taps 10 px left of and below the marker. With the old ReadingPane it fails (no 'utsb: note ref 0 ' after 3 taps). With the fix,note-sheet,verse-sheet,verse-text-tap,running-textandtap-retrypass.fix/marker-colour@4abed1962: the markers are blue and italic, and a tap on a marker opens its note (comment 2625).utsb: dark true, window #111111), buildb9451b9fc: the markers are #8ab4f8, the verse and note sheets are dark with readable text, and a tap beside the Prov 6:17 marker opens its note (comment 2630).b6b7400b0: a near tap still opens the note.Mockup
Artboard: "Letter markers" in the #13 Design canvas (
Main.dc.html). Mockup check: review 355. Differences that stay, with their reasons:Review
Reviewer: fresh-context review at high (356), 0 findings. Mockup check (355), 4 lows, all reasoned deviations (above). After the owner's phone findings, a delta review at medium (357) found 1 low (a near tap also matched running-text verse numbers), fixed in
b6b7400. 0 left open.Review-Confidence: 95/100. The only deduction is the
b6b7400fix, which came after the delta review. It is checked by the tap cases and the phone, but no e2e covers a tap at a running-text verse's end.Confidence by part
🤖 Generated with Claude Code
Mockup check
Compared: artboard "Letter markers" (target/mockup/Main.dc.html, #13 Design canvas) with the light-theme shot target/mockup/shot-after-light.png (desktop, Xvfb, 540x960 at scale 1), and with qml/pages/ReadingPane.qml marked(), qml/platform/SystemColours.qml, core/src/bible/note.rs marker() and core/src/bible/block.rs running().
Matches (no difference):
italic; vertical-align: super; text-decoration:none; code italic +<sup>+ text-decoration:none. Size: mockup 12/18 = 0.67 of text;<sup>is 2/3. Same.Differences:
padding: 0 2pxaround each marker; shot shows the marker touching the previous character (e.g. "light,”a"). Reason holds: Qt rich text has no inline padding (author's stated reason). Listed under "Mockup" in the PR body.<sup><font size="-1">), mockup 11/18 = 0.61. Reason holds: the block.rs comment says RichText only has steps, and this keeps the number's top inside its line; the difference is small. Add to the "Mockup" list in the PR body.<sup>. They are styled muted and without underline, so the look is the same; the behaviour is not part of this PR's view.Findings of medium severity: none.
Gap (not a finding): no dark-theme shot, as the e2e harness cannot switch the theme; the dark pair #8ab4f8 on #121212 is checked by the stated contrast figure only.
Review level: high (trunk: qml/pages/ReadingPane.qml). Reviewed commit
4abed19623. Range: origin/main...HEAD.Files reviewed: qml/pages/ReadingPane.qml, qml/platform/SystemColours.qml, docs/09-quality-gates.md; also read for context: core/src/bible/note.rs, core/src/bible/block.rs, core/src/bible/reader.rs, tools/e2e/cases/note-sheet.sh, docs/04-features.md.
Findings: none. The two split/join passes cover the only two
<a href=producers (note:in note.rs,verse:in block.rs), so no link is left unstyled and none is styled twice. Both call sites (verse body and running-text editor) go throughmarked().logMarkersand the e2e case read link targets, not colours, so they are unaffected. linkColour is defined on SystemColours and depends ondark, so it follows theme changes live.Screenshots (desktop build under Xvfb, light theme, Gen 1:3–4 BSB, cropped and scaled 200%).
Before (main): the note marker is muted grey and upright, like the verse numbers.
After: the marker is link blue (#1557a0) and italic, as in the #13 mockup; verse numbers stay muted.
Review: high review 356 found nothing; the mockup check (355) lists four low differences, each with a reason. Those go under "Mockup" in the body.
On the phone (journal:
utsb: build 0.0.1 fix/marker-colour@4abed1962, Suru light, KJV Prov 6:16–17): the markers are blue and italic, verse numbers muted. Tapping the 6:17 marker opens "Note a — A proud…: Heb. Haughty eyes", so the restyled links still work.Dark theme not checked: the phone is on Ambiance and the theme is a system setting, left to the owner.
b9451b9fixes two things the owner found on the phone.BottomSheetdrewpalette.window, which stays light under Suru dark (#85). It now drawsSystemColours.windowColour.On the phone, build
fix/marker-colour@b9451b9fc, dark theme (journal:utsb: dark true, window #111111):A tap left of and below the 6:17 marker opened its note.
note-sheetnow taps 10 px left of and 10 px below the marker. Without the fix it fails (no 'utsb: note ref 0 ' after 3 taps). With it,note-sheet,verse-sheet,verse-text-tap,running-textandtap-retrypass.WIP: Note markers blue and italic as in the #13 mockup; a mockup rule in docs/09to WIP: Note markers blue, italic and easy to tap; dark sheets; a mockup rule in docs/09Level: medium (re-review), trunk. Reviewed commit:
b9451b9fca. Range: 4abed19..b9451b9.Files reviewed: qml/pages/ReadingPane.qml, qml/controls/BottomSheet.qml, qml/platform/SystemColours.qml, qml/platform/ut/SystemColours.qml, tools/e2e/cases/note-sheet.sh, tools/e2e/lib.sh (e2e_tap_until).
Findings outside the diff: none.
Finding on a diff line (inline): low, ReadingPane.qml running-text tap handler.
Checked and fine: BottomSheet's SystemColours child is a non-visual zero-size Item under the popup's plain contentItem, outside the ColumnLayout, so it does not affect refit() (which reads headRow/column implicitHeight) or the e2e hooks. linkNear costs at most about 169 linkAt calls per tap. The note-sheet.sh offset tap (-10, +10) misses the raised letter exactly, so without linkNear it opens the verse sheet and the case fails; it proves the claim.
@ -567,3 +594,3 @@anchors.fill: parentonClicked: function (mouse) {const link = parent.linkAt(mouse.x, mouse.y)const link = pane.linkNear(parent, mouse.x, mouse.y)low: in running text every verse starts with a
verse:link (inline number). linkNear now returns that link for a tap on plain text within 24 px of it, so a tap on the last ~3 characters of verse N on the same line, or the first words of verse N+1, opens the sheet of whichever number link is nearest (often N+1 for a tap meant for N) instead of charAt/verseAt of the tapped verse. Scenario: tap the end of verse 5 when verse 6's number sits right after it: the sheet shows verse 6. Consider using linkNear only fornote:links (fall back to the exact linkAt forverse:), so the widened reach applies to the small markers only.Review 357, the low finding about running-text verse numbers: agreed and fixed in
b6b7400. The wider reach now finds onlynote:links. A verse number is still opened only by an exact hit, so a tap at a verse's end opens that verse's sheet again.note-sheet,verse-text-tapandrunning-textpass locally.WIP: Note markers blue, italic and easy to tap; dark sheets; a mockup rule in docs/09to Note markers blue, italic and easy to tap; dark sheets; a mockup rule in docs/09Merge gate (medium) on
b6b7400: approved.b6b7400fixes 357's finding. An exact hit still returns any link. The near search now accepts onlynote:links, so a tap near a running-text verse number falls through to the verse text tap.noteTappedstill handlesverse:for exact hits. I found no new defect.