feat: implement navbar redesign

[kantord]

This includes the navbar redesign from this PR: #1688 However, as discussed on the meeting, I undid the side-drawer design for the Assistant feature and converted it back to an independent page.

recording-2026-03-09_16-08-21.mp4

also adds a simple way to test windows/mac design differences without switching computers:

recording-2026-03-09_16-36-39.mp4

[kantord]

@claude do a deep review please

[claude]

Claude finished @kantord's task in 4m 9s —— View job

---

Deep Review

• Read CLAUDE.md and project conventions
• Review new files (nav-icon-button.tsx, nav-separator.tsx, os-design.ts)
• Review modified navbar components (index.tsx, container.tsx, window-controls.tsx)
• Review settings/secrets changes (secrets-tab.tsx, settings-tabs.tsx)
• Review e2e test changes
• Review route and renderer changes
• Synthesize findings and post review

---

Overall this is a clean, well-scoped redesign. The component decomposition is good, the new os-design.ts abstraction is a nice separation of concerns, and the consolidation of the Secrets page into a Settings tab is a good UX choice. A few issues worth addressing:

---

🔴 Issues

1. window.OsDesign exposed unconditionally in production
renderer/src/common/lib/os-design.ts L57

window.OsDesign = osDesignDevtools

osDesignDevtools is unconditionally attached to window on every import — including in production builds. The setMac() / setWindows() methods write to sessionStorage and call location.reload(), giving any script on the page the ability to manipulate the window chrome. While Electron's context isolation limits external script access, it still adds unnecessary attack surface and ships internal tooling to users.

Suggested fix: gate it behind import.meta.env.DEV:

if (import.meta.env.DEV) {
  window.OsDesign = osDesignDevtools
}

Fix this →

---

2. Async state update without unmount cleanup in WindowControls
renderer/src/common/components/layout/top-nav/window-controls.tsx L9–12

useEffect(() => {
  window.electronAPI.windowControls.isMaximized().then(setIsMaximized)
}, [])

If the component unmounts before the promise resolves, setIsMaximized is called on a stale component. The idiomatic fix is a cancelled/ignored flag:

useEffect(() => {
  let cancelled = false
  window.electronAPI.windowControls.isMaximized().then((v) => {
    if (!cancelled) setIsMaximized(v)
  })
  return () => { cancelled = true }
}, [])

Fix this →

---

🟡 Minor Issues

3. getOsDesignVariant() called twice per render in TopNav
renderer/src/common/components/layout/top-nav/index.tsx L147, L160

The function reads sessionStorage on every call:

{getOsDesignVariant() === 'mac' && <NavSeparator />}
...
{getOsDesignVariant() !== 'mac' && <NavSeparator />}

These two calls will always return the same value in a single render. Assign once:

const osVariant = getOsDesignVariant()

Fix this →

---

4. container.tsx uses twMerge directly instead of cn
renderer/src/common/components/layout/top-nav/container.tsx L2

The rest of the codebase uses the cn utility (which wraps twMerge + clsx). This should be consistent:

// Before
import { twMerge } from 'tailwind-merge'
// After
import { cn } from '@/common/lib/utils'

Fix this →

---

5. dark:hover:bg-white/10 is redundant in NavIconButton
renderer/src/common/components/layout/top-nav/nav-icon-button.tsx L20

`rounded-full text-white/90 hover:bg-white/10 hover:text-white
dark:hover:bg-white/10`

dark:hover:bg-white/10 is identical to hover:bg-white/10. Since the navbar always renders against the dark nav background (defined in CSS tokens), this dark variant adds nothing. Remove to keep the class list clean.

---

6. Three-column grid with only two children
renderer/src/common/components/layout/top-nav/container.tsx L18

'grid grid-cols-[auto_3fr_auto]'

TopNav only renders 2 children into this grid, so the third auto column collapses to 0. If a third column (e.g., a logo or title) is not planned, simplify to grid-cols-[auto_1fr] to make the intent clear and avoid confusion.

---

7. Pre-existing typo in import
renderer/src/common/components/settings/tabs/secrets-tab.tsx L7

import { useMutationCerateSecret } from '@/features/secrets/hooks/use-mutation-create-secret'

Cerate → Create. This was in the original file and is carried through, but the file is being substantially rewritten in this PR — a good chance to clean it up. (Would require renaming the hook file too.)

---

✅ Strengths

• os-design.ts: Clean separation of UI design variant from behavioural platform logic. Using sessionStorage (rather than localStorage) for overrides is correct — overrides are session-scoped and don't persist across app restarts.
• NavIconButton / NavSeparator: Well-decomposed, focused components with proper use of ComponentPropsWithoutRef<typeof Button> for type safety.
• settings.tsx: Thoughtful use of prefetchQuery instead of ensureQueryData — the comment correctly explains that a secrets API failure shouldn't block the Settings route when the user is on a different tab.
• Route consolidation: Removing the standalone /secrets route in favour of /settings?tab=secrets is a clean UX simplification. The route tree gen file and the e2e tests are all updated consistently.
• link-view-transition.tsx: Removing /playground and /secrets from the ordered routes is correct given that playground is now a panel and secrets is now a tab.
• Test coverage: Both the unit tests and e2e tests are properly updated to reflect the new navigation patterns. The accessibility checks in secrets-tab.test.tsx (sr-only descriptions, aria-describedby) are a nice touch.
• Tailwind v4 ! important modifier: Used correctly in the toast styles (L103–105 of index.tsx).

---

[Copilot]

Pull request overview

This PR continues the navbar redesign by restructuring top-nav actions (moving some links to icon buttons), introducing an overridable “OS design” variant for platform-specific chrome/layout, and integrating “Secrets” into Settings rather than a standalone route.

Changes:

• Move Secrets from a dedicated /secrets route into a new Settings → Secrets tab (including route-tree updates and query prefetching).
• Redesign the top navigation to keep primary links left-aligned and move Settings/Assistant/Help into right-side icon controls with separators.
• Add an os-design utility to override mac/windows visual variants (sessionStorage + DevTools helpers) and apply it to nav layout/window controls.

Reviewed changes

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

Show a summary per file

File	Description
renderer/src/routes/settings.tsx	Adds secrets tab handling and prefetches secrets keys for the Settings route.
renderer/src/route-tree.gen.ts	Removes the standalone /secrets route from the generated route tree.
renderer/src/renderer.tsx	Imports the new OS design devtools module for global DevTools access.
renderer/src/features/secrets/components/secrets-table.tsx	Adds header row with “New secret” CTA and improves search placeholder copy.
renderer/src/common/lib/os-design.ts	Introduces overridable OS visual variant detection + DevTools helpers.
renderer/src/common/components/settings/tabs/settings-tabs.tsx	Adds the new Secrets tab and renders SecretsTab content.
renderer/src/common/components/settings/tabs/secrets-tab.tsx	Converts the former Secrets route UI into a Settings tab and refreshes empty-state UI/copy.
renderer/src/common/components/settings/tabs/tests/settings-tabs.test.tsx	Wraps SettingsTabs in Suspense and adds coverage for the Secrets tab.
renderer/src/common/components/settings/tabs/tests/secrets-tab.test.tsx	Updates imports/selectors to match the new Secrets tab UI.
renderer/src/common/components/link-view-transition.tsx	Updates the ordered route list (removing playground/secrets) for transition behavior.
renderer/src/common/components/layout/top-nav/window-controls.tsx	Switches mac detection to use getOsDesignVariant() and tweaks spacing.
renderer/src/common/components/layout/top-nav/nav-separator.tsx	Adds a simple separator component for top-nav grouping.
renderer/src/common/components/layout/top-nav/nav-icon-button.tsx	Adds a reusable icon-only nav button abstraction.
renderer/src/common/components/layout/top-nav/index.tsx	Refactors top-nav layout: primary links left, icon group right (Help/Settings/Assistant).
renderer/src/common/components/layout/top-nav/container.tsx	Updates top-nav container layout + mac padding logic via OS design variant.
renderer/src/common/components/help/help-dropdown.tsx	Migrates the Help trigger to use the new NavIconButton.
e2e-tests/secrets.spec.ts	Updates Secrets e2e flow to navigate via Settings tab.
e2e-tests/playground.spec.ts	Updates Assistant navigation/controls and conversation reset flows.

Comments suppressed due to low confidence (1)

renderer/src/common/components/settings/tabs/secrets-tab.tsx:59

• Minor copy/grammar: "for use into" is ungrammatical here. Consider changing to "for use in" (and likely lowercasing "server" for consistency: "MCP server configurations").

[samuv]

Looks good! 👍🏼