Skip to content

Commit f1b7413

Browse files
samejrclaude
andcommitted
fix(webapp): apply the theme on click, not on the loader's next pass
Picking a theme from the account menu sometimes left the old one on screen. The switch waited for the write to come back through the root loader, but dismissing the popover unmounts the row that owns the fetcher, and without v3_fetcherPersist React Router drops an unmounted fetcher's revalidation. The POST had already gone out, so the preference saved and a refresh showed the new theme - which is why it looked intermittent. Both pickers now set the attribute themselves and let the write follow, sharing the resolution rule with useSystemThemeSync rather than restating it, and revert if the write comes back unsuccessful. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 07d62a1 commit f1b7413

3 files changed

Lines changed: 48 additions & 11 deletions

File tree

apps/webapp/app/components/navigation/AppearanceMenuItem.tsx

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import { EllipsisHorizontalIcon } from "@heroicons/react/20/solid";
22
import { useFetcher } from "@remix-run/react";
3+
import { useEffect } from "react";
34
import { useTypedRouteLoaderData } from "remix-typedjson";
45
import { ToggleSwitchIcon } from "~/assets/icons/ToggleSwitchIcon";
56
import { PopoverMenuItem } from "~/components/primitives/Popover";
67
import { THEME_OPTIONS } from "~/components/themeOptions";
8+
import { applyThemePreference } from "~/hooks/useSystemThemeSync";
79
import { type loader as rootLoader } from "~/root";
810
import { accountPath } from "~/utils/pathBuilder";
9-
import { normalizeThemePreference } from "~/utils/themePreference";
11+
import { normalizeThemePreference, type ThemePreference } from "~/utils/themePreference";
1012
import { SideMenuPopoverSubMenu } from "./SideMenuPopoverSubMenu";
1113
import { SIDE_MENU_POPOVER_ITEM_ICON, SIDE_MENU_POPOVER_ITEM_LABEL } from "./sideMenuTypes";
1214

@@ -20,20 +22,36 @@ const THEME_ACTION_PATH = "/resources/preferences/theme";
2022
*/
2123
export function AppearanceMenuItem() {
2224
const rootData = useTypedRouteLoaderData<typeof rootLoader>("root");
23-
const fetcher = useFetcher();
25+
const fetcher = useFetcher<{ success?: boolean }>();
26+
const savedTheme = rootData?.themePreference;
27+
28+
// A failed write would otherwise leave the optimistic theme on screen, since
29+
// the loader data never changes and so `useSystemThemeSync` never re-runs.
30+
useEffect(() => {
31+
if (fetcher.state !== "idle" || !fetcher.data || fetcher.data.success || !savedTheme) return;
32+
applyThemePreference(savedTheme);
33+
}, [fetcher.state, fetcher.data, savedTheme]);
2434

2535
if (!rootData?.showThemeSwitcher) {
2636
return null;
2737
}
2838

29-
// Move the check as soon as a theme is clicked; the theme itself follows once
30-
// the write lands and the root loader revalidates.
39+
// Move the check as soon as a theme is clicked; the write follows.
3140
const pendingTheme = fetcher.formData?.get("theme");
3241
const theme =
3342
typeof pendingTheme === "string"
3443
? normalizeThemePreference(pendingTheme)
3544
: rootData.themePreference;
3645

46+
const pickTheme = (value: ThemePreference) => {
47+
// Applied here rather than waiting for the write to come back through the
48+
// root loader: dismissing the popover unmounts this row, and an unmounted
49+
// fetcher's revalidation is dropped, which left the theme untouched even
50+
// though the preference had saved.
51+
applyThemePreference(value);
52+
fetcher.submit({ theme: value }, { method: "post", action: THEME_ACTION_PATH });
53+
};
54+
3755
return (
3856
// Much narrower than the standard submenu: these labels don't need the room.
3957
<SideMenuPopoverSubMenu title="Appearance" icon={ToggleSwitchIcon} contentClassName="min-w-36">
@@ -46,9 +64,7 @@ export function AppearanceMenuItem() {
4664
leadingIconClassName={SIDE_MENU_POPOVER_ITEM_ICON}
4765
className={SIDE_MENU_POPOVER_ITEM_LABEL}
4866
isSelected={theme === option.value}
49-
onClick={() =>
50-
fetcher.submit({ theme: option.value }, { method: "post", action: THEME_ACTION_PATH })
51-
}
67+
onClick={() => pickTheme(option.value)}
5268
/>
5369
))}
5470
</div>

apps/webapp/app/hooks/useSystemThemeSync.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,23 @@
11
import { useEffect } from "react";
22
import { type ThemePreference } from "~/utils/themePreference";
33

4+
/**
5+
* Puts a preference on <html> now, resolving `system` against the OS once. Use
6+
* this to apply a theme the moment it's picked: the preference round-trips
7+
* through the server and comes back via the root loader, and anything that waits
8+
* for that is at the mercy of whether the revalidation actually lands.
9+
*/
10+
export function applyThemePreference(preference: ThemePreference) {
11+
const resolved =
12+
preference === "system"
13+
? window.matchMedia("(prefers-color-scheme: dark)").matches
14+
? "dark"
15+
: "light"
16+
: preference;
17+
document.documentElement.setAttribute("data-theme", resolved);
18+
document.documentElement.setAttribute("data-theme-preference", preference);
19+
}
20+
421
/**
522
* Keeps `data-theme` on <html> in sync with the preference. For `system` it
623
* follows the OS color scheme live; for pinned themes it writes the attribute
@@ -13,7 +30,7 @@ import { type ThemePreference } from "~/utils/themePreference";
1330
export function useSystemThemeSync(preference: ThemePreference) {
1431
useEffect(() => {
1532
if (preference !== "system") {
16-
document.documentElement.setAttribute("data-theme", preference);
33+
applyThemePreference(preference);
1734
return;
1835
}
1936

apps/webapp/app/routes/account._index/route.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { ALL_THEME_OPTIONS, THEME_OPTIONS_BY_VALUE } from "~/components/themeOpt
3939
import { prisma } from "~/db.server";
4040
import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server";
4141
import { useFeatureFlags } from "~/hooks/useFeatureFlags";
42+
import { applyThemePreference } from "~/hooks/useSystemThemeSync";
4243
import { useFeatures } from "~/hooks/useFeatures";
4344
import { useHasAdminAccess, useUser } from "~/hooks/useUser";
4445
import { redirectWithSuccessMessage } from "~/models/message.server";
@@ -505,12 +506,15 @@ export default function Page() {
505506
<Select<ThemePreference, ThemePreference>
506507
aria-label="Interface theme"
507508
value={theme}
508-
setValue={(value) =>
509+
setValue={(value) => {
510+
// Applied here so the theme lands immediately rather than
511+
// on the root loader's next pass (see applyThemePreference).
512+
applyThemePreference(normalizeThemePreference(value));
509513
themeFetcher.submit(
510514
{ action: "update-theme", theme: value },
511515
{ method: "post" }
512-
)
513-
}
516+
);
517+
}}
514518
variant="secondary/medium"
515519
dropdownIcon
516520
items={ALL_THEME_OPTIONS.map((option) => option.value)}

0 commit comments

Comments
 (0)