Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions 03_implementation/ui/src/app/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,59 @@ export function tabIdFromHash(hash: string): string | null {
return TAB_IDS.includes(tabId as (typeof TAB_IDS)[number]) ? tabId : null;
}

/**
* Settings subtab URL-hash routing (W15-A17).
*
* Tabs with nested subtabs encode the selection in the trailing path
* segment, e.g. `#settings/general`, `#settings/mcp`. The first segment
* is owned by `tabIdFromHash`; the second segment is owned by the tab
* itself. This keeps deep links stable across reloads without coupling
* the subtab list to the global hash table.
*
* Note: the helper is intentionally generic over both the tab head and
* the allowed subtab keys so adjacent tabs (Voice / A18) can adopt the
* same routing shape without conflicting on `store.ts`. The Voice agent
* lane is expected to add `voiceSubtabFromHash` next to this helper.
*/
export function subtabFromHash<K extends string>(
hash: string,
expectedHead: string,
allowed: readonly K[],
): K | null {
const raw = hash.replace(/^#/, "").trim();
if (!raw) {
return null;
}
// Accept both `settings/general` and the legacy `settings.general` form;
// colon is reserved for dashboard mode routing.
const [head, sub] = raw.split(/[/.]/, 2);
if (head !== expectedHead || !sub) {
return null;
}
const decoded = decodeURIComponent(sub.toLowerCase());
return (allowed as readonly string[]).includes(decoded) ? (decoded as K) : null;
}

/**
* Convenience wrapper for the Settings tab. Keeps the rest of the app from
* having to import the generic helper and the subtab list separately.
*/
export const SETTINGS_SUBTAB_KEYS = [
"general",
"providers",
"agents",
"mcp",
"printers",
"environment",
"updates",
"about",
] as const;
export type SettingsSubtabKey = (typeof SETTINGS_SUBTAB_KEYS)[number];

export function settingsSubtabFromHash(hash: string): SettingsSubtabKey | null {
return subtabFromHash<SettingsSubtabKey>(hash, "settings", SETTINGS_SUBTAB_KEYS);
}

function initialActiveTabId(): string {
if (typeof window === "undefined") {
return "dashboard";
Expand Down
108 changes: 106 additions & 2 deletions 03_implementation/ui/src/components/settings/GeneralSubtab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,19 @@
* is internal state).
*/
import { useEffect, useState } from "react";
import { Languages, Moon, MonitorCog, Save } from "lucide-react";
import { Languages, Moon, MonitorCog, Palette, Save } from "lucide-react";
import { adapters } from "../../api/adapters";
import type { AppSettings, ThemeName } from "../../types/settings";
import {
applyPalette,
DEFAULT_PALETTE_ID,
isNamedPaletteId,
NAMED_PALETTES,
PALETTE_STORAGE_KEY,
readStoredPaletteId,
writeStoredPaletteId,
type NamedPaletteId,
} from "../../theme/palettes";
Comment on lines +36 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To correctly handle the live preview coordination and cleanup, you'll need to import useTheme and themeCssVars from the theme module. This allows the component to re-apply base variables when switching back to the 'default' palette or when the underlying light/dark mode changes.

Suggested change
import {
applyPalette,
DEFAULT_PALETTE_ID,
isNamedPaletteId,
NAMED_PALETTES,
PALETTE_STORAGE_KEY,
readStoredPaletteId,
writeStoredPaletteId,
type NamedPaletteId,
} from "../../theme/palettes";
import {
applyPalette,
DEFAULT_PALETTE_ID,
isNamedPaletteId,
NAMED_PALETTES,
PALETTE_STORAGE_KEY,
readStoredPaletteId,
writeStoredPaletteId,
useTheme,
themeCssVars,
type NamedPaletteId,
} from "../../theme";


type DashboardModePref = "simple" | "advanced" | "custom";
type UIModePref = "full" | "simple";
Expand Down Expand Up @@ -96,6 +106,12 @@ export function GeneralSubtab() {
const [dashboardMode, setDashboardMode] = useState<DashboardModePref>(() =>
readLocal(LS_DASHBOARD_MODE, ["simple", "advanced", "custom"] as const, "advanced"),
);
// Named palette is persisted to localStorage under the dedicated
// `h3d.theme.palette` key (W15-A17). Backend persistence is owned by
// A20 (`/api/settings/themes`); until that endpoint exists, the
// palette is preview-only and survives reload via local storage.
const [palette, setPalette] = useState<NamedPaletteId>(() => readStoredPaletteId());
const [savedPalette, setSavedPalette] = useState<NamedPaletteId>(palette);

const [savedLanguage, setSavedLanguage] = useState<LanguagePref>(language);
const [savedUiMode, setSavedUiMode] = useState<UIModePref>(uiMode);
Expand Down Expand Up @@ -125,11 +141,20 @@ export function GeneralSubtab() {
};
}, []);

// Live preview: every palette change re-applies the CSS variables.
// We intentionally do this BEFORE save — clicking a swatch should
// preview immediately, like VS Code's theme picker, with the persist
// step happening on Save (so revert == reload, or pick "default").
useEffect(() => {
applyPalette(palette);
}, [palette]);
Comment on lines +148 to +150

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current live preview implementation has three issues:

  1. Broken 'Default' Preview: Since applyPalette(DEFAULT_PALETTE_ID) is a no-op for CSS variables, clicking 'Default' after a named palette won't actually revert the colors in the UI.
  2. Theme Toggle Desync: If the user toggles light/dark mode while previewing a named palette, the preview is lost because ThemeProvider re-applies the saved palette.
  3. Leaky Preview: Navigating away from the Settings tab without saving leaves the previewed palette active on the entire application until a page reload.

Using useTheme to coordinate with the base theme and adding a cleanup effect solves these issues.

  const { resolvedTheme } = useTheme();

  // Live preview: every palette change re-applies the CSS variables.
  // We coordinate with the base theme to ensure 'default' correctly reverts
  // and that the preview survives light/dark mode toggles.
  useEffect(() => {
    if (palette === DEFAULT_PALETTE_ID) {
      const vars = themeCssVars(resolvedTheme);
      const root = document.documentElement;
      for (const [name, value] of Object.entries(vars)) {
        root.style.setProperty(name, value);
      }
      root.dataset.h3dPalette = DEFAULT_PALETTE_ID;
    } else {
      applyPalette(palette);
    }
  }, [palette, resolvedTheme]);

  // Cleanup: restore the actually persisted palette when navigating away
  // to prevent the preview from 'leaking' to the rest of the app.
  useEffect(() => {
    return () => {
      applyPalette(readStoredPaletteId());
    };
  }, []);

Comment on lines +148 to +150

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Default palette does not actually revert live preview.

At Line 149, applyPalette("default") is a no-op, so previously stamped non-default CSS vars remain active. That means users can’t truly preview/reset to baseline by selecting default without a reload/theme reapply.

Proposed fix
+import { themeCssVars } from "../../theme/tokens";
...
  useEffect(() => {
-    applyPalette(palette);
+    if (palette === DEFAULT_PALETTE_ID) {
+      const resolved = document.documentElement.dataset.h3dTheme === "light" ? "light" : "dark";
+      for (const [k, v] of Object.entries(themeCssVars(resolved))) {
+        document.documentElement.style.setProperty(k, v);
+      }
+      document.documentElement.dataset.h3dPalette = DEFAULT_PALETTE_ID;
+      return;
+    }
+    applyPalette(palette);
  }, [palette]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@03_implementation/ui/src/components/settings/GeneralSubtab.tsx` around lines
148 - 150, Summary: Selecting the "default" palette currently is a no-op because
applyPalette("default") doesn't undo previously set CSS variables; implement a
proper reset path so the live preview truly reverts. Fix: update applyPalette
(and its consumer in GeneralSubtab.tsx where useEffect watches palette) so that
when palette === "default" it removes or clears any palette-specific CSS custom
properties (or reapplies the baseline/theme CSS) instead of doing nothing;
alternatively add a resetPalette/clearPalette helper called from the useEffect
when palette === "default". Reference symbols: applyPalette, palette,
GeneralSubtab.tsx useEffect (the effect block that currently calls
applyPalette(palette)).


const dirty =
(serverTheme !== null && theme !== serverTheme) ||
language !== savedLanguage ||
uiMode !== savedUiMode ||
dashboardMode !== savedDashboardMode;
dashboardMode !== savedDashboardMode ||
palette !== savedPalette;

const handleSave = async () => {
if (!dirty || busy) return;
Expand All @@ -145,14 +170,20 @@ export function GeneralSubtab() {
writeLocal(LS_LANGUAGE, language);
writeLocal(LS_UI_MODE, uiMode);
writeLocal(LS_DASHBOARD_MODE, dashboardMode);
// The named palette has its own dedicated key (PALETTE_STORAGE_KEY)
// because the W8-3 ThemeProvider already owns `h3d.theme` for
// light/dark mode — keeping these orthogonal is the contract.
writeStoredPaletteId(palette);
setSavedLanguage(language);
setSavedUiMode(uiMode);
setSavedDashboardMode(dashboardMode);
setSavedPalette(palette);
await adapters.emitProofEvent("settings.general.saved", {
theme,
language,
uiMode,
dashboardMode,
palette,
});
setStatus({ tone: "ok", text: "Preferences saved." });
} catch (error) {
Expand Down Expand Up @@ -211,6 +242,79 @@ export function GeneralSubtab() {
</div>
</fieldset>

<fieldset
className="flex flex-col gap-2 rounded border border-border bg-surface2/30 p-3"
data-testid="settings-general-palettes"
>
<legend className="px-1 text-[10px] uppercase tracking-wide text-muted">
<Palette size={11} className="mr-1 inline" /> Theme Palette
</legend>
<p className="text-[10px] text-muted">
Switches the live CSS variables. Saved to your browser
(<code className="font-mono">{PALETTE_STORAGE_KEY}</code>); preview-only until the
backend <code className="font-mono">/api/settings/themes</code> endpoint ships.
</p>
<div
role="radiogroup"
aria-label="Theme palette"
className="grid grid-cols-1 gap-2 sm:grid-cols-2 lg:grid-cols-3"
>
{NAMED_PALETTES.map((opt) => {
const selected = palette === opt.id;
return (
<label
key={opt.id}
data-testid={`settings-general-palette-${opt.id}`}
className={[
"flex cursor-pointer flex-col gap-1 rounded border px-2 py-1.5 text-[11px]",
selected
? "border-accent-cyan bg-surface2 text-fg"
: "border-border text-muted hover:bg-surface2/60 hover:text-fg",
].join(" ")}
>
<input
type="radio"
name="general-palette"
value={opt.id}
checked={selected}
onChange={() => {
if (isNamedPaletteId(opt.id)) setPalette(opt.id);
}}
className="sr-only"
/>
<div className="flex items-center justify-between gap-2">
<span className="font-medium text-fg">{opt.label}</span>
<span className="flex items-center gap-1">
<span
aria-hidden="true"
data-testid={`settings-general-palette-${opt.id}-swatch-bg`}
className="inline-block h-3 w-3 rounded-sm border border-border"
style={{ backgroundColor: opt.swatches.background }}
/>
<span
aria-hidden="true"
data-testid={`settings-general-palette-${opt.id}-swatch-primary`}
className="inline-block h-3 w-3 rounded-sm border border-border"
style={{ backgroundColor: opt.swatches.primary }}
/>
</span>
</div>
<span className="text-[10px] text-muted">{opt.blurb}</span>
<span className="font-mono text-[10px] text-muted">{opt.id}</span>
</label>
);
})}
</div>
{palette !== DEFAULT_PALETTE_ID && (
<p
className="text-[10px] text-muted"
data-testid="settings-general-palette-reset-hint"
>
Pick <strong>{NAMED_PALETTES[0].label}</strong> to revert to the shipped baseline.
</p>
)}
</fieldset>

<div className="grid grid-cols-1 gap-3 sm:grid-cols-2">
<fieldset className="flex flex-col gap-2 rounded border border-border bg-surface2/30 p-3">
<legend className="px-1 text-[10px] uppercase tracking-wide text-muted">
Expand Down
113 changes: 97 additions & 16 deletions 03_implementation/ui/src/components/settings/SettingsPage.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
/**
* Settings landing page backed by the local GUI API.
*
* Hosts the four canonical subtabs:
* Hosts the eight canonical subtabs:
* - General · theme, language, defaults
* - Providers · LLM endpoint config (read-only `llm_policy.yaml` view)
* - Agents · agent policy preview
* - MCP · active MCP file locks
* - Printers · 12-printer fleet from `printers.toml` + optional health
* - Environment · env-var presence with values redacted to [set]/[not set]
* - Updates · update center, version + rollback
* - About · version, license, links to GitHub + docs
*
* Subtab routing (W15-A17): the URL hash is authoritative — visiting
* `#settings/<sub>` selects the matching subtab; clicking a subtab
* replaces the hash with `#settings/<sub>`. We use `replaceState` (not
* `pushState`) so navigating subtabs doesn't pollute browser history.
* The provider chain itself and the service-health endpoint are owned by
* other tracks; this page only consumes their public shapes (and gracefully
* degrades when they're not yet shipped).
*
* Subtab routing is internal `useState` — we MUST NOT introduce
* react-router; the universal shell uses the TABS pattern only.
* Subtab routing follows the same `#<tab>/<sub>` pattern AppRegistry uses
* (`#apps/<id>`). No react-router; the universal shell uses TABS only.
*/
import { useState } from "react";
import { useEffect, useState } from "react";
import {
Bot,
Cog,
Expand All @@ -29,7 +37,12 @@ import {
} from "lucide-react";
import { Panel } from "../layout/Panel";
import { ResizablePane } from "../layout/ResizablePane";
import { useStore } from "../../app/store";
import {
SETTINGS_SUBTAB_KEYS,
settingsSubtabFromHash,
useStore,
type SettingsSubtabKey,
} from "../../app/store";
import { GeneralSubtab } from "./GeneralSubtab";
import { ProvidersSubtab } from "./ProvidersSubtab";
import { PrintersSubtab } from "./PrintersSubtab";
Expand All @@ -39,15 +52,10 @@ import { AgentConfigSection } from "./AgentConfigSection";
import { UpdateCenterSubtab } from "./UpdateCenterSubtab";
import { McpSubtab } from "./McpSubtab";

type SubtabKey =
| "general"
| "providers"
| "agents"
| "mcp"
| "printers"
| "environment"
| "about"
| "updates";
type SubtabKey = SettingsSubtabKey;

const SETTINGS_HASH_PREFIX = "settings";
const DEFAULT_SUBTAB: SubtabKey = "general";

const SUBTABS: { key: SubtabKey; label: string; Icon: typeof Cpu; description: string }[] = [
{ key: "general", label: "General", Icon: MonitorCog, description: "Theme, language, defaults" },
Expand All @@ -60,9 +68,82 @@ const SUBTABS: { key: SubtabKey; label: string; Icon: typeof Cpu; description: s
{ key: "about", label: "About", Icon: Info, description: "Version + links" },
];

// Defensive: enforce SUBTABS and SETTINGS_SUBTAB_KEYS stay in sync at
// load time so reorderings here trip a console warning in dev instead of
// silently breaking the URL contract. `import.meta.env.DEV` is the Vite
// compile-time flag; we guard the access so non-Vite consumers (Jest)
// don't choke on the missing import.meta.
const __isDev: boolean =
typeof import.meta !== "undefined" &&
(import.meta as { env?: { DEV?: boolean } }).env?.DEV === true;
if (__isDev && SUBTABS.length !== SETTINGS_SUBTAB_KEYS.length) {
// eslint-disable-next-line no-console
console.warn(
`[SettingsPage] SUBTABS (${SUBTABS.length}) and SETTINGS_SUBTAB_KEYS (${SETTINGS_SUBTAB_KEYS.length}) length mismatch`,
);
}

function readInitialSubtab(): SubtabKey {
if (typeof window === "undefined") return DEFAULT_SUBTAB;
return settingsSubtabFromHash(window.location.hash) ?? DEFAULT_SUBTAB;
}

export function SettingsPage() {
const setActiveTabId = useStore((state) => state.setActiveTabId);
const [active, setActive] = useState<SubtabKey>("general");
const [active, setActive] = useState<SubtabKey>(readInitialSubtab);

// Sync the subtab selection FROM the URL hash so deep links and the
// back/forward buttons select the correct subtab. The interval is a
// safety-net for environments that swallow hashchange (Playwright on
// some Firefox builds); 500ms matches App.tsx's tab-level cadence.
useEffect(() => {
const sync = () => {
const next = settingsSubtabFromHash(window.location.hash);
if (next && next !== active) setActive(next);
};
sync();
window.addEventListener("hashchange", sync);
window.addEventListener("popstate", sync);
const timer = window.setInterval(sync, 500);
return () => {
window.removeEventListener("hashchange", sync);
window.removeEventListener("popstate", sync);
window.clearInterval(timer);
};
}, [active]);

// Sync the URL hash TO the subtab selection. We use replaceState so
// navigating subtabs doesn't bloat browser history — mirrors the
// tab-level logic in App.tsx.
useEffect(() => {
if (typeof window === "undefined") return;
const desired = `#${SETTINGS_HASH_PREFIX}/${active}`;
if (window.location.hash === desired) return;
// Only rewrite when the head segment is "settings" — never clobber
// a hash that points at a different tab (the user may have just
// clicked the sidebar).
const head = window.location.hash.replace(/^#/, "").split(/[/:.]/, 1)[0];
if (head && head !== SETTINGS_HASH_PREFIX) return;
window.history.replaceState(null, "", desired);
}, [active]);

const handleSelect = (key: SubtabKey) => {
setActive(key);
if (typeof window !== "undefined") {
const next = `#${SETTINGS_HASH_PREFIX}/${key}`;
if (window.location.hash !== next) {
window.history.replaceState(null, "", next);
// Fire hashchange so AgentChatMirror and any other listeners
// see the navigation; replaceState alone does not emit it.
try {
window.dispatchEvent(new HashChangeEvent("hashchange"));
} catch {
/* JSDOM occasionally fails to construct HashChangeEvent */
}
}
}
};

const meta = SUBTABS.find((s) => s.key === active) ?? SUBTABS[0];

return (
Expand Down Expand Up @@ -105,7 +186,7 @@ export function SettingsPage() {
aria-controls={`settings-panel-${s.key}`}
id={`settings-tab-${s.key}`}
data-testid={`settings-subtab-${s.key}`}
onClick={() => setActive(s.key)}
onClick={() => handleSelect(s.key)}
className={[
"w-full flex items-center gap-2 px-2 py-1.5 rounded border-l-2 text-left",
selected
Expand Down
Loading
Loading