fix: consolidate theme state on next-themes useTheme() - #1415
Merged
K1NGD4VID merged 2 commits intoSep 3, 2026
Conversation
Remove duplicate theme management from useSettings.ts and settings-content.tsx, both of which independently read/wrote the "flowfi-theme" localStorage key and manually toggled the dark class on <html> without coordinating with each other or with next-themes' ThemeProvider. This caused toggling theme in one surface (e.g. the settings page) to not reflect in another (e.g. ModeToggle in the navbar) until a page reload. All three theme surfaces now use next-themes' useTheme() as the single source of truth: - layout.tsx's ThemeProvider (unchanged — already the provider) - settings-content.tsx now calls useTheme() from next-themes instead of managing its own localStorage read/write and class toggling - useSettings.ts no longer manages theme at all; consumers needing theme use useTheme() from next-themes instead Non-theme settings (displayCurrency, amountFormat, decimalPlaces) and their utilities (formatAmountWithPreference, getDecimalPlaces, etc.) are preserved untouched. Closes LabsCrypt#1262 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
Collaborator
|
resolve conflict on this issue @Nife-tanny |
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
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.
Overview
Resolves #1262 by consolidating theme state management on
next-themes'useTheme()as the single source of truth, eliminating two independent implementations that read/wrote the sameflowfi-themelocalStorage key without coordinating with each other or with theThemeProvider.What Each System Did Before
layout.tsx— UsedThemeProvider(wrappingnext-themes) withstorageKey="flowfi-theme". This was the correct, canonical implementation.useSettings.ts— Had its ownsetTheme()callback that independently:sharedSettings.themestatelocalStoragewith key"flowfi-theme"darkclass ondocument.documentElementgetStoredTheme()andapplyStoredTheme()helperssettings-content.tsx— Had its ownuseStatefor theme initialized fromlocalStorage.getItem("flowfi-theme"), and atoggleTheme()function that independently:localStoragewith key"flowfi-theme"darkclass ondocument.documentElementThe bug: None of the three called each other's setters, so toggling theme in one surface didn't reflect in another within the same tab — only a page reload or cross-tab
storageevent synced them.What Changed
useSettings.tsThemetype,themefromSettingsinterface andDEFAULT_SETTINGSthemefromSTORAGE_KEYSsetTheme()callback from the hook returngetStoredTheme()andapplyStoredTheme()helpersloadSettingsFromStorage()displayCurrency,amountFormat,decimalPlacessettings and all their setters,formatAmountWithPreference(),getDecimalPlaces(),getAmountFormat(),getDisplayCurrency()settings-content.tsxuseStatefor theme + manualtoggleTheme()withuseTheme()fromnext-themessetTheme()fromuseTheme(), ensuring they share state with the providerdisplayCurrency,amountFormat,decimalPlaceslocal state and their UIuseSettings.test.tsthemeorsetTheme)displayCurrencyinstead of themeformatAmountWithPreferencetests, all non-theme settings testsNo changes to:
layout.tsx— already correctly configured as theThemeProviderModeToggle.tsx— already correctly usinguseTheme()fromnext-themestheme-provider.tsx— already correctly wrappingNextThemesProviderSame-Tab Sync Verification
After this change, all three surfaces share the same reactive state via
useTheme():setTheme()fromuseTheme()→ next-themes updates its React context → all subscribers re-rendersetTheme()fromuseTheme()→ same path as aboveSince
useTheme()returns values from theThemeProvider's React context, any call tosetTheme()immediately updates all other components reading fromuseTheme()in the same render tree — no page reload needed.Pre-Existing Failures (Intentionally Untouched)
create-stream-recipient-prefill.test.tsx: 1 test fails with a 5-second timeout. Confirmed pre-existing (fails identically on the base branch before any changes).tsc --noEmiterrors are in pre-existing test files (batch-stream-wizard.test.tsx,streams.test.ts,useStreamingAmount.test.tsx, etc.) — none in the files modified by this PR.Invalid distDirRoot: ".next"— confirmed pre-existing (same error on base branch).Local Verification
npm run test(vitest)tsc --noEmitnext buildCloses #1262