About redesign (#148 step 7) #161
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!161
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/redesign-about"
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
Settings' About row opens a dialog (board E10) with the icon, the version, the licence, a Source code row that hands the address to the system, and a Licences row.
Licences is a new page: each installed module as its conf states it, then what the app carries (Crimson Pro, Lucide, CLDR, ISO 639-3).
The row reads "Licences", shorter than the board's words, so it fits on one line. Close is an outlined button like the other dialogs, not the board's plain text.
Blast radius
Tier: trunk
Trunk paths touched:
qt/build.rs,qt/src/lib.rs,qt/src/module_licences.rs,tools/e2e/cases/about.shTesting tier: standard
Gate / rollback: no flag. Revert this merge; no schema or stored data changes.
Budgets: no measurable change at start-up. CI peak PSS 175330 kB normal, 178636 kB slow, of 185000 kB (#160: 173019/178272 kB). The conf read and the icon load only when their page or dialog opens.
Proof
835d5a1(run 334): changes, rust, audit, app, e2e and click all success. In e2e:PASS about (8s),PASS settings-pages (19s).cargo make: Build Done, no failures (run at the first commit; later commits touch only QML, PNGs and an e2e case).cargo make app-dockerfails its PSS check on this machine, and not because of this PR: branch 188792–196128 kB, untouched mainb9de36d186163–190846 kB. The gap to CI is unexplained; CI is the measure relied on here.aboutcase FAIL, and the restored tree gave PASS: installed modules dropped, licence returned empty, wrong source address, wrong icon path.Description=Bible <b>NT</b> & <i>tag</i>andDistributionLicense=<u>GPL</u>shows them literally under Xvfb. Throwaway; no committed test feeds it markup.IndexCache::installedover 100 copies of the KJV conf took 3.4–4.1 ms across five runs. Throwaway test, debug build on the desktop with a warm cache. Not measured on the phone or with a cold cache.b81d21b), Ambiance and SuruDark: dialog without elision; Licences with all 10 modules, scrolled to the last row; back to Settings. Source code opened the repo in Morph on an earlier build with the sameAboutDialog.qml, so confinement allows it.Review
Reviewer: fresh-context
code-reviewat high (413): 3 low findings; 1 fixed (conf text as plain text), 2 refuted with evidence (browser opens on the phone; conf read measured). Re-review at medium of 7da073f..835d5a1 (415): no findings. 0 deferred, 0 left open.Review-Confidence: 100/100, earlier gaps are covered by evidence; remaining risks are the unmeasured cold-cache read on the phone and no committed markup test, both stated above.
Confidence by part
module_licences.rsand the Licences page: high; e2e with mutation proof, and seen on the phone with real modules.BuildInfoText: high; e2e, and seen on the phone in both themes.LicenceRowplain text: medium; seen once, no regression test.The design canvas still needs First start and About added; this session has no Artifact tool.
🤖 Generated with Claude Code
Review level: high (blast radius: trunk, tier standard)
Commit:
7da073fefeRange: origin/main...HEAD (1 commit)
Files: docs/10-design.md, po/de.po, po/lectern.aps.pot, qml/icons/lectern*.png, qml/pages/AboutDialog.qml, BuildInfoPopup.qml, BuildInfoText.qml, LicenceRow.qml, LicencesPage.qml, SettingsPage.qml, qml/qml.qrc, qt/build.rs, qt/src/lib.rs, qt/src/module_licences.rs, tools/e2e/cases/about.sh
No high or medium findings. Findings outside the diff lines:
Inline: one low on LicenceRow.
@ -0,0 +30,4 @@wrapMode: Text.Wrapcolor: colours.secondaryfont.pixelSize: 13text: row.detaillow · title/detail come from module confs (Description, DistributionLicense) but both Labels keep the default textFormat (AutoText). A conf value that looks like markup (e.g. 'Bible ' or an entity) is rendered as rich text and the tag text vanishes;
may fetch a URL. BuildInfoText and PageDialog use Text.PlainText; set textFormat: Text.PlainText on these two Labels (lines 26 and 33 area).
Answers to review 413:
LicenceRow.qml, conf text as rich text: fixed inb81d21b; both Labels areText.PlainText. No automated test feeds it markup; theaboutcase passes.AboutDialog.qml:79,Qt.openUrlExternallyunder confinement: checked on the phone at the same QML (arm64 build of this tree, Suru). Tapping Source code opened the repo page in Morph. So no url_dispatcher group is needed.module_licences.rs:43, confs read on the GUI thread: left as is. It happens once per push of a page the owner rarely opens, and with a handful of modules it is small. Not measured with many modules.Review level: medium. Commit:
835d5a1. Range: 7da073f..835d5a1.Files: qml/icons/lectern.png, qml/icons/lectern@2x.png, qml/icons/lectern@3x.png, qml/pages/LicenceRow.qml, tools/e2e/cases/about.sh.
No defects found. Checked: the icon PNGs are 72x72 and 216x216 RGBA with transparent corners; LicenceRow's PlainText applies to both labels; and the about.sh assertion "KJV · GPL" matches the KJV conf's DistributionLicense=GPL as rendered by LicencesPage.qml:69.
Evidence for two answers to review 413, from throwaway runs that are not committed. The tree is unchanged at
835d5a1.module_licences.rs:43, confs read on the GUI thread): refuted by measurement.IndexCache::installedover 100 copies of the KJV conf took 3.4–4.1 ms across five runs, in a debug build on the desktop. No phone measurement; the owner's phone has 10 modules, so about a tenth of that work on a slower CPU. That is well under a frame, not a stutter.LicenceRow.qml, markup in conf text): the fix was seen to work. With a module whose conf hasDescription=Bible <b>NT</b> & <i>tag</i>andDistributionLicense=<u>GPL</u>, the page under Xvfb shows those strings literally, tags and entity included, with no formatting. It is a one-off run; no committed test feeds it markup.WIP: About redesign (#148 step 7)to About redesign (#148 step 7)Merge gate on
835d5a1: approved. The PR has no high or medium findings; review 413's three low findings are closed (finding 1 fixed inb81d21bwith PlainText labels, findings 2 and 3 refuted with phone and timing evidence in 414 and 416), and review 415 of 7da073f..835d5a1 found nothing. Head is835d5a1with no commits after the last review round, and all six CI jobs (changes, rust, audit, app, e2e, click) are success. The Proof lists no unrun elevated, maximum or AI-behavior gate for this standard-tier PR. The PR body states the remaining risks: an unmeasured cold-cache conf read on the phone, no committed markup test, and a final icon not yet seen on the phone (owner accepted).