Skip to content

fix(settings): persist the "System" theme choice - #97

Merged
Adron merged 2 commits into
mainfrom
fix/theme-system-persists
Sep 17, 2026
Merged

Adron merged 2 commits into
mainfrom
fix/theme-system-persists

Conversation

@Adron

@Adron Adron commented Sep 16, 2026

Copy link
Copy Markdown
Member

Summary

Closes #79. Settings → Appearance → Theme → System now sticks.

The picker mapped "system" to nil, and Swift's synthesized Encodable omits a nil field
(encodeIfPresent), so the PATCH /api/user/update body encoded to {}. The route applies
...(theme !== undefined && { theme }), so an absent key is a no-op: the stored theme was never
changed, the response returned the unchanged user, and syncFromUser snapped the picker back to
the old value.

The comment justifying the nil mapping — "Sending the string "system" is rejected or treated as
default (light) by the server" — was factually wrong. The route destructures theme and stores
whatever string arrives with no validation (app/api/user/update/route.ts:24,102,157), and the web
persists the same literal "system" (components/layout/ThemeProvider.tsx:11,17), so the value
round-trips with the web Settings page.

What's included

  • InterlinedList/Views/SettingsView.swift — send newValue verbatim from the theme picker;
    delete the nil mapping and the incorrect comment. The spurious-save guard is unchanged.
  • InterlinedListTests/APIClientTests/APIClientProfileTests.swift — two tests pinning the wire
    format: theme: "system" sends exactly {"theme":"system"}, and a nil theme omits the key
    entirely (the silent no-op this bug rode on, with the route semantics noted as the "why").
  • RootView.preferredScheme verified, not changed: it returns nil for any non-light/dark
    value, so "system" already means "follow the OS".

No new files, so no project.pbxproj change.

Testing

xcodebuild -scheme InterlinedList \
  -destination 'platform=iOS Simulator,id=302E002E-9A0C-4F79-B54A-E9739A3EE582' \
  -derivedDataPath <worktree DerivedData> \
  -parallel-testing-enabled NO \
  -skip-testing:InterlinedListTests/E2EReadOnlyTests test

** TEST SUCCEEDED ** — Executed 1149 tests, with 0 failures, including the two new ones.

The encoder behaviour was also confirmed directly against the updateUserSettings body struct:
the old "system" → nil mapping encodes to {}, the fix encodes to {"theme":"system"}.

Not verified here (needs a live account): the round-trip through GET /api/user and an app
relaunch, and parity with the web Settings page.

🤖 Generated with Claude Code

The theme picker mapped "system" to nil, and a synthesized Encodable
omits a nil field with encodeIfPresent, so the PATCH body encoded to
`{}`. `/api/user/update` applies `...(theme !== undefined && { theme })`,
so the key never arrived, the stored theme was never changed, and the
response returned the unchanged user — snapping the picker back.

Send "system" verbatim instead. The route performs no validation on
`theme` and the web persists the same literal string, so the value
round-trips; RootView.preferredScheme already treats any non-light/dark
value as "follow the OS", so no change was needed there.

The comment claiming the server rejects "system" was wrong and is gone.

Tests pin the wire format: theme "system" sends {"theme":"system"}, and
a nil theme omits the key entirely (the silent no-op that caused this).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017bss5MgZa7Jvj2m9zdaUd1
@Adron
Adron merged commit 1fc7974 into main Sep 17, 2026
1 check passed
@Adron
Adron deleted the fix/theme-system-persists branch September 17, 2026 08:55
Adron added a commit that referenced this pull request Sep 17, 2026
Conflict confined to project.pbxproj, but a plain union of both sides was wrong
here: this branch was stacked on #97 and so carries object definitions main had
already picked up at a different position in the file. Taking both produced six
duplicate UUID definitions — a corrupt project, not a merge.

Resolved by unioning the hunks and then dropping any object definition whose
UUID was already defined earlier in the file. Verified afterwards that
ThemePreference.swift and ThemePreferenceTests.swift each still have exactly one
PBXBuildFile, Sources entry, PBXFileReference and group entry, and that the
mute files main contributed are likewise intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017bss5MgZa7Jvj2m9zdaUd1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Theme "System" never persists — nil optional is omitted, so the route never clears the stored theme

1 participant