Merge pull request #58 from lordmathis/fix/theme-switching

Fix/theme switching

app/src/App.tsx

@@ -1,5 +1,9 @@
 import React from 'react';
-import { MantineProvider, ColorSchemeScript } from '@mantine/core';
+import {
+  MantineProvider,
+  ColorSchemeScript,
+  localStorageColorSchemeManager,
+} from '@mantine/core';
 import { Notifications } from '@mantine/notifications';
 import { ModalsProvider } from '@mantine/modals';
 import Layout from './components/layout/Layout';
@@ -39,11 +43,18 @@ const AuthenticatedContent: React.FC<AuthenticatedContentProps> = () => {
 
 type AppProps = object;
 
+const colorSchemeManager = localStorageColorSchemeManager({
+  key: 'mantine-color-scheme',
+});
+
 const App: React.FC<AppProps> = () => {
   return (
     <>
       <ColorSchemeScript defaultColorScheme="light" />
-      <MantineProvider defaultColorScheme="light">
+      <MantineProvider
+        defaultColorScheme="light"
+        colorSchemeManager={colorSchemeManager}
+      >
         <Notifications />
         <ModalsProvider>
           <AuthProvider>

app/src/components/navigation/WorkspaceSwitcher.tsx

@@ -110,10 +110,10 @@ const WorkspaceSwitcher: React.FC = () => {
                 </Center>
               ) : (
                 workspaces.map((workspace) => {
-                  const isSelected = workspace.name === currentWorkspace?.name;
+                  const isSelected = workspace.id === currentWorkspace?.id;
                   return (
                     <Paper
-                      key={workspace.name}
+                      key={workspace.id}
                       p="xs"
                       withBorder
                       style={(theme) =>

app/src/components/settings/workspace/AppearanceSettings.test.tsx

@@ -20,8 +20,6 @@ const render = (ui: React.ReactElement) => {
 };
 
 describe('AppearanceSettings', () => {
-  const mockOnThemeChange = vi.fn();
-
   beforeEach(async () => {
     vi.clearAllMocks();
     const { useTheme } = await import('../../../contexts/ThemeContext');
@@ -32,7 +30,7 @@ describe('AppearanceSettings', () => {
   });
 
   it('renders dark mode toggle with correct state', () => {
-    render(<AppearanceSettings onThemeChange={mockOnThemeChange} />);
+    render(<AppearanceSettings />);
 
     expect(screen.getByText('Dark Mode')).toBeInTheDocument();
     const toggle = screen.getByRole('switch');
@@ -46,20 +44,19 @@ describe('AppearanceSettings', () => {
       updateColorScheme: mockUpdateColorScheme,
     });
 
-    render(<AppearanceSettings onThemeChange={mockOnThemeChange} />);
+    render(<AppearanceSettings />);
 
     const toggle = screen.getByRole('switch');
     expect(toggle).toBeChecked();
   });
 
   it('toggles theme from light to dark', () => {
-    render(<AppearanceSettings onThemeChange={mockOnThemeChange} />);
+    render(<AppearanceSettings />);
 
     const toggle = screen.getByRole('switch');
     fireEvent.click(toggle);
 
     expect(mockUpdateColorScheme).toHaveBeenCalledWith(Theme.Dark);
-    expect(mockOnThemeChange).toHaveBeenCalledWith(Theme.Dark);
   });
 
   it('toggles theme from dark to light', async () => {
@@ -69,12 +66,11 @@ describe('AppearanceSettings', () => {
       updateColorScheme: mockUpdateColorScheme,
     });
 
-    render(<AppearanceSettings onThemeChange={mockOnThemeChange} />);
+    render(<AppearanceSettings />);
 
     const toggle = screen.getByRole('switch');
     fireEvent.click(toggle);
 
     expect(mockUpdateColorScheme).toHaveBeenCalledWith(Theme.Light);
-    expect(mockOnThemeChange).toHaveBeenCalledWith(Theme.Light);
   });
 });

app/src/components/settings/workspace/AppearanceSettings.tsx

@@ -1,21 +1,14 @@
 import React from 'react';
 import { Text, Switch, Group, Box } from '@mantine/core';
-import { useTheme } from '../../../contexts/ThemeContext';
 import { Theme } from '@/types/models';
+import { useTheme } from '../../../contexts/ThemeContext';
 
-interface AppearanceSettingsProps {
-  onThemeChange: (newTheme: Theme) => void;
-}
-
-const AppearanceSettings: React.FC<AppearanceSettingsProps> = ({
-  onThemeChange,
-}) => {
+const AppearanceSettings: React.FC = () => {
   const { colorScheme, updateColorScheme } = useTheme();
 
   const handleThemeChange = (): void => {
     const newTheme = colorScheme === 'dark' ? Theme.Light : Theme.Dark;
     updateColorScheme(newTheme);
-    onThemeChange(newTheme);
   };
 
   return (

app/src/components/settings/workspace/WorkspaceSettings.test.tsx

@@ -11,6 +11,7 @@ import WorkspaceSettings from './WorkspaceSettings';
 import { Theme } from '@/types/models';
 
 const mockUpdateSettings = vi.fn();
+const mockUpdateColorScheme = vi.fn();
 vi.mock('../../../hooks/useWorkspace', () => ({
   useWorkspace: vi.fn(),
 }));
@@ -48,11 +49,9 @@ vi.mock('./GeneralSettings', () => ({
 }));
 
 vi.mock('./AppearanceSettings', () => ({
-  default: ({ onThemeChange }: { onThemeChange: (theme: string) => void }) => (
+  default: () => (
     <div data-testid="appearance-settings">
-      <button onClick={() => onThemeChange('dark')} data-testid="theme-toggle">
-        Toggle Theme
-      </button>
+      Appearance Settings
     </div>
   ),
 }));
@@ -105,7 +104,7 @@ describe('WorkspaceSettings', () => {
       updateSettings: mockUpdateSettings,
       loading: false,
       colorScheme: 'light',
-      updateColorScheme: vi.fn(),
+      updateColorScheme: mockUpdateColorScheme,
       switchWorkspace: vi.fn(),
       deleteCurrentWorkspace: vi.fn(),
     });
@@ -152,13 +151,10 @@ describe('WorkspaceSettings', () => {
     });
   });
 
-  it('handles theme changes', () => {
+  it('renders appearance settings', () => {
     render(<WorkspaceSettings />);
 
-    const themeToggle = screen.getByTestId('theme-toggle');
-    fireEvent.click(themeToggle);
-
-    expect(screen.getByText('Unsaved Changes')).toBeInTheDocument();
+    expect(screen.getByTestId('appearance-settings')).toBeInTheDocument();
   });
 
   it('closes modal when cancel is clicked', () => {
@@ -192,4 +188,16 @@ describe('WorkspaceSettings', () => {
 
     expect(mockUpdateSettings).not.toHaveBeenCalled();
   });
+
+  it('reverts theme when canceling', () => {
+    render(<WorkspaceSettings />);
+
+    // Click cancel
+    const cancelButton = screen.getByRole('button', { name: 'Cancel' });
+    fireEvent.click(cancelButton);
+
+    // Theme should be reverted to saved state (Light)
+    expect(mockUpdateColorScheme).toHaveBeenCalledWith(Theme.Light);
+    expect(mockSetSettingsModalVisible).toHaveBeenCalledWith(false);
+  });
 });

app/src/components/settings/workspace/WorkspaceSettings.tsx

@@ -1,4 +1,4 @@
-import React, { useReducer, useEffect, useCallback, useRef } from 'react';
+import React, { useReducer, useEffect, useCallback } from 'react';
 import {
   Modal,
   Badge,
@@ -72,14 +72,13 @@ function settingsReducer(
 }
 
 const WorkspaceSettings: React.FC = () => {
-  const { currentWorkspace, updateSettings } = useWorkspace();
+  const { currentWorkspace, updateSettings, updateColorScheme, colorScheme } =
+    useWorkspace();
   const { settingsModalVisible, setSettingsModalVisible } = useModalContext();
   const [state, dispatch] = useReducer(settingsReducer, initialState);
-  const isInitialMount = useRef<boolean>(true);
 
   useEffect(() => {
-    if (isInitialMount.current && currentWorkspace) {
-      isInitialMount.current = false;
+    if (currentWorkspace && settingsModalVisible) {
       const settings: Partial<Workspace> = {
         name: currentWorkspace.name,
         theme: currentWorkspace.theme,
@@ -96,7 +95,7 @@ const WorkspaceSettings: React.FC = () => {
       };
       dispatch({ type: SettingsActionType.INIT_SETTINGS, payload: settings });
     }
-  }, [currentWorkspace]);
+  }, [currentWorkspace, settingsModalVisible]);
 
   const handleInputChange = useCallback(
     <K extends keyof Workspace>(key: K, value: Workspace[K]): void => {
@@ -118,7 +117,13 @@ const WorkspaceSettings: React.FC = () => {
         return;
       }
 
-      await updateSettings(state.localSettings);
+      // Save with current Mantine theme
+      const settingsToSave = {
+        ...state.localSettings,
+        theme: colorScheme as Theme,
+      };
+
+      await updateSettings(settingsToSave);
       dispatch({ type: SettingsActionType.MARK_SAVED });
       notifications.show({
         message: 'Settings saved successfully',
@@ -137,8 +142,12 @@ const WorkspaceSettings: React.FC = () => {
   };
 
   const handleClose = useCallback(() => {
+    // Revert theme to saved state
+    if (state.initialSettings.theme) {
+      updateColorScheme(state.initialSettings.theme);
+    }
     setSettingsModalVisible(false);
-  }, [setSettingsModalVisible]);
+  }, [setSettingsModalVisible, state.initialSettings.theme, updateColorScheme]);
 
   return (
     <Modal
@@ -180,11 +189,7 @@ const WorkspaceSettings: React.FC = () => {
           <Accordion.Item value="appearance">
             <AccordionControl>Appearance</AccordionControl>
             <Accordion.Panel>
-              <AppearanceSettings
-                onThemeChange={(newTheme: string) =>
-                  handleInputChange('theme', newTheme as Theme)
-                }
-              />
+              <AppearanceSettings />
             </Accordion.Panel>
           </Accordion.Item>
 

app/src/contexts/ThemeContext.tsx

@@ -2,6 +2,7 @@ import React, {
   createContext,
   useContext,
   useCallback,
+  useMemo,
   type ReactNode,
 } from 'react';
 import { useMantineColorScheme, type MantineColorScheme } from '@mantine/core';
@@ -32,11 +33,16 @@ export const ThemeProvider: React.FC<ThemeProviderProps> = ({ children }) => {
   );
 
   // Ensure colorScheme is never undefined by falling back to light theme
-  const value: ThemeContextType = {
-    colorScheme:
-      colorScheme === 'light' || colorScheme === 'dark' ? colorScheme : 'light',
-    updateColorScheme,
-  };
+  const normalizedColorScheme =
+    colorScheme === 'light' || colorScheme === 'dark' ? colorScheme : 'light';
+
+  const value: ThemeContextType = useMemo(
+    () => ({
+      colorScheme: normalizedColorScheme,
+      updateColorScheme,
+    }),
+    [normalizedColorScheme, updateColorScheme]
+  );
 
   return (
     <ThemeContext.Provider value={value}>{children}</ThemeContext.Provider>

app/src/contexts/WorkspaceDataContext.tsx

@@ -74,7 +74,8 @@ export const WorkspaceDataProvider: React.FC<WorkspaceDataProviderProps> = ({
         });
       }
     },
-    [updateColorScheme]
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    []
   );
 
   const loadFirstAvailableWorkspace = useCallback(async (): Promise<void> => {