Dismiss the account menu on Escape and outside click #279
- MA
## Bundle decision
Kept the proposed 1KB raw JavaScript allowance for shared menu dismissal and account keyboard behavior. The current build uses 255,471 bytes raw and 80,441 bytes gzip; the raw limit is 256,000 bytes and the compressed limit is unchanged.
## Why The sidebar account menu ignored Escape and outside clicks, and sat 12px above its button in a different style from other menus. The header document-actions menu was end-aligned, so it hung left over the Projects sidebar.
## What changed - New `useMenuDismissal` hook (`apps/web/src/menu-dismissal.ts`): Escape (restores focus to the trigger), outside pointerdown and focusin. Shared by the document actions menu and the account menu. - Account menu: dismisses like the others, opens upward at the trigger's width with a 4px gap, same item height, padding and colours as the document actions menu (44px on coarse pointers, z-index 60). - `DocumentActionsMenu` gains `align` (default `end`, so sidebar row menus are unchanged); the header passes `start`. Viewport clamping is unchanged. - E2E added in `e2e/document-navigation.e2e.ts` (account Escape + outside click + focus return; header menu left edge equals trigger left edge).
## Screenshots Before: account menu

After: account menu

Before: actions menu

After: actions menu

## Testing
- Local checks and TypeScript pass. All 479 web tests and 21 navigation browser tests pass, including Escape/outside dismissal, focus return, and header alignment. - Reviewed and renewed only the three exact dynamic owner hashes for menu positioning/motion and caller classes. - Rebased onto main after #272 merged. Full GitHub validation, browser integration, and container checks run before merge.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- MA
**Review:** Looks good
Verified in a built server (desktop 1440x900, phone 390x844 touch) against main. Non-blocking findings below.
**Works** - Account menu: Escape closes and refocuses the trigger (main: stayed open); outside click and trigger re-click toggle correctly. Menu is trigger-width, 4px above it, item 35px (44px on phone, coarse pointer). - Header actions menu left edge now aligns to its trigger; clamp unchanged. Escape returns focus to the trigger. - Hook cleans up all three listeners; `closeAccount` and `closeMenu` are stable so no resubscribe churn. Double Escape in `DocumentActionsMenu` (panel `onKeyDown` plus document listener) is idempotent. - Bundle: net-lean. The hook replaces an inline copy and is reused, so it should cost fewer bytes than it saves. The 1 byte headroom is not made worse by anything I can see.
**Findings** 1. `apps/web/src/menu-dismissal.ts:18` Escape inside the phone drawer closes the account menu AND the whole drawer. `NavigationFocusScope` (`navigation-focus.tsx:63`) dismisses on the React keydown first, then the hook's document listener runs and tries to focus a trigger in the closing drawer. Focus ends on "Open Projects sidebar". Same as main for the drawer, but Escape should peel the menu first. Fix: register the keydown listener in the capture phase and `stopPropagation()` (or have the scope ignore Escape when `[aria-expanded=true]` is inside it). 2. `navigation-shell.tsx:780` / `project-sidebar.tsx:456` the account menu has no keyboard contract despite `role="menu"`: no `aria-haspopup`/`aria-controls` on the trigger, no focus move on open, no arrow keys, and the menu is before the trigger in DOM order, so Tab from the trigger skips it (and closes it via focusin) and only Shift+Tab reaches "Sign out". Pre-existing, but the PR now presents it as the same menu as the document actions menu. Follow-up: focus first item on open (as `DocumentActionsMenu` does) or drop `role="menu"`. 3. `menu-dismissal.ts:30` the `// refs are stable` comment is fine but incomplete: `regions` is deliberately omitted from deps too (new array each render, read at event time). Say so. 4. `navigation-shell.tsx:315` `closeAccount` finds the trigger with `querySelector("[aria-expanded]")`. Works, but a `ref` on the trigger would be less fragile than an attribute query; low priority. 5. Account menu is CSS-positioned (no clamp). Fine because it is pinned to the sidebar bottom at trigger width.
**Other popovers not using the hook (follow-ups, not required here)** - `apps/web/src/anchored-picker.tsx:80-105` is a verbatim copy of the same three listeners (pointerdown, focusin, Escape + focus return). Best candidate: it would be a one-line swap and cut bytes. Used by `repository-picker` and `document-picker`. - `packages/editor/src/changes-chip.tsx:112-121` outside dismissal via mousedown only, no Escape, no focusin. - `packages/editor/src/widgets/language-menu.tsx:93-99` has Escape in its panel keydown and outside pointerdown, but not a document-level Escape (only works when focus is in the panel) and no focusin. - `apps/web/src/chat/reference-picker.tsx` and `mention-picker.tsx` are composer-driven (Escape handling on the textarea only in reference-picker, none in mention-picker); check Escape and outside click. - `packages/editor/src/toolbar/slash.tsx` has Escape only (editor-driven, probably fine). Callout menu delegates to `onOpenChange` (fine). `conversation-plan/markers.tsx`, `evidence-hover.tsx` and `resolved-layer`/`comment-layer` have their own pointerdown/focusin handling.
**Design contract:** only the known dynamic-owner hash failures noted in the PR; the change is safe (no new dynamic values).
- MA
Addressed review: Escape listener is now capture-phase with stopPropagation (drawer stays open); account menu has aria-haspopup/aria-controls, focuses its item on open, handles arrows/Home/End/Tab, menu now follows the trigger in DOM order, trigger uses a ref; hook comment explains the omitted `regions` dep; `anchored-picker.tsx` now uses the shared hook. Initial JS budget raised 255000 to 256000 B raw (usage 255471 B; gzip unchanged). e2e and container pass; validation fails only on the design-contract review hashes (not renewed).
MaggieAppletonreferenced this in Make popovers dismiss consistently and badge the phone Decisions count #303last month on GitHub
lavaman131referenced 8a307f69ca0clast month on GitHub
MaggieAppletonhead-ref-force-pushed 745e905ebebclast month on GitHub
MaggieAppletonhead-ref-force-pushed 011f20a736fflast month on GitHub
- MA
CI is still failing in `Format, lint, and enforce design tokens`. From the recorded run metadata plus the current diff, the likely root cause is stale design-contract review metadata in `scripts/design-contract/exceptions/dynamic-web.json` for the edited dynamic owners in `apps/web/src/document-actions-menu.tsx`, `apps/web/src/navigation-shell.tsx`, and `apps/web/src/project-sidebar.tsx`.
I did not push a speculative fix because this runner does not have Bun 1.4.2 available, and the normal Bun installer is blocked here, so I could not run the required reproduction/verification commands (`bun install --frozen-lockfile`, `bun run fix`, `bun run ci`).
Next action for a human: on a machine with Bun 1.4.2, run those commands on `011f20a736ff97a91f2994b633c8a590c7797644`, renew the reviewed `sourceHash` entries if the dynamic values are still intentionally unchanged, and rerun CI.
`babysit-head:011f20a736ff97a91f2994b633c8a590c7797644`
> [!WARNING] > <details> > <summary>Firewall blocked 1 domain</summary> > > The following domain was blocked by the firewall during workflow execution: > > - `releaseassets.githubusercontent.com` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "releaseassets.githubusercontent.com" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details>
> Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37618976002) · gpt54 · 78.9 AIC · ⌖ 6.27 AIC · ⊞ 15.1K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-call-id%3A+githubnext%2Fchopin%2Fpr-ci-fix%22&type=issues)
<!-- gh-aw-agentic-workflow: PR CI fixer, engine: codex, model: gpt-5.4, id: 37618976002, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37618976002 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->
MaggieAppletonhead-ref-force-pushed 352b4adf3e39last month on GitHub
MaggieAppletonhead-ref-force-pushed 3d29480b3d5dlast month on GitHub
MaggieAppletonhead-ref-force-pushed 840eb9a91bc7last month on GitHub
- MA
CI babysitter rechecked the latest failed `ci` run for `840eb9a91bc7a3ad17f1f3b41cfadda92db13c16`. The head still matches, `container` and `e2e` are green, and the first meaningful failure is still the `Format, lint, and enforce design tokens` step in the `format, lint, types, tests` job. From the current diff, that points at stale design-contract review metadata for the edited dynamic owners in `apps/web/src/document-actions-menu.tsx`, `apps/web/src/navigation-shell.tsx`, and `apps/web/src/project-sidebar.tsx` plus the related `scripts/design-contract/exceptions/dynamic-web.json` entries.
I did not queue a commit from this runner because `bun` and `mise` are not installed here, so I could not run the required local reproduction and verification commands (`bun install --frozen-lockfile`, `bun run fix`, `bun run ci`). Next action for a human: on a machine with Bun 1.4.2 available, rerun those commands on this head, review the design-contract output for those files, renew the exact exception entries only if the dynamic ownership is still intentional, and rerun CI.
`babysit-head:840eb9a91bc7a3ad17f1f3b41cfadda92db13c16`
> Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37627234570) · gpt54 · 42 AIC · ⌖ 4.92 AIC · ⊞ 15.1K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-call-id%3A+githubnext%2Fchopin%2Fpr-ci-fix%22&type=issues)
<!-- gh-aw-agentic-workflow: PR CI fixer, engine: codex, model: gpt-5.4, id: 37627234570, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37627234570 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->
- MA
@copilot the shadow on these popovers is too dark. bump it down to a lighter shadow value from the design system.
Copilotmentionedlast month on GitHub
Copilotsubscribedlast month on GitHub
MaggieAppletoncopilot-work-startedlast month on GitHub
> @copilot the shadow on these popovers is too dark. bump it down to a lighter shadow value from the design system.
Changed both account and document-actions popovers from `--shadow-overlay` to the lighter `--shadow-raised` token in ccb4312. Build and 475 web tests pass. CI still hits the pre-existing design-contract hash failures.
Copilotmentionedlast month on GitHub
Copilotsubscribedlast month on GitHub
MaggieAppletoncopilot-work-finishedlast month on GitHub
MaggieAppletonhead-ref-force-pushed 9a472ce0c1dblast month on GitHub
MaggieAppletonhead-ref-force-pushed 57ea469a319dlast month on GitHub
MaggieAppletonmerged 7830f6798943last month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub