SAM-1: Drop Settings from the desktop primary nav #802

Closed
opened 2026-05-03 19:45:42 +00:00 by code-lead · 0 comments
Collaborator

As an operator, I want the desktop top nav to carry only operational surfaces, so that I can scan to my day-to-day pages without parsing past a configuration tab I rarely use.

Acceptance criteria

Nav definition

  • NAV_ITEMS in apps/web/src/components/app-shell.tsx no longer contains the /settings entry. The remaining order is Board · Workspace · Agents · Flows.
  • The Settings icon import from lucide-react stays — it is reused by the avatar-menu Settings row added in SAM-2 — or moves with the row, whichever import graph is cleaner.
  • No other consumer of NAV_ITEMS exists; if a search turns one up it is updated in the same commit.

Mobile drawer

  • The mobile drawer (<Drawer side="left"> block in app-shell.tsx, today rendering NAV_ITEMS.map(...)) keeps a visible Settings entry. Implementation choice — either (a) keep NAV_ITEMS as the desktop list and define a separate MOBILE_NAV_ITEMS = [...NAV_ITEMS, settingsItem], or (b) render NAV_ITEMS then append a hand-rolled Settings <Link> after it. Pick the one that doesn't fork the icon/test-id contract.
  • The mobile entry preserves data-testid="nav-drawer-nav-settings" so the existing Playwright selector still resolves.
  • Tapping the mobile Settings entry closes the drawer (onClick={closeMenu}) like every other drawer link.

Tests

  • Vitest covers app-shell.tsx: the desktop nav rendering (viewport ≥ md) renders four <Link> children with labels Board, Workspace, Agents, Flows — no Settings.
  • Vitest covers the mobile drawer rendering (viewport < md, drawer open): Settings is present and clickable.
  • A Playwright spec under apps/web/tests/ opens the desktop shell and asserts getByTestId('nav-settings') is not visible, while getByTestId('avatar-menu-settings') (introduced in SAM-2) is reachable from the avatar menu.

Out of scope

  • Removing other nav entries (covered by ui-consolidation.md).
  • Renaming /settings/* routes.
  • Changing the Settings page itself.

References

  • Spec: specs/settings-into-avatar-menu.md § "Story SAM-1"
  • apps/web/src/components/app-shell.tsxNAV_ITEMS (line 103), desktop nav (line 201), mobile drawer nav (line 259)
  • Sibling spec specs/ui-consolidation.md — prior nav-collapse work; this spec is consistent with that direction

Dependencies

Blocks on #801 (SAM-2). The avatar-menu Settings entry must exist before this issue removes the desktop nav entry — otherwise the page is unreachable from the shell between the two PRs landing.

As an operator, I want the desktop top nav to carry only operational surfaces, so that I can scan to my day-to-day pages without parsing past a configuration tab I rarely use. ## Acceptance criteria ### Nav definition - [ ] `NAV_ITEMS` in `apps/web/src/components/app-shell.tsx` no longer contains the `/settings` entry. The remaining order is `Board · Workspace · Agents · Flows`. - [ ] The `Settings` icon import from `lucide-react` stays — it is reused by the avatar-menu Settings row added in SAM-2 — or moves with the row, whichever import graph is cleaner. - [ ] No other consumer of `NAV_ITEMS` exists; if a search turns one up it is updated in the same commit. ### Mobile drawer - [ ] The mobile drawer (`<Drawer side="left">` block in `app-shell.tsx`, today rendering `NAV_ITEMS.map(...)`) keeps a visible **Settings** entry. Implementation choice — either (a) keep `NAV_ITEMS` as the desktop list and define a separate `MOBILE_NAV_ITEMS = [...NAV_ITEMS, settingsItem]`, or (b) render `NAV_ITEMS` then append a hand-rolled Settings `<Link>` after it. Pick the one that doesn't fork the icon/test-id contract. - [ ] The mobile entry preserves `data-testid="nav-drawer-nav-settings"` so the existing Playwright selector still resolves. - [ ] Tapping the mobile Settings entry closes the drawer (`onClick={closeMenu}`) like every other drawer link. ### Tests - [ ] Vitest covers `app-shell.tsx`: the desktop nav rendering (viewport `≥ md`) renders **four** `<Link>` children with labels `Board, Workspace, Agents, Flows` — no Settings. - [ ] Vitest covers the mobile drawer rendering (viewport `< md`, drawer open): Settings is present and clickable. - [ ] A Playwright spec under `apps/web/tests/` opens the desktop shell and asserts `getByTestId('nav-settings')` is **not** visible, while `getByTestId('avatar-menu-settings')` (introduced in SAM-2) is reachable from the avatar menu. ## Out of scope - Removing other nav entries (covered by `ui-consolidation.md`). - Renaming `/settings/*` routes. - Changing the Settings page itself. ## References - Spec: `specs/settings-into-avatar-menu.md` § "Story SAM-1" - `apps/web/src/components/app-shell.tsx` — `NAV_ITEMS` (line 103), desktop nav (line 201), mobile drawer nav (line 259) - Sibling spec `specs/ui-consolidation.md` — prior nav-collapse work; this spec is consistent with that direction ## Dependencies Blocks on #801 (SAM-2). The avatar-menu Settings entry must exist before this issue removes the desktop nav entry — otherwise the page is unreachable from the shell between the two PRs landing.
dev closed this issue 2026-05-03 20:19:00 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
charles/claude-hooks#802
No description provided.