Fix theme persistence and auth session validation. by biplab-sutradhar · Pull Request #986 · TeamShiksha/openlogo

biplab-sutradhar · 2026-03-30T16:31:46Z

closes #987

Description

Updated ThemeProvider to support light, dark, and device modes
optimized AuthProvider session validation to avoid slow initial loading

What type of PR is this? (Check all applicable)

Screenshots (if applicable)

Theme persistence

thememode.mp4

Auth session validation

auth.mp4

Checklist

I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

…ests

Copilot

Pull request overview

This PR updates the UI context layer to improve theme persistence (adding light / dark / device modes) and refactors AuthProvider startup session validation to reduce initial loading delays.

Changes:

Replaced the boolean dark mode state with a string-based theme state (light/dark/device) and persists it to localStorage.
Updated the theme toggle UI to cycle through the three theme modes.
Refactored AuthProvider session validation flow and updated associated unit tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`packages/ui/src/utils/Constants.js`	Adds a theme-cycling mapping used by the toggle component.
`packages/ui/src/contexts/ThemeContext.jsx`	Implements `theme` state + device-mode handling via `matchMedia`, persists theme to `localStorage`, and changes provided context shape.
`packages/ui/src/contexts/AuthContext.jsx`	Changes session validation flow and removes gating of children rendering.
`packages/ui/src/components/darkModeToggle/DarkModeToggle.jsx`	Updates toggle behavior to cycle light → dark → device.
`packages/ui/__tests__/contexts/ThemeContext.test.jsx`	Updates tests for the new `theme` API and device-mode behavior.
`packages/ui/__tests__/contexts/AuthContext.test.jsx`	Updates mocks/tests to reflect refactored session validation and logout calls.
`packages/ui/__tests__/components/DarkModeToggle.test.jsx`	Updates toggle tests for the new cycling behavior and context API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T19:13:53Z

+    const saved = localStorage.getItem("theme");
+    if (saved) return saved;


Theme initialization reads localStorage.getItem("theme") without validation/migration. This can regress existing users who have the old darkMode key, and it also allows invalid stored values (e.g. "foo") which later break icon[theme] / nextTheme[theme] and prevents applying any theme. Consider validating against an allow-list (light/dark/device), migrating darkMode -> theme on first load, and falling back to a safe default when the stored value is unrecognized.

Suggested change

const saved = localStorage.getItem("theme");

if (saved) return saved;

const allowedThemes = new Set(["light", "dark", "device"]);

const saved = localStorage.getItem("theme");

if (saved && allowedThemes.has(saved)) {

return saved;

}

const legacyDarkMode = localStorage.getItem("darkMode");

if (legacyDarkMode === "true") {

localStorage.setItem("theme", "dark");

return "dark";

}

if (legacyDarkMode === "false") {

localStorage.setItem("theme", "light");

return "light";

}

localStorage.setItem("theme", "light");

Copilot · 2026-03-31T19:13:53Z

  const contextValue = useMemo(
-    () => ({ isDarkMode, toggleTheme }),
-    [isDarkMode]
+    () => ({ theme, setTheme, toggleTheme }),
+    [theme]
  );


ThemeProvider no longer exposes isDarkMode in the context value (it now provides { theme, setTheme, toggleTheme }). There are existing consumers (e.g. Graph.jsx) that read isDarkMode from ThemeContext, which will now be undefined and cause dark-theme styling to never apply. To avoid breaking existing components, either update all consumers in this PR or keep backward compatibility by also providing a derived isDarkMode boolean.

Copilot · 2026-03-31T19:13:53Z

+      const mq = window.matchMedia("(prefers-color-scheme: dark)");
+      const update = () => applyTheme(mq.matches);
+      update();
+      mq.addEventListener("change", update);
+      return () => mq.removeEventListener("change", update);


Device mode calls window.matchMedia(...) and uses addEventListener/removeEventListener without checking availability. In environments where matchMedia is missing or where only addListener/removeListener exist, selecting theme === "device" will throw. Consider guarding for window.matchMedia and supporting the older listener API (or falling back to light/dark) to avoid runtime crashes.

Suggested change

const mq = window.matchMedia("(prefers-color-scheme: dark)");

const update = () => applyTheme(mq.matches);

update();

mq.addEventListener("change", update);

return () => mq.removeEventListener("change", update);

// Guard against environments without window or matchMedia

if (typeof window === "undefined" || typeof window.matchMedia !== "function") {

// Fallback: default to light theme when device preference is unavailable

applyTheme(false);

return;

}

const mq = window.matchMedia("(prefers-color-scheme: dark)");

const update = () => applyTheme(mq.matches);

update();

// Prefer modern event listener API if available

if (typeof mq.addEventListener === "function" && typeof mq.removeEventListener === "function") {

mq.addEventListener("change", update);

return () => mq.removeEventListener("change", update);

}

// Fallback to older addListener/removeListener APIs

if (typeof mq.addListener === "function" && typeof mq.removeListener === "function") {

mq.addListener(update);

return () => mq.removeListener(update);

}

// If no listener API is available, we already applied the theme once

return;

Copilot · 2026-03-31T19:13:54Z

+export const nextTheme = {
+  light: "dark",
+  dark: "device",
+  device: "light",
+};


nextTheme is exported with a lower-camel name while every other exported constant in this module is UPPER_SNAKE_CASE. To stay consistent and make intent clearer, consider renaming to something like NEXT_THEME/THEME_CYCLE (and updating imports), or colocating this mapping in the theme module since it’s not a general UI string/asset constant.

Copilot · 2026-03-31T19:13:54Z

+    <button
+      className={styles.darkModeToggle}
+      onClick={() => setTheme(nextTheme[theme])}
+    >
+      {icon[theme]}
    </button>


onClick={() => setTheme(nextTheme[theme])} will set theme to undefined if the current theme value is unexpected (e.g. corrupted localStorage value). This leaves the UI without an icon and prevents theme application. Consider adding a fallback (e.g. default to "light") and/or ensure the provider guarantees theme is always one of the supported values before it reaches this component.

Copilot · 2026-03-31T19:13:54Z

@@ -41,21 +37,19 @@ export const AuthProvider = ({ children }) => {
      setIsAuthenticated(false);
      toast.success(MESSAGES.SIGN_OUT_SUCCESS);
    }
-  }, [setIsAuthenticated, makeRequest, toast]);
+  }, [makeRequest, toast]);

  const authValue = useMemo(
    () => ({
      isAuthenticated,
      setIsAuthenticated,
      logout,
    }),
-    [isAuthenticated, setIsAuthenticated, logout]
+    [isAuthenticated, logout]
  );

  return (
-    <AuthContext.Provider value={authValue}>
-      {isAuthCheckComplete ? children : null}
-    </AuthContext.Provider>
+    <AuthContext.Provider value={authValue}>{children}</AuthContext.Provider>
  );


AuthProvider now renders children immediately and removed the auth-check completion gate. With isAuthenticated defaulting to false, routes wrapped in ProtectedRoute will redirect to / before validateSession() resolves, breaking session persistence on page refresh. Consider restoring an isAuthCheckComplete/isValidatingSession flag (and using it in ProtectedRoute to show a spinner instead of redirecting) or keep the provider-level gating until validation finishes.

Copilot · 2026-03-31T19:13:55Z

+  it("cleans up matchMedia listener on unmount when using device mode", async () => {
+    const removeListener = vi.fn();
+    globalThis.matchMedia = vi.fn().mockReturnValue({
+      matches: false,
+      addEventListener: vi.fn(),
+      removeEventListener: removeListener,
+    });

-    render(
+    const { unmount } = render(
      <ThemeProvider>
        <TestComponent />
      </ThemeProvider>
    );

-    const themeStatus = screen.getByTestId("theme-status").textContent;
-    expect(themeStatus).toBe("Light Mode");
-
-    globalThis.matchMedia = originalMatchMedia;
-  });
-
-  it("provides correct context value", async () => {
-    let contextValue = null;
-
-    const ContextChecker = () => {
-      contextValue = useContext(ThemeContext);
-      return null;
-    };
+    fireEvent.click(screen.getByTestId("set-device-btn"));

-    render(
-      <ThemeProvider>
-        <ContextChecker />
-      </ThemeProvider>
-    );
+    unmount();

-    await waitFor(() => {
-      expect(contextValue).toHaveProperty("isDarkMode");
-      expect(contextValue).toHaveProperty("toggleTheme");
-      expect(typeof contextValue.isDarkMode).toBe("boolean");
-      expect(typeof contextValue.toggleTheme).toBe("function");
-    });
+    expect(removeListener).toHaveBeenCalled();


This test can be flaky because it unmounts immediately after switching to device mode; React effects that register the matchMedia listener may not have run yet, so there may be no cleanup to execute on unmount. Consider awaiting an assertion that the device-mode effect ran (e.g. theme applied or addEventListener called) before calling unmount(), then assert removeEventListener was called.

…use theme context

sachinkmrsin

Code looks correct, please address the issues raised by copilot.

…e-auth

sonarqubecloud · 2026-04-20T16:52:37Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

refactor: Update theme handling in DarkModeToggle, auth and related t…

81757f4

…ests

biplab-sutradhar self-assigned this Mar 30, 2026

Dhirenderchoudhary requested a review from Copilot March 31, 2026 19:08

Copilot started reviewing on behalf of Dhirenderchoudhary March 31, 2026 19:09 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

biplab-sutradhar added the enhancement New feature or request label Apr 1, 2026

refactor: Simplify theme cycling tests and update Graph component to …

115603b

…use theme context

biplab-sutradhar marked this pull request as ready for review April 3, 2026 16:33

biplab-sutradhar requested review from AryaDharkar and Smayur0 and removed request for AryaDharkar April 3, 2026 16:47

sachinkmrsin requested changes Apr 8, 2026

View reviewed changes

biplab-sutradhar added 3 commits April 10, 2026 22:02

Merge branch 'stage' of github.com:TeamShiksha/openlogo into fix/them…

93264b7

…e-auth

Merge branch 'stage' of github.com:TeamShiksha/openlogo into fix/them…

deea12e

…e-auth

feat: enhance theme management

f6c8d57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix theme persistence and auth session validation.#986

Fix theme persistence and auth session validation.#986
biplab-sutradhar wants to merge 5 commits intostagefrom
fix/theme-auth

biplab-sutradhar commented Mar 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

sachinkmrsin left a comment

Uh oh!

sonarqubecloud Bot commented Apr 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const saved = localStorage.getItem("theme");
		if (saved) return saved;

-    const saved = localStorage.getItem("theme");
-    if (saved) return saved;
+    const allowedThemes = new Set(["light", "dark", "device"]);
+    const saved = localStorage.getItem("theme");
+    if (saved && allowedThemes.has(saved)) {
+      return saved;
+    }
+    const legacyDarkMode = localStorage.getItem("darkMode");
+    if (legacyDarkMode === "true") {
+      localStorage.setItem("theme", "dark");
+      return "dark";
+    }
+    if (legacyDarkMode === "false") {
+      localStorage.setItem("theme", "light");
+      return "light";
+    }
+    localStorage.setItem("theme", "light");

-      const mq = window.matchMedia("(prefers-color-scheme: dark)");
-      const update = () => applyTheme(mq.matches);
-      update();
-      mq.addEventListener("change", update);
-      return () => mq.removeEventListener("change", update);
+      // Guard against environments without window or matchMedia
+      if (typeof window === "undefined" || typeof window.matchMedia !== "function") {
+        // Fallback: default to light theme when device preference is unavailable
+        applyTheme(false);
+        return;
+      }
+      const mq = window.matchMedia("(prefers-color-scheme: dark)");
+      const update = () => applyTheme(mq.matches);
+      update();
+      // Prefer modern event listener API if available
+      if (typeof mq.addEventListener === "function" && typeof mq.removeEventListener === "function") {
+        mq.addEventListener("change", update);
+        return () => mq.removeEventListener("change", update);
+      }
+      // Fallback to older addListener/removeListener APIs
+      if (typeof mq.addListener === "function" && typeof mq.removeListener === "function") {
+        mq.addListener(update);
+        return () => mq.removeListener(update);
+      }
+      // If no listener API is available, we already applied the theme once
+      return;

Conversation

biplab-sutradhar commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What type of PR is this? (Check all applicable)

Screenshots (if applicable)

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

sachinkmrsin left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 20, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

biplab-sutradhar commented Mar 30, 2026 •

edited

Loading