Follow the system theme: remove the Night setting (#85, #41) #106

Merged

claude-bot merged 3 commits from fix/remove-night into main 2026-10-28 14:47:04 +00:00

Conversation 8 Commits 3 Files changed 23 +154 -179

claude-bot commented 2026-10-28 13:27:48 +00:00

Collaborator

• Copy link
• Copy Markdown

What

The Night mode setting is gone. The app now follows the system theme live: the window, the pages and the verse text.

• The reading text takes its colours from qml/platform/SystemColours.qml:
  • on Ubuntu Touch, from Suru's colours (platform/ut/);
  • elsewhere, from the palette.
• The red of the red letters is chosen by a dark flag, sent unsaved through the new Command::SetDark.
• An old saved text.night row is ignored.

Closes #85
Closes #41

Blast radius

Tier: trunk
Trunk paths touched: qml/Main.qml, qml/pages/ReadingPane.qml, qt/ glue (reading_model.rs), core/src/worker*, tools/e2e/
Testing tier: standard. It mostly removes code, but it is a UI change, so it needed the phone.
Gate / rollback: revert this merge. There is no migration and no flag. No memory or start-up effect is expected (mem-check on d97afa1: 156–161 MB).

Proof

• CI on head d97afa1: rust, audit, app, e2e and click all succeed.
• The e2e cases display-settings and display-settings-kill are updated for the removed setting.
• Phone, owner checklist steps 1–7 pass (comments 2205 and 2210). In the dark theme the journal shows dark true, window #111111, text #f7f7f7.
• The palette fallback (no Suru) was checked by hand with qmltestrunner (comment 2213).

Review

Reviewer: claude-review

• 294: high, no findings.
• 295: delta; the findings are fixed or answered in comment 2213.
• 296: approved.

Review-Confidence: 80

• −5 for fixing a delta finding.
• −15 because the fallback is checked only by hand.

🤖 Generated with Claude Code

## What The Night mode setting is gone. The app now follows the system theme live: the window, the pages and the verse text. - The reading text takes its colours from `qml/platform/SystemColours.qml`: - on Ubuntu Touch, from Suru's colours (`platform/ut/`); - elsewhere, from the palette. - The red of the red letters is chosen by a dark flag, sent unsaved through the new `Command::SetDark`. - An old saved `text.night` row is ignored. Closes #85 Closes #41 ## Blast radius **Tier:** trunk **Trunk paths touched:** `qml/Main.qml`, `qml/pages/ReadingPane.qml`, `qt/` glue (`reading_model.rs`), `core/src/worker*`, `tools/e2e/` **Testing tier:** standard. It mostly removes code, but it is a UI change, so it needed the phone. **Gate / rollback:** revert this merge. There is no migration and no flag. No memory or start-up effect is expected (mem-check on d97afa1: 156–161 MB). ## Proof - CI on head d97afa1: rust, audit, app, e2e and click all succeed. - The e2e cases display-settings and display-settings-kill are updated for the removed setting. - Phone, owner checklist steps 1–7 pass (comments 2205 and 2210). In the dark theme the journal shows `dark true, window #111111, text #f7f7f7`. - The palette fallback (no Suru) was checked by hand with qmltestrunner (comment 2213). ## Review **Reviewer:** claude-review - 294: high, no findings. - 295: delta; the findings are fixed or answered in comment 2213. - 296: approved. **Review-Confidence:** 80 - −5 for fixing a delta finding. - −15 because the fallback is checked only by hand. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-10-28 13:27:48 +00:00

Remove the Night mode setting; the app follows the system theme (#85, #41) ... bbf8ebd99b

Main.qml no longer forces a palette over the style, so the window, pages and
verse text take the system theme's colours live. The red of the red letters
is chosen by a dark flag the reading pane derives from the live palette and
sends unsaved (Command::SetDark); the text.night setting is gone and an old
saved row is ignored.

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

claude-review reviewed 2026-10-28 13:28:43 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Review level: high (trunk). Reviewed commit: bbf8ebd99b. Range: origin/main...HEAD.

Files read in context: qml/Main.qml, qml/pages/ReadingPane.qml, qml/pages/DisplaySettings.qml, qml/theme/Theme.qml, qt/src/reading_model.rs, core/src/worker.rs, core/src/worker/session.rs, core/src/worker/session/jump.rs, core/src/render.rs, core/src/settings.rs, core/src/bible/reader.rs, tools/e2e/lib.sh, tools/e2e/cases/display-settings.sh, tools/e2e/cases/display-settings-kill.sh, plus docs and po changes.

Findings: no defects with a concrete failure scenario.

Traced:

• SetDark is queued from Component.onCompleted. Session.render.dark is not reset by settings() or load, so the first load and slides use the right red. If the worker has no Bible yet, set_dark only stores the flag.
• Rerendered with key "dark" falls through to the _ => return arm in ReadingModel::setting. The rows are still applied, and no property is changed.
• The old text.night row in app.sqlite3 is simply ignored (the test covers apply()).
• The settings start order and the e2e hooks (display size) are consistent between QML and lib.sh.

Notes only (not defects):

• If the system theme is dark at start, the first paint uses the day red until SetDark's Rerendered arrives, so there can be one frame with the wrong red.
• No e2e case covers the dark path any more (menu-night was removed). The worker unit test covers the red switch, but the QML palette.window.hslLightness binding is only checked on the phone.

Review level: high (trunk). Reviewed commit: bbf8ebd99b3806e7d617c594d91db5b2db8042bd. Range: origin/main...HEAD. Files read in context: qml/Main.qml, qml/pages/ReadingPane.qml, qml/pages/DisplaySettings.qml, qml/theme/Theme.qml, qt/src/reading_model.rs, core/src/worker.rs, core/src/worker/session.rs, core/src/worker/session/jump.rs, core/src/render.rs, core/src/settings.rs, core/src/bible/reader.rs, tools/e2e/lib.sh, tools/e2e/cases/display-settings.sh, tools/e2e/cases/display-settings-kill.sh, plus docs and po changes. Findings: no defects with a concrete failure scenario. Traced: - SetDark is queued from Component.onCompleted. Session.render.dark is not reset by settings() or load, so the first load and slides use the right red. If the worker has no Bible yet, set_dark only stores the flag. - Rerendered with key "dark" falls through to the `_ => return` arm in ReadingModel::setting. The rows are still applied, and no property is changed. - The old `text.night` row in app.sqlite3 is simply ignored (the test covers apply()). - The settings start order and the e2e hooks (`display size`) are consistent between QML and lib.sh. Notes only (not defects): - If the system theme is dark at start, the first paint uses the day red until SetDark's Rerendered arrives, so there can be one frame with the wrong red. - No e2e case covers the dark path any more (menu-night was removed). The worker unit test covers the red switch, but the QML `palette.window.hslLightness` binding is only checked on the phone.

claude-bot commented 2026-10-28 14:04:35 +00:00

Author

Collaborator

• Copy link
• Copy Markdown

Phone check, docs/09 "On the phone". The build of this head (bbf8ebd) was installed on the phone at 15:02. Before step 1, hold the reading pane's footer: it should read fix/remove-night@bbf8ebd…. If it reads refactor/shared-controls@f5dd50a, the phone still has the old install; tell me and I'll reinstall.

1. With the system theme light (System Settings → Background & Appearance), open the app. The reading text, footer, ⋮ menu, Settings and Modules are all readable, with no washed-out or low-contrast text (#41).
2. Settings → Display: there is no Night mode switch. Text size, Sans/Serif, Verse numbers and Words of Jesus in red are still there.
3. Go to a chapter with red letters in a Bible that has them (KJV, e.g. Matthew 5). The red is readable on the light background.
4. With the app still open, switch the system theme to dark. The app turns dark by itself (no restart needed): reading text, footer, menu, Settings, Modules, the Go to grids and Back history are all readable.
5. In dark, the KJV red letters switch to a lighter red that is readable on the dark background.
6. Switch back to light with the app open. Everything returns, red included.
7. Change text size and Serif, close the app from the app spread and reopen it. The settings are kept.

Reply with ✅ or ⚠️ per step, as before.

Phone check, docs/09 "On the phone". The build of this head (bbf8ebd) was installed on the phone at 15:02. Before step 1, hold the reading pane's footer: it should read `fix/remove-night@bbf8ebd…`. If it reads `refactor/shared-controls@f5dd50a`, the phone still has the old install; tell me and I'll reinstall. 1. With the system theme **light** (System Settings → Background & Appearance), open the app. The reading text, footer, ⋮ menu, Settings and Modules are all readable, with no washed-out or low-contrast text (#41). 2. Settings → Display: there is **no Night mode switch**. Text size, Sans/Serif, Verse numbers and Words of Jesus in red are still there. 3. Go to a chapter with red letters in a Bible that has them (KJV, e.g. Matthew 5). The red is readable on the light background. 4. With the app still open, switch the system theme to **dark**. The app turns dark by itself (no restart needed): reading text, footer, menu, Settings, Modules, the Go to grids and Back history are all readable. 5. In dark, the KJV red letters switch to a lighter red that is readable on the dark background. 6. Switch back to **light** with the app open. Everything returns, red included. 7. Change text size and Serif, close the app from the app spread and reopen it. The settings are kept. Reply with ✅ or ⚠️ per step, as before.

schaefera commented 2026-10-28 14:07:45 +00:00

Owner

• Copy link
• Copy Markdown

Steps:

1. ✅
2. ✅
3. ✅
4. ⚠️ The normal text color is too similar to the dark mode background. I'll leave the phone on at this step for a screenshot so you can evaluate potential causes.

Steps: 1. ✅ 2. ✅ 3. ✅ 4. ⚠️ The normal text color is too similar to the dark mode background. I'll leave the phone on at this step for a screenshot so you can evaluate potential causes.

claude-bot commented 2026-10-28 14:11:03 +00:00

Author

Collaborator

• Copy link
• Copy Markdown

Step 4 cause: on the phone, Suru's controls take their colours from Suru's own attached object, not the Qt palette. The palette stays light when the system theme is dark, and the reading pane coloured its verse text from the palette. That left near-black text on Suru's dark background, and dark never became true, so the red stayed the day red. The footer and menus were already fine because they are Suru controls.

Fix e3dad59: qml/platform/SystemColours.qml takes the colours from Suru under qml/platform/ut/ and falls back to the palette on desktop. The reading pane takes its text colour and the dark flag from it. On the phone, dark theme now logs dark true, window #111111, text #f7f7f7: white verse text and the lighter red.

That build is installed and running now (the footer may read …bbf8ebd99-dirty, the same code as e3dad59). Please carry on from step 4: check it yourself in dark, then 5–7. Local gates are running on e3dad59. It gets pushed when they pass, then CI and a fresh review of the delta.

Step 4 cause: on the phone, Suru's controls take their colours from Suru's own attached object, not the Qt palette. The palette stays light when the system theme is dark, and the reading pane coloured its verse text from the palette. That left near-black text on Suru's dark background, and `dark` never became true, so the red stayed the day red. The footer and menus were already fine because they are Suru controls. Fix e3dad59: `qml/platform/SystemColours.qml` takes the colours from Suru under `qml/platform/ut/` and falls back to the palette on desktop. The reading pane takes its text colour and the dark flag from it. On the phone, dark theme now logs `dark true, window #111111, text #f7f7f7`: white verse text and the lighter red. That build is installed and running now (the footer may read `…bbf8ebd99-dirty`, the same code as e3dad59). Please carry on from step 4: check it yourself in dark, then 5–7. Local gates are running on e3dad59. It gets pushed when they pass, then CI and a fresh review of the delta.

schaefera referenced this pull request from a commit 2026-10-28 14:14:22 +00:00

Reading text and dark flag from Suru's colours on the phone (#85)

schaefera added 1 commit 2026-10-28 14:14:22 +00:00

Reading text and dark flag from Suru's colours on the phone (#85) ... e3dad59d8e

Suru's controls take their colours from its attached object, not the
palette, which stays light when the system theme is dark: the verse
text stayed near-black on Suru's dark background and the red letters
kept the day red (owner phone check, PR #106 step 4). SystemColours
reads Suru under platform/ut/ and falls back to the palette elsewhere.

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

schaefera commented 2026-10-28 14:15:39 +00:00

Owner

• Copy link
• Copy Markdown

4. ✅
5. ✅
6. ✅
7. ✅

4. ✅ 5. ✅ 6. ✅ 7. ✅

claude-review reviewed 2026-10-28 14:15:46 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Review of bbf8ebd..e3dad59 (trunk path, high depth). No blocker; two things to check.

1. qml/platform/SystemColours.qml:11-12 (medium, unverified): the fallback reads palette.text and palette.window on a plain Item. On Qt 5.15 (clickable.yaml) Item has no palette; it exists only on Controls 2 types (Control, Popup, Window) and became an Item property in Qt 6. If so, the fallback is a ReferenceError and text and background fall back to the default (black), so the desktop reading pane would be black on black in a dark theme, and dark would always be false. The old code used pane.palette (a Page), which was valid. The e2e cases will not catch it if the docker image has QtQuick.Controls.Suru, because the fallback is never taken there. Suggest making the root a Control (or taking the palette from a Control instance, e.g. Control { id: probe } and probe.palette.text), then checking by forcing the fallback (point the Loader source at a missing file) and looking at a shot. I could not run it here.

2. qml/platform/SystemColours.qml:15-17 (low): unlike Connectivity.qml, which loads lazily, this Loader is active at startup. Where Suru is missing (desktop) a "module QtQuick.Controls.Suru is not installed" warning is printed on every launch. Harmless, but it is new startup log noise. Also the fallback has no e2e coverage (see 1).

Checked and fine:

• Resolution of the Suru attached object from a Loader child of an invisible Item: the attached Suru walks the parent chain and falls back to the style's default theme. Your phone log (dark true, window #111111, text #f7f7f7) confirms it.
• dark and text both derive from the same loader.item bindings, so they change together on any live change. Suru.backgroundColor and foregroundColor are bindable properties.
• qmldir, qml.qrc and the import "../platform" are all registered. platform/ut/SystemColours.qml is imported only from the Loader, so Suru stays under platform/ut/ as CLAUDE.md requires.
• Other palette users: grep of qml/ shows only the diagnostic modulesItem.palette.windowText and menu.palette.window log in ReadingPane.qml:190-191 (not rendering). The remaining raw Text items in the reading pane (delegate, ornament, book, chapter) all moved to colours.text. Everything else is Label, which Suru colours itself. I found no Rectangle colours that read palette.
• The e2e log line: display-settings.sh parses utsb: menu icon color ..., not the utsb: dark ... line, so appending , text #... breaks nothing. window is now the Suru background rather than the palette; the e2e greps for it are unaffected.

Review of bbf8ebd..e3dad59 (trunk path, high depth). No blocker; two things to check. 1. qml/platform/SystemColours.qml:11-12 (medium, unverified): the fallback reads `palette.text` and `palette.window` on a plain `Item`. On Qt 5.15 (clickable.yaml) `Item` has no `palette`; it exists only on Controls 2 types (Control, Popup, Window) and became an Item property in Qt 6. If so, the fallback is a ReferenceError and `text` and `background` fall back to the default (black), so the desktop reading pane would be black on black in a dark theme, and `dark` would always be false. The old code used `pane.palette` (a Page), which was valid. The e2e cases will not catch it if the docker image has QtQuick.Controls.Suru, because the fallback is never taken there. Suggest making the root a `Control` (or taking the palette from a Control instance, e.g. `Control { id: probe }` and `probe.palette.text`), then checking by forcing the fallback (point the Loader source at a missing file) and looking at a shot. I could not run it here. 2. qml/platform/SystemColours.qml:15-17 (low): unlike Connectivity.qml, which loads lazily, this Loader is active at startup. Where Suru is missing (desktop) a "module QtQuick.Controls.Suru is not installed" warning is printed on every launch. Harmless, but it is new startup log noise. Also the fallback has no e2e coverage (see 1). Checked and fine: - Resolution of the Suru attached object from a Loader child of an invisible Item: the attached Suru walks the parent chain and falls back to the style's default theme. Your phone log (`dark true, window #111111, text #f7f7f7`) confirms it. - `dark` and `text` both derive from the same `loader.item` bindings, so they change together on any live change. Suru.backgroundColor and foregroundColor are bindable properties. - qmldir, qml.qrc and the `import "../platform"` are all registered. `platform/ut/SystemColours.qml` is imported only from the Loader, so Suru stays under `platform/ut/` as CLAUDE.md requires. - Other palette users: grep of qml/ shows only the diagnostic `modulesItem.palette.windowText` and `menu.palette.window` log in ReadingPane.qml:190-191 (not rendering). The remaining raw `Text` items in the reading pane (delegate, ornament, book, chapter) all moved to `colours.text`. Everything else is `Label`, which Suru colours itself. I found no `Rectangle` colours that read palette. - The e2e log line: display-settings.sh parses `utsb: menu icon color ...`, not the `utsb: dark ...` line, so appending `, text #...` breaks nothing. `window` is now the Suru background rather than the palette; the e2e greps for it are unaffected.

schaefera added 1 commit 2026-10-28 14:20:47 +00:00

SystemColours is a Control: Qt 5's Item has no palette (review 295) ... d97afa122b

On a plain Item the palette fallback threw a ReferenceError (checked
with qmltestrunner in the CI image, Qt 5.15.13), so where Suru is
missing the reading text would have been black. Control has a FINAL
background, so the colours are textColour and windowColour.

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

claude-bot commented 2026-10-28 14:20:57 +00:00

Author

Collaborator

• Copy link
• Copy Markdown

Review 295:

• Medium, Item has no palette on Qt 5: confirmed and fixed in d97afa1. I checked it with qmltestrunner in the CI image (Qt 5.15.13). On an Item, palette.text throws ReferenceError: palette is not defined; on a Control it gives #353637 #ffffff. SystemColours is now a Control. Control has a FINAL background, so the colours are renamed textColour and windowColour. I loaded the real file both ways in the CI image. Without ut/ (the fallback) it gives text #353637 on #ffffff; with Suru, #111111 on #ffffff. On the phone the rebuilt app logs dark false, window #ffffff, text #111111 (light theme). The Suru path is the one the owner's steps 4–7 passed, and this change does not touch it.
• Low, eager Loader warns where Suru is missing: left as is. That is one log line at start-up, and the reading pane needs the colours from its first frame, so a lazy load would not help.

Local on d97afa1: cargo make app-docker passed (PSS 161277 kB normal and 156389 kB slow; budget 165000 kB). display-settings, display-settings-kill and red-letters e2e: 3 passed.

Review 295: - **Medium, `Item` has no `palette` on Qt 5: confirmed and fixed in d97afa1.** I checked it with `qmltestrunner` in the CI image (Qt 5.15.13). On an `Item`, `palette.text` throws `ReferenceError: palette is not defined`; on a `Control` it gives `#353637 #ffffff`. `SystemColours` is now a `Control`. `Control` has a FINAL `background`, so the colours are renamed `textColour` and `windowColour`. I loaded the real file both ways in the CI image. Without `ut/` (the fallback) it gives text `#353637` on `#ffffff`; with Suru, `#111111` on `#ffffff`. On the phone the rebuilt app logs `dark false, window #ffffff, text #111111` (light theme). The Suru path is the one the owner's steps 4–7 passed, and this change does not touch it. - **Low, eager Loader warns where Suru is missing:** left as is. That is one log line at start-up, and the reading pane needs the colours from its first frame, so a lazy load would not help. Local on d97afa1: `cargo make app-docker` passed (PSS 161277 kB normal and 156389 kB slow; budget 165000 kB). `display-settings`, `display-settings-kill` and `red-letters` e2e: 3 passed.

claude-bot referenced this pull request 2026-10-28 14:21:13 +00:00

CI: stop main-branch runs from holding up PR runs #107

claude-review approved these changes 2026-10-28 14:46:40 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Approved at d97afa1.

• Review 294 (high, bbf8ebd): no findings.
• Review 295 (delta, e3dad59): the medium is fixed in d97afa1, as explained in comment 2213. The low was answered there.
• CI is green on all five jobs.
• The owner passed phone steps 1–7 on the Suru path.

Fresh audit: Review-Confidence 80.

• −5 for fixing a delta finding.
• −15 because the palette fallback has only a manual qmltestrunner check.

Optional follow-up: an automated test for the fallback and the dark theme.

Approved at d97afa1. - Review 294 (high, bbf8ebd): no findings. - Review 295 (delta, e3dad59): the medium is fixed in d97afa1, as explained in comment 2213. The low was answered there. - CI is green on all five jobs. - The owner passed phone steps 1–7 on the Suru path. Fresh audit: Review-Confidence 80. - −5 for fixing a delta finding. - −15 because the palette fallback has only a manual qmltestrunner check. Optional follow-up: an automated test for the fallback and the dark theme.

claude-bot changed title from WIP: Remove Night mode; follow the system theme (#85, #41) to Follow the system theme: remove the Night setting (#85, #41) 2026-10-28 14:46:44 +00:00

claude-bot merged commit 3f9511155c into main 2026-10-28 14:47:04 +00:00

claude-bot referenced this pull request from a commit 2026-10-28 14:47:05 +00:00

Merge pull request 'Follow the system theme: remove the Night setting (#85, #41)' (#106) from fix/remove-night into main

schaefera deleted branch fix/remove-night 2026-10-28 14:53:35 +00:00

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

No description provided.