From 9c321538791a9760b7559f0f8269aeab7d9d70ed Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 9 Apr 2026 19:16:22 -0700 Subject: [PATCH] feat(FN-1440): promote Authentication in Settings sidebar with globe icon - Add Authentication section to Settings sidebar with globe icon - Update SettingsModal to display Authentication link in sidebar - Add corresponding tests for Authentication section visibility - Update mobile settings tests to reflect new sidebar structure --- .fusion/memory.md | 1 + .../app/components/SettingsModal.tsx | 39 +++--- .../app/components/__tests__/App.test.tsx | 11 +- .../__tests__/SettingsModal.test.tsx | 116 +++++++++++------- .../__tests__/settings-mobile.test.tsx | 10 +- 5 files changed, 116 insertions(+), 61 deletions(-) diff --git a/.fusion/memory.md b/.fusion/memory.md index 174167941..ab878d80a 100644 --- a/.fusion/memory.md +++ b/.fusion/memory.md @@ -97,6 +97,7 @@ The plugin system is built on three layers: - QuickEntryBox control test IDs are reused in `ListView` integration tests; when control layout changes (for example nested menu → inline buttons), update both `QuickEntryBox.test.tsx` and `ListView.test.tsx` together to avoid cascading failures. - When `InlineCreateCard` layout changes, also check `Column.test.tsx` and `board-mobile.test.tsx` for references to moved/removed test IDs like `inline-create-description-actions`. - `mission-store.test.ts` has a flaky test (`getMissionHealth computes mission metrics and latest error context`) that fails intermittently when timestamps collide in the same millisecond — this is pre-existing and not related to dashboard changes. +- **SettingsModal sidebar reordering**: When reordering sections in `SETTINGS_SECTIONS`, update all tests that assume a specific section is the default. Tests using `screen.getByText("SectionName")` may fail with "multiple elements found" when the section heading also appears in the content area alongside the sidebar item. Use `screen.getAllByText("SectionName")[0]` or navigate to the section explicitly before accessing its fields. - When adding light-theme overrides for CSS components that already use `var(--*)` tokens, most selectors inherit correctly from the light-theme root variable redefinitions. Only add explicit `[data-theme="light"]` overrides where fine-tuning is needed (e.g., slightly different opacity values, subtle box-shadows for contrast). - `--surface-hover` is used but never defined as a CSS custom property in the root or light theme blocks — it resolves to invalid/empty. Components using `var(--surface-hover)` (like `.github-import-tab:hover`) get no background. Either define it in the theme roots or use fallbacks like `var(--surface-hover, rgba(0,0,0,0.03))`. diff --git a/packages/dashboard/app/components/SettingsModal.tsx b/packages/dashboard/app/components/SettingsModal.tsx index 1497d8a69..7197794f3 100644 --- a/packages/dashboard/app/components/SettingsModal.tsx +++ b/packages/dashboard/app/components/SettingsModal.tsx @@ -37,22 +37,30 @@ import { applyPresetToSelection, generateUniquePresetId } from "../utils/modelPr * - notifications: ntfy.sh notification settings (global) * - authentication: OAuth provider status, login/logout (independent) */ -const SETTINGS_SECTIONS = [ - { id: "general", label: "General", scope: "project" as const }, - { id: "models", label: "Models", scope: "project" as const }, - { id: "appearance", label: "Appearance", scope: "global" as const }, - { id: "scheduling", label: "Scheduling", scope: "project" as const }, - { id: "worktrees", label: "Worktrees", scope: "project" as const }, - { id: "commands", label: "Commands", scope: "project" as const }, - { id: "merge", label: "Merge", scope: "project" as const }, - { id: "memory", label: "Memory", scope: "project" as const }, - { id: "backups", label: "Backups", scope: "project" as const }, - { id: "notifications", label: "Notifications", scope: "global" as const }, - { id: "plugins", label: "Plugins", scope: "project" as const }, - { id: "authentication", label: "Authentication", scope: undefined }, -] as const; +/** Section entry type with optional icon */ +type SettingsSection = { + id: string; + label: string; + scope: "global" | "project" | undefined; + icon?: typeof Globe; +}; -export type SectionId = (typeof SETTINGS_SECTIONS)[number]["id"]; +const SETTINGS_SECTIONS: SettingsSection[] = [ + { id: "authentication", label: "Authentication", scope: undefined, icon: Globe }, + { id: "appearance", label: "Appearance", scope: "global" }, + { id: "notifications", label: "Notifications", scope: "global" }, + { id: "general", label: "General", scope: "project" }, + { id: "models", label: "Models", scope: "project" }, + { id: "scheduling", label: "Scheduling", scope: "project" }, + { id: "worktrees", label: "Worktrees", scope: "project" }, + { id: "commands", label: "Commands", scope: "project" }, + { id: "merge", label: "Merge", scope: "project" }, + { id: "memory", label: "Memory", scope: "project" }, + { id: "backups", label: "Backups", scope: "project" }, + { id: "plugins", label: "Plugins", scope: "project" }, +]; + +export type SectionId = SettingsSection["id"]; interface SettingsModalProps { onClose: () => void; @@ -2006,6 +2014,7 @@ export function SettingsModal({ > {section.scope === "global" && } {section.scope === "project" && } + {section.icon && section.scope !== "global" && section.scope !== "project" && } {section.label} ))} diff --git a/packages/dashboard/app/components/__tests__/App.test.tsx b/packages/dashboard/app/components/__tests__/App.test.tsx index 5d0e70745..058ae1899 100644 --- a/packages/dashboard/app/components/__tests__/App.test.tsx +++ b/packages/dashboard/app/components/__tests__/App.test.tsx @@ -650,7 +650,7 @@ describe("App auto-open Settings on unauthenticated", () => { expect(screen.queryByText("Set Up AI Provider")).toBeNull(); }); - it("re-opening Settings via gear icon defaults to General tab after closing onboarding", async () => { + it("re-opening Settings via gear icon defaults to Authentication tab after closing onboarding", async () => { // fetchGlobalSettings returns {} by default → onboarding opens render(); @@ -672,8 +672,15 @@ describe("App auto-open Settings on unauthenticated", () => { const settingsButton = screen.getByTitle("Settings"); fireEvent.click(settingsButton); - // Now it should open to General section (default) + // Settings should open with Authentication section (first/default) await waitFor(() => expect(fetchSettings).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); + + // Authentication section content should be visible (providers listed) + expect(screen.getByText("Anthropic")).toBeTruthy(); + + // Click on General to verify General section has Task Prefix + fireEvent.click(screen.getAllByText("General")[0]); expect(screen.getByLabelText("Task Prefix")).toBeTruthy(); }); }); diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.test.tsx index e8bb62454..913a8fea4 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.test.tsx @@ -72,10 +72,13 @@ describe("SettingsModal", () => { expect(screen.getAllByText("Merge").length).toBeGreaterThanOrEqual(1); }); - it("shows General fields by default", async () => { + it("shows General fields when General section is selected", async () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + // Click on General (Authentication is now default, so we need to navigate) + fireEvent.click(screen.getAllByText("General")[0]); + expect(screen.getByLabelText("Task Prefix")).toBeTruthy(); // Fields from other sections should not be visible expect(screen.queryByLabelText("Max Concurrent Tasks")).toBeNull(); @@ -102,7 +105,8 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - // General (default) + // General + fireEvent.click(screen.getAllByText("General")[0]); expect(screen.getByLabelText("Task Prefix")).toBeTruthy(); // Scheduling @@ -159,6 +163,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + fireEvent.click(screen.getAllByText("General")[0]); const input = screen.getByLabelText("Task Prefix") as HTMLInputElement; fireEvent.change(input, { target: { value: "PROJ" } }); expect(input.value).toBe("PROJ"); @@ -174,6 +179,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + fireEvent.click(screen.getAllByText("General")[0]); const input = screen.getByLabelText("Task Prefix") as HTMLInputElement; fireEvent.change(input, { target: { value: "" } }); @@ -188,6 +194,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + fireEvent.click(screen.getAllByText("General")[0]); const input = screen.getByLabelText("Task Prefix") as HTMLInputElement; fireEvent.change(input, { target: { value: "bad" } }); @@ -198,6 +205,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + fireEvent.click(screen.getAllByText("General")[0]); const input = screen.getByLabelText("Task Prefix") as HTMLInputElement; fireEvent.change(input, { target: { value: "bad" } }); @@ -384,6 +392,9 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + // Click on General to navigate to General section + fireEvent.click(screen.getAllByText("General")[0]); + // General section is project-scoped — change a project setting const input = screen.getByLabelText("Task Prefix") as HTMLInputElement; fireEvent.change(input, { target: { value: "TEST" } }); @@ -743,11 +754,25 @@ describe("SettingsModal", () => { expect(screen.getAllByText("Authentication").length).toBeGreaterThanOrEqual(1); }); + it("Authentication nav item has globe icon in sidebar", async () => { + const { container } = render(); + await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + + // Find the Authentication nav item in the sidebar + const authNavItem = container.querySelector(".settings-nav-item"); + expect(authNavItem).toBeTruthy(); + expect(authNavItem?.textContent?.trim()).toBe("Authentication"); + + // Check that it has a globe icon (Globe icon component renders as SVG with specific aria-label) + const globeIcon = authNavItem?.querySelector('[aria-label="Global setting"]'); + expect(globeIcon).toBeTruthy(); + }); + it("shows provider auth status when Authentication section is selected", async () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); expect(screen.getByText("Anthropic")).toBeTruthy(); @@ -762,7 +787,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); expect(screen.getByText("✓ Authenticated")).toBeTruthy(); @@ -776,7 +801,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); fireEvent.click(screen.getByText("Login")); @@ -795,7 +820,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); fireEvent.click(screen.getByText("Logout")); @@ -817,7 +842,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const authBadge = screen.getByTestId("auth-status-anthropic"); @@ -833,7 +858,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const providerRow = screen.getByText("Anthropic").closest(".auth-provider-row"); @@ -933,7 +958,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); fireEvent.click(screen.getByText("Login")); @@ -956,14 +981,15 @@ describe("SettingsModal", () => { expect(screen.queryByLabelText("Task Prefix")).toBeNull(); }); - it("defaults to General section when no initialSection is passed", async () => { + it("defaults to Authentication section when no initialSection is passed", async () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); - // General content should be visible - expect(screen.getByLabelText("Task Prefix")).toBeTruthy(); - // Authentication content should NOT be visible - expect(screen.queryByText("✗ Not authenticated")).toBeNull(); + // Authentication content should be visible (it's the default section now) + expect(screen.getByText("Anthropic")).toBeTruthy(); + // General content should NOT be visible + expect(screen.queryByLabelText("Task Prefix")).toBeNull(); }); it("shows sign-in hint when no providers are authenticated", async () => { @@ -977,7 +1003,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); expect(screen.getByText("Sign in to at least one provider to get started.")).toBeTruthy(); @@ -997,7 +1023,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); expect(screen.queryByText("Sign in to at least one provider to get started.")).toBeNull(); @@ -1018,7 +1044,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const input = screen.getByPlaceholderText("Enter API key") as HTMLInputElement; @@ -1044,7 +1070,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); // The Clear button should be inside the auth-apikey-section @@ -1074,7 +1100,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const input = screen.getByPlaceholderText("Enter API key"); @@ -1097,7 +1123,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const apiKeySection = container.querySelector(".auth-apikey-section")!; @@ -1124,7 +1150,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const apiKeySection = container.querySelector(".auth-apikey-section")!; @@ -1147,7 +1173,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const input = screen.getByPlaceholderText("Enter API key"); @@ -1177,7 +1203,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); // Initially should show "Clear" button (authenticated) @@ -1210,7 +1236,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const apiKeySection = container.querySelector(".auth-apikey-section")!; @@ -1231,7 +1257,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); // OAuth provider shows Login @@ -1250,7 +1276,7 @@ describe("SettingsModal", () => { render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const input = screen.getByPlaceholderText("Enter API key") as HTMLInputElement; @@ -1269,29 +1295,30 @@ describe("SettingsModal", () => { expect(layout!.querySelector(".settings-content")).toBeTruthy(); }); - it("has .settings-sidebar with 11 .settings-nav-item buttons for all sections", async () => { + it("has .settings-sidebar with 12 .settings-nav-item buttons for all sections", async () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); const sidebar = container.querySelector(".settings-sidebar"); expect(sidebar).toBeTruthy(); const navItems = sidebar!.querySelectorAll(".settings-nav-item"); - expect(navItems.length).toBe(11); + expect(navItems.length).toBe(12); // Labels include scope icons (Globe for global, Folder for project) - const labels = Array.from(navItems).map((el) => el.textContent); + const labels = Array.from(navItems).map((el) => el.textContent?.trim()); expect(labels).toEqual([ + "Authentication", + "Appearance", + "Notifications", "General", "Models", - "Appearance", "Scheduling", "Worktrees", "Commands", "Merge", "Memory", "Backups", - "Notifications", - "Authentication", + "Plugins", ]); }); @@ -1311,16 +1338,16 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - // Default active section is General + // Default active section is Authentication (first in sidebar order) const activeItems = container.querySelectorAll(".settings-nav-item.active"); expect(activeItems.length).toBe(1); - expect(activeItems[0].textContent).toBe("General"); + expect(activeItems[0].textContent?.trim()).toBe("Authentication"); - // Switch to Scheduling - fireEvent.click(screen.getByText("Scheduling")); + // Switch to General + fireEvent.click(screen.getAllByText("General")[0]); const newActive = container.querySelectorAll(".settings-nav-item.active"); expect(newActive.length).toBe(1); - expect(newActive[0].textContent).toBe("Scheduling"); + expect(newActive[0].textContent?.trim()).toBe("General"); }); it("auth provider rows contain .auth-provider-info and action button", async () => { @@ -1334,7 +1361,7 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - fireEvent.click(screen.getByText("Authentication")); + fireEvent.click(screen.getAllByText("Authentication")[0]); await waitFor(() => expect(fetchAuthStatus).toHaveBeenCalled()); const rows = container.querySelectorAll(".auth-provider-row"); @@ -2017,18 +2044,23 @@ describe("SettingsModal", () => { const { container } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); - // General section is project-scoped → should show project banner - expect(container.querySelector(".settings-scope-project")).toBeTruthy(); + // Authentication is first (no scope banner) → should show no scope banner + expect(container.querySelector(".settings-scope-project")).toBeNull(); expect(container.querySelector(".settings-scope-global")).toBeNull(); // Switch to Appearance → should show global banner - fireEvent.click(screen.getByText("Appearance")); + fireEvent.click(screen.getAllByText("Appearance")[0]); expect(container.querySelector(".settings-scope-global")).toBeTruthy(); expect(container.querySelector(".settings-scope-global")?.textContent).toContain("Fusion"); expect(container.querySelector(".settings-scope-project")).toBeNull(); + // Switch to General → should show project banner + fireEvent.click(screen.getAllByText("General")[0]); + expect(container.querySelector(".settings-scope-project")).toBeTruthy(); + expect(container.querySelector(".settings-scope-global")).toBeNull(); + // Switch to Models → should show project banner - fireEvent.click(screen.getByText("Models")); + fireEvent.click(screen.getAllByText("Models")[0]); expect(container.querySelector(".settings-scope-project")).toBeTruthy(); expect(container.querySelector(".settings-scope-global")).toBeNull(); }); diff --git a/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx b/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx index 7a1b017b1..bc0d05b6c 100644 --- a/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx +++ b/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx @@ -89,19 +89,25 @@ describe("SettingsModal mobile adaptations", () => { }); it("renders form controls inside settings-content for 16px mobile targeting", async () => { - const { container } = render(); + const user = userEvent.setup(); + const { container, getByText } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + // Authentication is first by default, so click General to see form controls + await user.click(getByText("General")); + const controls = container.querySelectorAll(".settings-content input, .settings-content select, .settings-content textarea"); expect(controls.length).toBeGreaterThan(0); }); it("shows scope indicators and updates scope banner across sections", async () => { const user = userEvent.setup(); - const { container, getByText } = render(); + const { container, getByText, getAllByText } = render(); await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + // Authentication is first with no scope banner by default - click General to see project scope expect(container.querySelectorAll(".settings-scope-icon").length).toBeGreaterThan(0); + await user.click(getAllByText("General")[0]); expect(getByText("These settings only affect this project.")).toBeTruthy(); await user.click(getByText("Appearance"));