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!89
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/offline-install-icons"
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?
Closes #84. Closes #87.
What
QT_QUICK_CONTROLS_STYLE=Suru, whose menu follows the system theme and ignores the palette, while the icons had been tinted from Theme.qml/platform/ut/), which needs the newconnectivityAppArmor policy group; withnetworkingalone the phone's journal showed the D-Bus calls DENIED and the app always looked offline.Blast radius
Tier: trunk
Trunk paths touched:
qt/glue (qt/src/module_manager.rs),qml/pages/ReadingPane.qml,core/src/tasks/exec.rs,utstudybible.apparmor(+connectivity),tools/e2e/Testing tier: standard (also: UI change, phone try-out done)
Gate / rollback: revert the merge; no schema or setting changes.
Proof
2f6b02f: rust, audit, app, e2e, click all green.tools/e2e/cases/modules-offline.sh;display-settings.shasserts icon colour = label colour in day and night (Default style).a_download_waits_for_the_connectionanda_network_failure_is_plain_without_the_raw_textfail with the logic disabled.8f48ec2found Install greyed while online and dim icons with Night off in system dark; both fixed in5e7d934. Round 2 at5e7d934: all six items passed (icons in both themes, online install, offline greying, back online, plain mid-download error).2f6b02fonly shortens the offline line, as asked.Budgets
5e7d934, Modules open with the connectivity plugin loaded) and 74,391 kB (8f48ec2); budget 165,000 kB.8f48ec2: 159,345 / 164,406 kB (normal / slow); the connectivity plugin is not loaded there.Review
Reviewer: fresh-context
code-review(claude-review) at8f48ec2(no findings), delta review 8f48ec2..2f6b02f (one low, refuted:display-settingspasses locally and in CI without the menu palette), gate APPROVED on2f6b02f.Review-Confidence: 85 (audit at
8f48ec2; its deduction was the untried Lomiri path, since tried on the phone).Follow-ups
🤖 Generated with Claude Code
Review level: high. Blast radius: trunk. Reviewed commit:
8f48ec2057. Range: origin/main...HEAD.Files read in context (hunks plus callers and docs): core/src/catalog.rs, core/src/catalog/action.rs, core/src/tasks/exec.rs, qt/src/module_manager.rs, qml/pages/ModulesPage.qml, qml/pages/ReadingPane.qml, qml/platform/Connectivity.qml, qml/platform/ut/Connectivity.qml, qml/platform/qmldir, qml/qml.qrc, tools/e2e/lib.sh, tools/e2e/cases/display-settings.sh, tools/e2e/cases/modules-offline.sh, docs/04-features.md. The po files were checked only for the removed "no connection: %1" string, which is now obsolete.
Findings: no defects with a concrete failure scenario.
Checked:
Notes, not defects:
requestslog has no 'BSB'. That passes only while the index fetch does not name BSB in a request path. If the case fails for that reason, narrow the grep to the module's zip path.Installed
8f48ec2on the phone (USB, 19:05). Phone checklist:(Memory is sampled over adb while you test; nothing to do for it.)
Thanks, both ⚠️ and the contrast note had real causes, found on the phone (journal over adb). Fixed in
5e7d934, installed on the phone now:com.lomiri.connectivity1 … GetAll … DENIED): it needs theconnectivitypolicy group, not justnetworking. Soonlinestayed false. Addedconnectivity(a common group; click review passes); docs 02/07 updated.QT_QUICK_CONTROLS_STYLE=Suru. Suru's menu follows the system theme and ignores our palette, so the menu was dark while my override tinted the icons with Theme's day text (dark grey). The override is gone: both Suru and the Default style tint a menu icon with its label's colour, so they match in every combination. The earlier "popup takes the system palette" theory was wrong.Please re-check:
Otherwise looks good.
Delta review 8f48ec2..2f6b02f
No high findings.
4988387). If a popup in Qt 5.15 does not inherit here, thewindow #121212check (night) fails in the e2e job, though the phone is unaffected. I could not run the e2e; the first CI run of display-settings answers it.#~entry, and no test or e2e case references the old text (modules-offline.sh only checks the button/online state).Gate approval at head
2f6b02f.8f48ec2had no high or medium findings, so none are open.5e7d934(adds theconnectivityAppArmor policy group, drops the ReadingPane palette overrides andicon.colorso the Suru style tints the icons, updates docs) and2f6b02f(shortens the offline note). I read both diffs and found no defects. Theconnectivitygroup is the fix forNetworkingStatus.onlinestaying false undernetworkingalone.successon2f6b02f. The branch contains origin/main and merges without conflicts.8f48ec2, phone try-out pending and review pending, and the title is still "WIP:". Update these and take the PR out of draft.WIP: Menu icon contrast and offline Install (#84, #87)to Menu icon contrast and offline Install (#84, #87)