Repository navigation
Split CampfireSettings into focused settings interfaces - #1189
Merged
Merged
Conversation
Generated by 🚫 Danger Kotlin against 14c8458 |
Collaborator
This was referenced Oct 2, 2026
Merged
r0adkll
added this pull request to stack #1207
October 6, 2026 14:23
CampfireSettings mixed device identity, privacy choices, app state, layout, connection and library-view preferences in one interface. It is now split by concern into PrivacySettings, DeviceSettings, AppStateSettings, LayoutSettings, ConnectionSettings and LibraryViewSettings, with the theme id and mode joining ThemeSettings, so each can later move to the right storage layer (#607). The property-and-observe API, delegates, storage keys and defaults are unchanged.
r0adkll
force-pushed
the
refactor/split-campfire-settings
branch
from
October 6, 2026 14:24
a208757 to
14c8458
Compare
r0adkll
added a commit
that referenced
this pull request
Oct 6, 2026
<!-- pr-stack:start --> ## Stack - #1189 - [This PR] #1204 <!-- pr-stack:end --> Closes #607. Each account now keeps its own library sorting and layout. Everything else stays app-wide, including the theme, and all settings stay on the existing preference-backed store. ## What changed - **`UserPrefixedSettings`** is an `ObservableSettings` that keeps one account's values in the app's existing preferences, under `user:<userId>:` keys. - Accounts share the app's storage, and its Android backup, without mixing with each other. - Its `keys`, `size` and `clear()` only see that account's settings. - The prefix stays short because desktop's `java.util.prefs` rejects keys longer than 80 characters. A test checks every prefixed key against that limit. - **`LibraryViewSettingsImpl` moves into the user graph**, on the account's settings (`@ForScope(UserScope::class) ObservableSettings`). Its observers run in the user graph's coroutine scope, so they end with it. - Signed out, there's no account, so the app-wide values are used. - The root UI takes the settings from the user graph it already receives. Every other consumer was already user-scoped. - **`DefaultUserSettingsStore`** hands each user graph its account's settings. - An account's settings start from the library view values the app kept for everyone, copied the first time they're opened. Accounts added later start from the same values. - Logging out clears that account's keys, through the new `UserSettingsStore` API. - **The app's store is now qualified too:** `@ForScope(AppScope::class) ObservableSettings` everywhere it's provided and injected, so each injection site says which store it reads. ## Verification - `./scripts/ktlint --check` passes. Desktop, Android (alpha debug) and iOS (`:app:ios`) compile, and the Metro graph checks pass. - `./gradlew jvmTest test` passes. The two native-FFmpeg audio engine modules were excluded locally, because the bundled FFmpeg native library doesn't load on this machine. New tests: - `UserPrefixedSettingsTest`: per-account keys, isolation from other accounts and the app, scoped `keys`/`size`/`clear()`, listeners; - `DefaultUserSettingsStoreTest`: seeding once, keeping an account's changes, accounts apart, signed out, clearing; - `UserPrefixedKeyLengthTest`: desktop's key length limit. - **On device** (emulator, local testbed server, fossDebug, two accounts): - Upgrading from #1189 kept account A's list layout sorted by duration and its series sort. A's prefixed copies were written, and the app-wide values stayed in place. - B started from the same values, then changed to grid sorted by size and series by name. Switching A ↔ B four times showed each account's own layout and sort immediately. - A cold start on B showed B's grid with no frame of A's. - Signing out of B removed only B's keys. Signing out of A left no per-account keys. Signing A back in started it again from the app-wide values. - No settings errors or missing bindings in logcat. ## Follow-ups - #1201: a second user on the same server URL replaces the first account, without going through logout, so the replaced account's keys stay behind. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
First step of #607.
CampfireSettingsmixed device identity, privacy choices, app state, window layout, the socket toggle and library view preferences in one interface. This splits it by concern so each group can later move to the right storage layer (global vs per-user, then DataStore).What changed
CampfireSettingsis split into:PrivacySettings: consent, crash/analytics reporting, keep signed in after reinstallDeviceSettings: device id, analytics id, current userAppStateSettings: last seen version, app update prompts, widget pinningLayoutSettings: navigation rail, supporting pane widthConnectionSettings: socket toggleLibraryViewSettings: sort/display state, marquee, time in book, download confirmationthemeId,themeModeandobserveTheme()move onto the existingThemeSettings.TestCampfireSettingsis split the same way intoTest*Settings.AnalyticsInitializerandPlayerWidgetComponentheld aCampfireSettingsthey never read; those are removed.What didn't change
The members were moved over unchanged: the
var+observeX()API,AppSettingsdelegates, storage keys and defaults are all the same. There's no user-facing change, so no changelog entry.Verification
./scripts/ktlint --checkpasses.:app:desktop:compileKotlinand:app:android:compileAlphaDebugKotlincompile, and the touched modules compile for the iOS simulator../gradlew jvmTest test: everything passes except the four ffmpeg tests ininfra/audioplayer/engine-tests, which fail locally because the native library doesn't load (UnsatisfiedLinkError: no jniavutil). This PR doesn't touch anything those tests use.🤖 Generated with Claude Code