diff --git a/app/components/league/settings/SettingsNavigation.tsx b/app/components/league/settings/SettingsNavigation.tsx
index ec0a139..d6542bb 100644
--- a/app/components/league/settings/SettingsNavigation.tsx
+++ b/app/components/league/settings/SettingsNavigation.tsx
@@ -1,6 +1,7 @@
import type { ComponentType } from "react";
import type { LucideProps } from "lucide-react";
import { ArrowLeft } from "lucide-react";
+import { Link } from "react-router";
import { cn } from "~/lib/utils";
type SettingsNavSection = {
@@ -17,10 +18,37 @@ export type SettingsGridSection = SettingsNavSection & {
export function SettingsMobileGridNav({
sections,
onSectionChange,
+ buildHref,
}: {
sections: readonly SettingsGridSection[];
- onSectionChange: (sectionId: string) => void;
+ onSectionChange?: (sectionId: string) => void;
+ buildHref?: (sectionId: string) => string;
}) {
+ const cardClassName = (section: SettingsGridSection) =>
+ cn(
+ "flex cursor-pointer flex-col gap-3 rounded-xl border bg-card p-4 text-left transition-colors hover:border-primary/40 active:scale-[0.98]",
+ section.isDanger && "border-destructive/40"
+ );
+
+ const cardInner = (section: SettingsGridSection) => (
+ <>
+
+
+
+
+
{section.label}
+
{section.subtitle}
+
+ >
+ );
+
return (
@@ -29,32 +57,22 @@ export function SettingsMobileGridNav({
- {sections.map((section) => (
-
- ))}
+ {cardInner(section)}
+
+ )
+ )}
);
@@ -62,19 +80,30 @@ export function SettingsMobileGridNav({
export function SettingsMobileSectionPill({
onShowGrid,
+ backHref,
}: {
- onShowGrid: () => void;
+ onShowGrid?: () => void;
+ backHref?: string;
}) {
+ const className =
+ "flex cursor-pointer items-center gap-1.5 rounded-full border bg-card px-3 py-1.5 text-sm font-medium hover:bg-muted";
+ const content = (
+ <>
+
+ Back to all settings
+ >
+ );
return (
-
-
- Back to all settings
-
+ {backHref ? (
+
+ {content}
+
+ ) : (
+
+ {content}
+
+ )}
);
}
@@ -83,33 +112,57 @@ export function SettingsDesktopNav({
sections,
activeSection,
onSectionChange,
+ buildHref,
+ navLabel = "League settings",
}: {
sections: readonly SettingsGridSection[];
activeSection: string;
- onSectionChange: (sectionId: string) => void;
+ onSectionChange?: (sectionId: string) => void;
+ buildHref?: (sectionId: string) => string;
+ navLabel?: string;
}) {
+ const itemClassName = (section: SettingsGridSection) =>
+ cn(
+ "flex w-full cursor-pointer items-center gap-2.5 rounded-md px-3 py-2 text-left text-sm font-medium text-muted-foreground hover:bg-muted hover:text-foreground",
+ activeSection === section.id && "bg-muted text-foreground"
+ );
+
+ const itemInner = (section: SettingsGridSection) => (
+ <>
+
+ {section.label}
+ >
+ );
+
return (
diff --git a/app/components/league/settings/__tests__/SettingsNavigation.test.tsx b/app/components/league/settings/__tests__/SettingsNavigation.test.tsx
new file mode 100644
index 0000000..fe5dd64
--- /dev/null
+++ b/app/components/league/settings/__tests__/SettingsNavigation.test.tsx
@@ -0,0 +1,67 @@
+import { describe, it, expect, vi } from "vitest";
+import { render, screen, fireEvent } from "@testing-library/react";
+import { MemoryRouter } from "react-router";
+import { Bell, User } from "lucide-react";
+import {
+ SettingsDesktopNav,
+ SettingsMobileGridNav,
+ SettingsMobileSectionPill,
+ type SettingsGridSection,
+} from "../SettingsNavigation";
+
+const sections: readonly SettingsGridSection[] = [
+ { id: "profile", label: "Profile", icon: User, subtitle: "Name, avatar" },
+ { id: "notifications", label: "Notifications", icon: Bell, subtitle: "Email & Discord" },
+];
+
+describe("SettingsNavigation link mode", () => {
+ it("renders desktop nav entries as anchors built from buildHref", () => {
+ render(
+
+ `/settings/${id}`}
+ />
+
+ );
+
+ expect(screen.getByRole("link", { name: "Profile" })).toHaveAttribute("href", "/settings/profile");
+ expect(screen.getByRole("link", { name: "Notifications" })).toHaveAttribute(
+ "href",
+ "/settings/notifications"
+ );
+ expect(screen.getByRole("link", { name: "Profile" })).toHaveAttribute("aria-current", "page");
+ });
+
+ it("renders the mobile grid as anchors and the back pill as an anchor", () => {
+ render(
+
+ `/settings/${id}`} />
+
+
+ );
+
+ expect(screen.getByRole("link", { name: /Profile/i })).toHaveAttribute("href", "/settings/profile");
+ expect(screen.getByRole("link", { name: /back to all settings/i })).toHaveAttribute(
+ "href",
+ "/settings"
+ );
+ });
+});
+
+describe("SettingsNavigation button mode (league settings)", () => {
+ it("still fires onSectionChange when no buildHref is provided", () => {
+ const onSectionChange = vi.fn();
+ render(
+
+ );
+
+ fireEvent.click(screen.getByRole("button", { name: "Notifications" }));
+ expect(onSectionChange).toHaveBeenCalledWith("notifications");
+ });
+});
diff --git a/app/components/user/settings/AccountSection.tsx b/app/components/user/settings/AccountSection.tsx
index 27597c7..d5c07b0 100644
--- a/app/components/user/settings/AccountSection.tsx
+++ b/app/components/user/settings/AccountSection.tsx
@@ -31,7 +31,7 @@ export function AccountSection({ email, linkedAccounts }: Props) {
setLinkingDiscord(true);
setLinkError(null);
try {
- await authClient.linkSocial({ provider: "discord", callbackURL: "/settings?section=account" });
+ await authClient.linkSocial({ provider: "discord", callbackURL: "/settings/account" });
} catch {
setLinkError("Failed to connect Discord. Please try again.");
setLinkingDiscord(false);
diff --git a/app/components/user/settings/NotificationsSection.tsx b/app/components/user/settings/NotificationsSection.tsx
index ff0c7b5..a2bce43 100644
--- a/app/components/user/settings/NotificationsSection.tsx
+++ b/app/components/user/settings/NotificationsSection.tsx
@@ -1,4 +1,4 @@
-import { useFetcher } from "react-router";
+import { Link, useFetcher } from "react-router";
import { Switch } from "~/components/ui/switch";
import { Label } from "~/components/ui/label";
@@ -14,10 +14,9 @@ type Props = {
discordPingEnabled: boolean;
hasDiscordLinked: boolean;
draftEmailNotificationsEnabled: boolean;
- onNavigateToAccount: () => void;
};
-export function NotificationsSection({ discordPingEnabled, hasDiscordLinked, draftEmailNotificationsEnabled, onNavigateToAccount }: Props) {
+export function NotificationsSection({ discordPingEnabled, hasDiscordLinked, draftEmailNotificationsEnabled }: Props) {
const discordFetcher = useFetcher();
const emailFetcher = useFetcher();
@@ -87,13 +86,12 @@ export function NotificationsSection({ discordPingEnabled, hasDiscordLinked, dra
{!hasDiscordLinked ? (
Link your Discord account in{" "}
-
Account settings
- {" "}
+ {" "}
to enable Discord pings for league notifications.
) : (
diff --git a/app/components/user/settings/__tests__/AccountSection.test.tsx b/app/components/user/settings/__tests__/AccountSection.test.tsx
index b8f7366..287d753 100644
--- a/app/components/user/settings/__tests__/AccountSection.test.tsx
+++ b/app/components/user/settings/__tests__/AccountSection.test.tsx
@@ -55,7 +55,7 @@ describe("AccountSection", () => {
await waitFor(() => {
expect(mockLinkSocial).toHaveBeenCalledWith({
provider: "discord",
- callbackURL: "/settings?section=account",
+ callbackURL: "/settings/account",
});
});
});
diff --git a/app/routes.ts b/app/routes.ts
index 9e90920..c4300dc 100644
--- a/app/routes.ts
+++ b/app/routes.ts
@@ -69,7 +69,7 @@ export default [
route("forgot-password", "routes/forgot-password.tsx"),
route("reset-password", "routes/reset-password.tsx"),
route("onboarding", "routes/onboarding.tsx"),
- route("settings", "routes/settings.tsx"),
+ route("settings/:section?", "routes/settings.tsx"),
route("user-profile", "routes/user-profile-redirect.tsx"),
route("how-to-play", "routes/how-to-play.tsx"),
route("rules", "routes/rules.tsx"),
diff --git a/app/routes/__tests__/settings.test.ts b/app/routes/__tests__/settings.test.ts
new file mode 100644
index 0000000..660dcca
--- /dev/null
+++ b/app/routes/__tests__/settings.test.ts
@@ -0,0 +1,102 @@
+import { describe, it, expect, vi } from "vitest";
+
+vi.mock("~/lib/auth.server", () => ({
+ auth: { api: { getSession: vi.fn() } },
+}));
+vi.mock("~/lib/cloudinary.server", () => ({ deleteCloudinaryImageByUrl: vi.fn() }));
+vi.mock("~/lib/email.server", () => ({
+ sendEmail: vi.fn(),
+ wrapInEmailTemplate: vi.fn(),
+ emailParagraph: vi.fn(),
+ escapeHtml: vi.fn(),
+}));
+vi.mock("~/models/user", () => ({
+ findUserById: vi.fn(),
+ findUserByUsername: vi.fn(),
+ isUserInActiveDraft: vi.fn(),
+ updateUser: vi.fn(),
+ anonymizeUserAccount: vi.fn(),
+ USERNAME_RE: /^[a-zA-Z0-9_-]{3,30}$/,
+}));
+vi.mock("~/models/account", () => ({ findLinkedAccountsByUserId: vi.fn() }));
+vi.mock("~/models/team", () => ({ findTeamsByOwnerId: vi.fn(), removeTeamOwner: vi.fn() }));
+vi.mock("~/models/commissioner", () => ({ removeAllCommissionersByUserId: vi.fn() }));
+
+import { loader, shouldRevalidate } from "../settings";
+import { auth } from "~/lib/auth.server";
+
+type RevalidateArgs = Parameters[0];
+
+function revalidateArgs(overrides: Partial): RevalidateArgs {
+ return {
+ currentParams: {},
+ nextParams: {},
+ formMethod: undefined,
+ defaultShouldRevalidate: true,
+ ...overrides,
+ } as unknown as RevalidateArgs;
+}
+
+type LoaderArgs = Parameters[0];
+
+function loaderArgs(section?: string): LoaderArgs {
+ const path = section ? `/settings/${section}` : "/settings";
+ return {
+ params: { section },
+ request: new Request(`http://localhost${path}`),
+ context: {},
+ } as unknown as LoaderArgs;
+}
+
+describe("/settings loader section handling", () => {
+ it("redirects an unknown section back to /settings", async () => {
+ const result = (await loader(loaderArgs("bogus"))) as Response;
+ expect(result).toBeInstanceOf(Response);
+ expect(result.status).toBe(302);
+ expect(result.headers.get("Location")).toBe("/settings");
+ // The bad section short-circuits before we ever touch the session.
+ expect(auth.api.getSession).not.toHaveBeenCalled();
+ });
+
+ it("does not redirect a valid section before auth runs", async () => {
+ vi.mocked(auth.api.getSession).mockResolvedValue(null as never);
+ const result = (await loader(loaderArgs("account"))) as Response;
+ // Falls through to the auth check, which redirects to login (not /settings).
+ expect(result.headers.get("Location")).toBe("/login?redirectTo=/settings");
+ expect(auth.api.getSession).toHaveBeenCalled();
+ });
+});
+
+describe("/settings shouldRevalidate", () => {
+ it("skips revalidation when only the section param changes", () => {
+ expect(
+ shouldRevalidate(
+ revalidateArgs({ currentParams: { section: "profile" }, nextParams: { section: "account" } })
+ )
+ ).toBe(false);
+ });
+
+ it("revalidates after a mutation regardless of section", () => {
+ expect(
+ shouldRevalidate(
+ revalidateArgs({
+ currentParams: { section: "profile" },
+ nextParams: { section: "account" },
+ formMethod: "POST",
+ })
+ )
+ ).toBe(true);
+ });
+
+ it("defers to the default when the section is unchanged", () => {
+ expect(
+ shouldRevalidate(
+ revalidateArgs({
+ currentParams: { section: "account" },
+ nextParams: { section: "account" },
+ defaultShouldRevalidate: true,
+ })
+ )
+ ).toBe(true);
+ });
+});
diff --git a/app/routes/settings.tsx b/app/routes/settings.tsx
index a7d4455..99cebd1 100644
--- a/app/routes/settings.tsx
+++ b/app/routes/settings.tsx
@@ -1,5 +1,4 @@
-import { redirect, useSearchParams } from "react-router";
-import { useState } from "react";
+import { redirect, type ShouldRevalidateFunctionArgs } from "react-router";
import { Bell, Key, Lock, Shield, User } from "lucide-react";
import { auth } from "~/lib/auth.server";
import { findUserById, findUserByUsername, isUserInActiveDraft, updateUser, anonymizeUserAccount, USERNAME_RE } from "~/models/user";
@@ -57,7 +56,26 @@ export function meta(): Route.MetaDescriptors {
return [{ title: "Settings - Brackt" }];
}
+export function shouldRevalidate({
+ currentParams,
+ nextParams,
+ formMethod,
+ defaultShouldRevalidate,
+}: ShouldRevalidateFunctionArgs) {
+ // Mutations (profile/avatar/notification updates) must refresh loader data.
+ if (formMethod && formMethod !== "GET") return true;
+ // Switching sections only changes the path param, and the loader payload is the
+ // same for every section, so don't refetch user/draft status/linked accounts.
+ if (currentParams.section !== nextParams.section) return false;
+ return defaultShouldRevalidate;
+}
+
export async function loader(args: Route.LoaderArgs) {
+ const section = args.params.section;
+ if (section && !(VALID_SECTION_IDS as Set).has(section)) {
+ return redirect("/settings");
+ }
+
const session = await auth.api.getSession({ headers: args.request.headers });
if (!session) {
return redirect("/login?redirectTo=/settings");
@@ -213,18 +231,12 @@ export async function action(args: Route.ActionArgs): Promise).has(raw) ? (raw as SectionId) : "profile";
- const [activeSection, setActiveSection] = useState(initialSection);
- const [mobileView, setMobileView] = useState<"grid" | "section">("grid");
-
- const handleSectionChange = (id: string, mobile = false) => {
- setActiveSection(id as SectionId);
- if (mobile) setMobileView("section");
- };
+ const section = params.section;
+ const activeSection: SectionId =
+ section && (VALID_SECTION_IDS as Set).has(section) ? (section as SectionId) : "profile";
+ const mobileView: "grid" | "section" = section ? "section" : "grid";
const ad = actionData as ActionData | undefined;
const profileSuccess = ad?.intent === "update-profile" && "success" in ad;
@@ -246,20 +258,21 @@ export default function SettingsPage({ loaderData, actionData }: Route.Component
handleSectionChange(id, true)}
+ buildHref={(id) => `/settings/${id}`}
/>
)}
{mobileView === "section" && (
- setMobileView("grid")} />
+
)}
handleSectionChange(id)}
+ buildHref={(id) => `/settings/${id}`}
+ navLabel="Account settings"
/>
@@ -279,7 +292,6 @@ export default function SettingsPage({ loaderData, actionData }: Route.Component
discordPingEnabled={user.discordPingEnabled}
hasDiscordLinked={linkedAccounts.some((a) => a.providerId === "discord")}
draftEmailNotificationsEnabled={user.draftEmailNotificationsEnabled}
- onNavigateToAccount={() => handleSectionChange("account")}
/>
)}
{activeSection === "api" && }