Skip to content

feat(shell): TopBar avatar opens account menu with Sign out - #454

Merged
remyluslosius merged 4 commits into
mainfrom
feat/topbar-account-menu
Jun 2, 2026
Merged

feat(shell): TopBar avatar opens account menu with Sign out#454
remyluslosius merged 4 commits into
mainfrom
feat/topbar-account-menu

Conversation

@remyluslosius

Copy link
Copy Markdown
Contributor

Summary

Fixes the operator-blocking bug: the TopBar avatar was a non-interactive <div>, so there was no way to sign out from the UI. Replaces it with a proper <button> that opens a popover menu with a Sign out action.

New spec frontend-shell-account-menu v1.0.0 (Tier 2, 5 C / 7 AC) locks the pattern.

Sign out path

  1. POST /api/v1/auth/logout (await; the cookie is still valid here)
  2. useAuthStore.clear() — local identity null
  3. navigate({ to: '/login' })

Order matters: API first; local clear + redirect run regardless of API result. A backend session that lingers is acceptable; a UI that thinks it's logged in while the backend has revoked is not.

Accessibility

  • <button> (keyboard-focusable, screen-reader-announced)
  • aria-haspopup="menu" + aria-expanded reflecting state
  • role="menu" container + role="menuitem" item
  • Escape (document keydown) AND click-outside (document mousedown) close the menu

Out of scope

  • Profile / Preferences menu items — deferred to v1.1 (the routes exist already)

Test plan

  • 7 vitest tests pass — topbar-account-menu.test.tsx
  • Full frontend suite green
  • tsc --noEmit clean
  • Spec validates
  • CI: spec-checks, frontend-tests

New spec frontend-shell-account-menu v1.0.0 (Tier 2, 5 C / 7 AC)
locks the account-menu pattern: <button> with aria-haspopup=menu,
role=menu dropdown, Escape + click-outside close, gated render on
identity presence.

Pre-fix: TopBar avatar was a non-interactive <div> showing the
user's initial. Operators had no way to sign out from the UI —
session ended only by inactivity timeout or manual cookie clear.

Fix path on Sign out:
  1. POST /api/v1/auth/logout (await; cookie still valid)
  2. useAuthStore.clear() — local identity null
  3. navigate({ to: '/login' })

Order matters: API first while the cookie is still valid; local
clear + redirect run regardless of API result. A backend session
that lingers is acceptable; a UI that thinks it's authenticated
while the backend has revoked is not.

Standards:
  - <button> not <div> (keyboard-focusable, SR-announced)
  - aria-haspopup=menu + aria-expanded reflecting state
  - role=menu container + role=menuitem item
  - Escape (document keydown) + click-outside (document mousedown)
    close the menu — both standard popover patterns

7 vitest tests covering AC-01..AC-07:
  - source-inspect: <button> + aria-haspopup, no legacy <div>
  - open/close flips aria-expanded
  - Sign out menuitem present
  - Sign out click: POST /auth/logout + store.clear + navigate
  - Escape closes
  - click outside closes
  - no avatar when identity is null
@remyluslosius
remyluslosius enabled auto-merge June 2, 2026 01:01
@github-actions github-actions Bot added the size/L label Jun 2, 2026
remyluslosius added a commit that referenced this pull request Jun 2, 2026
Brings in PRs #455 (host detail completion suite), #458 (pre-commit
spec-coverage hook), #459 (firewall-rule-count fix). The spec-coverage
hook is now active on this branch and detects a known specter v0.13.2
walker quirk on frontend-shell-account-menu (only credits AC-01 despite
all 7 ACs being correctly annotated in topbar-account-menu.test.tsx).
That spec belongs to PR #454, not this PR. CI is the authoritative gate.
# Conflicts:
#	app/specter.yaml
…specter walker bug

specter v0.13.2's `coverage --strict` walker has a per-file-AC-credit
bug that prevented this PR from passing CI's spec-coverage gate. The
walker credits exactly one AC per source file even when:

  - The test file's runtime annotations (test names contain
    spec-id/AC-NN) all fire and pass.
  - specter ingest correctly writes 7 'passed' entries in
    .specter-results.json.
  - The source has 7 // @ac AC-NN annotations, structurally identical
    to other working multi-AC test files (e.g. host-detail-system-
    card.test.tsx, which credits 6/6 of its in-file ACs).

I exhausted the obvious workarounds:
  - renamed test file to match spec basename
  - stripped the AC traceability docstring header
  - renamed the spec file
  - flipped spec status to draft

None of them changed the 1/7 credited result. The walker behavior is
genuinely a specter quirk we can't address from the spec/test side.

Workaround: one AC per test file. specter's walker now sees exactly
one @ac per file and credits it correctly. Verified locally:

  $ specter ingest --go-test ... --junit ...
  $ specter coverage --strict | grep shell-account-menu
  frontend-shell-account-menu  T2  7  7  100%  PASS

Layout:
  shell-account-menu-helpers.ts        — TOPBAR_SRC snapshot, signIn,
                                         clearAuth (pure helpers only)
  shell-account-menu-ac01.test.tsx     — source-inspection
  shell-account-menu-ac02.test.tsx     — open + flip aria-expanded
  shell-account-menu-ac03.test.tsx     — Sign out menuitem present
  shell-account-menu-ac04.test.tsx     — Sign out fires logout + nav
  shell-account-menu-ac05.test.tsx     — Escape closes
  shell-account-menu-ac06.test.tsx     — click-outside closes
  shell-account-menu-ac07.test.tsx     — anon = no avatar

Each of the 6 rendering files declares its own vi.mock for
@tanstack/react-router + @/api/client because vitest's vi.mock hoister
only operates on the test file the call is in — moving them to the
helper would silently break the mocks (Cannot read properties of null
(reading 'navigate')).

Net behavior: identical test coverage. Same 7 assertions, same
mocks, same fixtures. No production code changed.

Original topbar-account-menu.test.tsx removed. The new file basenames
match the spec id, which also makes the directory listing self-
documenting (each AC -> one file).
@github-actions github-actions Bot added size/XL and removed size/L labels Jun 2, 2026
@remyluslosius
remyluslosius merged commit 0f78e33 into main Jun 2, 2026
14 checks passed
@remyluslosius
remyluslosius deleted the feat/topbar-account-menu branch June 2, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant