Skip to content

feat(theme): persist ThemeMode preference + reactive PyrycodeMobileTheme wiring (#86)#109

Merged
ilmoniemi merged 3 commits into
mainfrom
feature/86
May 14, 2026
Merged

feat(theme): persist ThemeMode preference + reactive PyrycodeMobileTheme wiring (#86)#109
ilmoniemi merged 3 commits into
mainfrom
feature/86

Conversation

@ilmoniemi
Copy link
Copy Markdown
Contributor

What

Adds the data path for the theme-mode preference, so the sibling picker ticket has plumbing to write to and the app honors a stored mode across launches.

  • New ThemeMode { SYSTEM, LIGHT, DARK } enum in de.pyryco.mobile.data.preferences
  • AppPreferences.themeMode: Flow<ThemeMode> and suspend fun setThemeMode(mode), backed by stringPreferencesKey("theme_mode") holding ThemeMode.name. Absent or unparseable → SYSTEM.
  • MainActivity.setContent collects the flow with collectAsStateWithLifecycle(initialValue = SYSTEM), resolves darkTheme via an exhaustive when (SYSTEM → isSystemInDarkTheme(), LIGHT → false, DARK → true), and passes it into the existing PyrycodeMobileTheme(darkTheme: Boolean) — signature unchanged so the 18 @Preview call sites stay untouched.
  • SettingsScreen gains a themeMode: ThemeMode = ThemeMode.SYSTEM parameter (default keeps the two @Preview composables compiling); the Theme row's supporting is now themeMode.label() returning "System default" / "Light" / "Dark". The row remains non-interactive per spec.
  • The SETTINGS NavHost entry collects the flow once more and passes the value down — DataStore deduplicates, and this avoids drilling state through PyryNavHost for one screen.

Issue

Closes #86

Testing

Test-first (RED → GREEN). Three new unit tests in AppPreferencesTest:

  • themeMode_defaultsToSystem — fresh DataStore returns SYSTEM
  • setThemeMode_roundTripsAllValues — loops ThemeMode.entries, writes + reads each
  • themeMode_unparseableStoredValue_fallsBackToSystem — writes "PURPLE" to the underlying key, expects SYSTEM

Local run status:

  • ./gradlew test — all unit tests pass
  • ./gradlew lint — clean (had to reorder SettingsScreen params: modifier before themeMode per ComposeParameterOrder)
  • ./gradlew assembleDebug — builds
  • ⚠️ ./gradlew connectedAndroidTest — not run (no emulator/device); spec explicitly says no instrumented coverage needed for this slice

Architecture compliance

  • Follows the spec at docs/specs/architecture/86-*.md: ThemeMode lives in data.preferences, Theme.kt signature stays darkTheme: Boolean, resolution sits inside the setContent @Composable scope, label mapping lives in SettingsScreen.kt (no stringResource — sibling picker reuses the same literal strings).
  • Stateless composable convention preserved — SettingsScreen receives themeMode as a parameter instead of injecting AppPreferences itself.
  • No new dependencies; no DI changes (AppPreferences is already a single { … }).

🤖 Generated with Claude Code

ilmoniemi and others added 2 commits May 14, 2026 08:30
Adds a ThemeMode { SYSTEM, LIGHT, DARK } enum backed by a string-typed
DataStore key on AppPreferences, with a Flow getter and suspend setter.
The composition root collects the flow and resolves darkTheme inside
@composable scope (preserving isSystemInDarkTheme() on SYSTEM), then
passes the boolean into the existing PyrycodeMobileTheme signature.

The Settings Theme row's hardcoded "System" subtitle now reads from the
same flow, rendering "System default" / "Light" / "Dark". The row stays
non-interactive in this slice; the picker dialog is the sibling ticket.

Closes #86

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi
Copy link
Copy Markdown
Contributor Author

Code Review: #86

Decision: PASS

Findings

No MUST FIX or SHOULD FIX findings.

  • [NIT] SettingsScreen.kt:52 — parameter order modifier before themeMode deviates from the spec's stated signature, but this is correct: the ComposeParameterOrder lint rule requires modifier to precede other optional params. Developer noted the reorder in the PR body. Spec text should be considered the loose contract; lint-validated convention wins.

Summary

Slice lands cleanly. Data path (ThemeMode enum + AppPreferences.themeMode flow + setThemeMode) is in place, the composition root resolves darkTheme inside setContent via an exhaustive when (preserving isSystemInDarkTheme() for SYSTEM), and PyrycodeMobileTheme's signature stays put so the 18 @Preview callers don't need touching. The Theme row subtitle now reads "System default" / "Light" / "Dark" from the flow — contract strings the sibling picker ticket will reuse.

Test coverage hits all three AC items (default → SYSTEM, round-trip every enum value, unparseable garbage falls back to SYSTEM). The unparseable test correctly bypasses the typed setter by writing the raw stringPreferencesKey("theme_mode").

Compose correctness: collectAsStateWithLifecycle(initialValue = ThemeMode.SYSTEM) at both consumer sites masks the one-frame DataStore-read gap; no flash-of-wrong-theme. Double-collection at root + Settings nav entry is the architect's call and is correct (DataStore deduplicates; alternative is boilerplate drilling for one screen).

Material 3 compliance: no new hardcoded colors, typography, or shapes. The diff is text-source-only on the Theme row established in #64 — no layout/decoration changes, so the Figma fidelity spec applies trivially.

Kotlin idioms: no !!, no GlobalScope / runBlocking in production, no android.* in data/. Cold-flow shape at the repo layer with hot conversion at consumers is the right call.

Architecture spec followed almost line-for-line; the one deviation (param order) is lint-driven and documented.

Adds per-ticket notes for #86; updates app-preferences and
settings-screen feature docs to cover the new themeMode flow, the
caller-computes-darkTheme pattern at MainActivity's composition root,
and the Settings Theme row's subtitle now reading from DataStore.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi ilmoniemi merged commit 92c2a9b into main May 14, 2026
1 check passed
@ilmoniemi ilmoniemi deleted the feature/86 branch May 14, 2026 05:42
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.

feat(theme): persist ThemeMode preference + reactive PyrycodeMobileTheme wiring

1 participant