Skip to content

fix: improved theme logic #504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ignaciosantise wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

fix: improved theme logic #504

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
927be36
fix: improved theme logic
ignaciosantise Nov 14, 2025
b599e77
chore: code improvements
ignaciosantise Nov 14, 2025
a7f1584
chore: fixed tests
ignaciosantise Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions packages/appkit/src/__tests__/hooks/useAppKitTheme.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jest.mock('valtio', () => ({
jest.mock('@reown/appkit-core-react-native', () => ({
ThemeController: {
state: {
themeMode: undefined,
themeMode: 'light',
themeVariables: {}
},
setThemeMode: jest.fn(),
Expand All @@ -42,7 +42,7 @@ describe('useAppKitTheme', () => {
jest.clearAllMocks();
// Reset ThemeController state
ThemeController.state = {
themeMode: undefined,
themeMode: 'light',
themeVariables: {}
};
});
Expand All @@ -61,7 +61,7 @@ describe('useAppKitTheme', () => {
it('should return initial theme state', () => {
const { result } = renderHook(() => useAppKitTheme(), { wrapper });

expect(result.current.themeMode).toBeUndefined();
expect(result.current.themeMode).toBe('light');
expect(result.current.themeVariables).toStrictEqual({});
});

Expand Down Expand Up @@ -99,24 +99,24 @@ describe('useAppKitTheme', () => {
expect(result.current.themeVariables).toEqual(themeVariables);
});

it('should call ThemeController.setThemeMode when setThemeMode is called', () => {
it('should call ThemeController.setDefaultThemeMode when setThemeMode is called', () => {
const { result } = renderHook(() => useAppKitTheme(), { wrapper });

act(() => {
result.current.setThemeMode('dark');
});

expect(ThemeController.setThemeMode).toHaveBeenCalledWith('dark');
expect(ThemeController.setDefaultThemeMode).toHaveBeenCalledWith('dark');
});

it('should call ThemeController.setThemeMode with undefined', () => {
it('should call ThemeController.setDefaultThemeMode with undefined', () => {
const { result } = renderHook(() => useAppKitTheme(), { wrapper });

act(() => {
result.current.setThemeMode(undefined);
});

expect(ThemeController.setThemeMode).toHaveBeenCalledWith(undefined);
expect(ThemeController.setDefaultThemeMode).toHaveBeenCalledWith(undefined);
});

it('should call ThemeController.setThemeVariables when setThemeVariables is called', () => {
Expand Down Expand Up @@ -172,10 +172,10 @@ describe('useAppKitTheme', () => {
result.current.setThemeMode(undefined);
});

expect(ThemeController.setThemeMode).toHaveBeenCalledTimes(3);
expect(ThemeController.setThemeMode).toHaveBeenNthCalledWith(1, 'dark');
expect(ThemeController.setThemeMode).toHaveBeenNthCalledWith(2, 'light');
expect(ThemeController.setThemeMode).toHaveBeenNthCalledWith(3, undefined);
expect(ThemeController.setDefaultThemeMode).toHaveBeenCalledTimes(3);
expect(ThemeController.setDefaultThemeMode).toHaveBeenNthCalledWith(1, 'dark');
expect(ThemeController.setDefaultThemeMode).toHaveBeenNthCalledWith(2, 'light');
expect(ThemeController.setDefaultThemeMode).toHaveBeenNthCalledWith(3, undefined);
});

it('should handle multiple setThemeVariables calls', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/appkit/src/hooks/useAppKitTheme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ export interface UseAppKitThemeReturn {
*/
export function useAppKitTheme(): UseAppKitThemeReturn {
useAppKitContext();

const { themeMode, themeVariables } = useSnapshot(ThemeController.state);

const stableFunctions = useMemo(
() => ({
setThemeMode: ThemeController.setThemeMode.bind(ThemeController),
setThemeMode: ThemeController.setDefaultThemeMode.bind(ThemeController),
setThemeVariables: ThemeController.setThemeVariables.bind(ThemeController)
}),
[]
Expand Down
8 changes: 3 additions & 5 deletions packages/appkit/src/modal/w3m-modal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function AppKit() {
const { bottom, top } = useSafeAreaInsets();
const { close } = useInternalAppKit();
const { open } = useSnapshot(ModalController.state);
const { themeMode, themeVariables, defaultThemeMode } = useSnapshot(ThemeController.state);
const { themeMode, themeVariables } = useSnapshot(ThemeController.state);
const { projectId } = useSnapshot(OptionsController.state);

const handleBackPress = () => {
Expand All @@ -35,10 +35,8 @@ export function AppKit() {
};

useEffect(() => {
if (theme && !defaultThemeMode) {
ThemeController.setThemeMode(theme);
}
}, [theme, defaultThemeMode]);
ThemeController.setSystemThemeMode(theme);
}, [theme]);

const prefetch = useCallback(async () => {
await ApiController.prefetch();
Expand Down
43 changes: 26 additions & 17 deletions packages/core/src/controllers/ThemeController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,58 @@
import { Appearance } from 'react-native';
import { proxy, subscribe as sub } from 'valtio';
import type { ThemeMode, ThemeVariables } from '@reown/appkit-common-react-native';
import { derive } from 'derive-valtio';

// -- Types --------------------------------------------- //
export interface ThemeControllerState {
themeMode?: ThemeMode;
defaultThemeMode?: ThemeMode;
systemThemeMode?: ThemeMode | null;
defaultThemeMode?: ThemeMode | null;
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type inconsistency: systemThemeMode and defaultThemeMode are typed as ThemeMode | null but are initialized with undefined. This creates a mismatch between the type definition and the actual runtime values. Consider either:

  1. Changing the type to ThemeMode | null | undefined or ThemeMode | undefined
  2. Initializing with null instead of undefined on lines 14-15

This inconsistency could lead to type-safety issues when accessing these properties.

Copilot uses AI. Check for mistakes.
themeVariables: ThemeVariables;
}

// -- State --------------------------------------------- //
const state = proxy<ThemeControllerState>({
themeMode: undefined,
const baseState = proxy<ThemeControllerState>({
systemThemeMode: undefined,
defaultThemeMode: undefined,
themeVariables: {}
});

// -- Derived State ------------------------------------- //
const derivedState = derive(
{
themeMode: (get): ThemeMode => {
const snap = get(baseState);

return snap.defaultThemeMode ?? snap.systemThemeMode ?? 'light';
}
},
{
proxy: baseState
}
);

// -- Controller ---------------------------------------- //
export const ThemeController = {
state,
state: derivedState,

subscribe(callback: (newState: ThemeControllerState) => void) {
return sub(state, () => callback(state));
return sub(derivedState, () => callback(derivedState));
},

setThemeMode(themeMode?: ThemeControllerState['themeMode']) {
if (!themeMode) {
state.themeMode = (Appearance.getColorScheme() ?? 'light') as ThemeMode;
} else {
state.themeMode = themeMode;
}
setSystemThemeMode(systemThemeMode?: ThemeControllerState['systemThemeMode']) {
baseState.systemThemeMode = systemThemeMode ?? 'light';
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setSystemThemeMode function always defaults to 'light' when the parameter is undefined or null. However, when the system theme changes to null (which useColorScheme() can return), this will force it to 'light' rather than preserving the null value. This could prevent proper fallback to the default theme mode. Consider allowing null to be set when explicitly passed, or handle the React Native's useColorScheme() null return value more gracefully.

Suggested change
baseState.systemThemeMode = systemThemeMode ?? 'light';
baseState.systemThemeMode = systemThemeMode === undefined ? 'light' : systemThemeMode;

Copilot uses AI. Check for mistakes.
},

setDefaultThemeMode(themeMode?: ThemeControllerState['defaultThemeMode']) {
state.defaultThemeMode = themeMode;
this.setThemeMode(themeMode);
baseState.defaultThemeMode = themeMode;
},

setThemeVariables(themeVariables?: ThemeControllerState['themeVariables']) {
if (!themeVariables) {
state.themeVariables = {};
baseState.themeVariables = {};

return;
}

state.themeVariables = { ...state.themeVariables, ...themeVariables };
baseState.themeVariables = { ...baseState.themeVariables, ...themeVariables };
}
};
Loading