Polish shell menus and Add project focus #337
- MA
## Why Audit findings 8, 10, 11 (shell). After adding a project with the keyboard, a heavy ring boxed the whole project group. The document and account menus were bare, with no Copy link. The Chat toggle was named "Close sidebar", colliding with the Projects "Hide sidebar".
## What changed - Add project: focus goes to the project's disclosure button (not the `li`), the group scrolls into view (`block: nearest`), and the project row flashes `--color-selected` to transparent over `--duration-linger` (motion-safe only). - Document actions menu: new "Copy link" (first item), 14 px leading icons on every item (new `LinkIcon` and `PencilIcon` in `@chopin/icons`). Copying shows a quiet "Link copied" status (live region, 2 s). - Account menu: adds "Manage repository access" (`/auth/github/install`), a separator, icons. No keyboard-shortcuts entry. - Notice: one shared `NavigationNotice` (message, optional action, duration) reuses the `.navigation-creation-status` surface. Account menu body and project focus/flash moved into the lazy sidebar; initial JS is 255895 B (budget 256000). - Chat toggle: "Hide chat" / "Show chat" (was "Close sidebar" / "Show chat pane"). Tests and E2E selectors updated. - Checked #303 (it only changes `menu-dismissal.ts`, no overlap) and #283 copy conventions.
## Screenshots Before add-project focus:  After add-project focus:  Before document menu:  After document menu:  After Link copied (shared notice) desktop:  Phone:  Before account menu:  After account menu: 
## Testing - `bun run types`: pass. `bun test apps/web`: 500 pass. - `bun run ci`: dprint, oxlint, tokens pass; design-contract fails only on dynamic-owner exceptions (below). - E2E not run locally (suspended); relying on PR CI.
## Needs a design-contract review `bun run ci` reports "reviewed dynamic owner changed" (line shifts and new JSX icon props, no new style logic) for: `packages/icons/src/line.tsx`, `apps/web/src/document-actions-menu.tsx`, `apps/web/src/navigation-shell.tsx`, `apps/web/src/workspace.tsx` (label strings only; line shifts). The checker prints no new hash; exception files need renewal by the design-contract owner.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- MA
**Review:** Changes needed
Checked in a real browser (desktop 1440x900 and phone 390x844, fake server vs the diff). Menus, icons and the Hide chat label look right. Two CI failures and some design points need attention.
**Blocking** 1. `e2e` fails: `e2e/design/interactions.e2e.ts:16` still expects "Rename" to be the first focused menu item. "Copy link" is now first (`document-actions-menu.tsx` `documentMenuItems`), so both wide and narrow runs fail. Update the test to expect "Copy link" (and the ArrowDown order). 2. `container` fails: `[initial-javascript-budget] initial JavaScript 264477 B raw / 84730 B gzip exceeds budget 256000 B / 81500 B`. Reproduced locally (264481 B). Main passes, so this PR (static icon imports in `navigation-shell.tsx`, `document-actions-menu.tsx`, plus the notice state) pushes the entry chunk over. Needs a trim, a lazy-loaded menu, or a conscious budget decision by the owner. 3. `format, lint, types, tests` fails only on design-contract review hashes (document-actions-menu, navigation-shell, workspace, icons/line). Acceptable per the audit rules; owner renews hashes. `types` was not reached by that job, so run `bun run types` before merging.
**Design findings** - "Link copied" toast: there is no existing toast. The only comparable transient surface is `.navigation-creation-status` (`navigation.css:591`, role=status, `--color-page`, `--shadow-resting`, `--text-sm`, `--color-text-tertiary`). It is top-anchored, so a bottom toast is a second, different pattern. Suggest sharing that surface's styling (`--shadow-resting`, same padding) rather than `--shadow-raised`/`--color-text-secondary`, and extracting a tiny shared notice component. Visually the pill is white on near-white with a faint shadow, sits at `bottom: 16px` and overlaps the composer send button on desktop and the last card on phone (`d-toast.png`, `m-toast.png`). Raise it above composer height or move it where creation-status lives. Behaviour is otherwise good: role=status live region, 2s auto-dismiss, reduced-motion guarded, clipboard verified to hold the document URL. - Copy link robustness: `navigator.clipboard.writeText` throws synchronously if `navigator.clipboard` is undefined (non-secure context, e.g. plain http on a LAN host). The `.then(ok, fail)` handler does not catch that; wrap in try/catch to show "Could not copy link". - "Manage repository access" (`navigation-shell.tsx:831`, `/auth/github/install`): safe. It is a plain GET that 302s to `github.com/apps/<validated slug>/installations/new` (`apps/server/src/auth/routes.ts:267`, slug regex-validated, no state or user input) and the same link already exists in Add project, the repository picker and the new-document dialog. Fine for all signed-in users; the GitHub page itself enforces who may install. Two nits: it is an `<a>` with role=menuitem that leaves the app in the same tab (consider `target="_blank" rel="noopener"` so the user keeps document state), and in local-auth mode the link may point at a slug the operator did not configure; check it is hidden or valid there. - Separator: `[role=separator]` uses `--color-selected` (gray-200, the selected-row fill). The app's actual divider token is `--color-edge` with `--edge-width` (`navigation.css:728, 826`; the search palette footer uses exactly this). Switch to `border-top: var(--edge-width) solid var(--color-edge)`. There are no other menu separators to compare with, so this sets the precedent. It renders acceptable but faint, and the 4px margin plus menu padding makes the gap above it look larger than below. - Icons: `LinkIcon` and `PencilIcon` use the shared `LineIcon` (18 viewBox, 1.5 stroke, round caps), rendered at size 14 in the menu; they match Archive/Lock/SignIn in the screenshots. `PencilIcon` is visually the same glyph as the New document sidebar pencil, which is good. Minor: `CloseIcon` for "Delete permanently" and `ArrowUpIcon` for "Restore" are weak metaphors (not in the default flow, so low priority). - Row flash: it works (focus lands on the disclosure and the row is scrolled into view) but is subtle. Sampled computed background every 50ms: alpha 0.82 at 50ms, 0.31 at 150ms, 0.05 at 300ms, about 0 by 600ms. The ease-out decay means it is clearly visible for roughly 150ms only, and gray-200 on the sidebar is close to hover. Suggest a longer plateau (hold ~250ms then fade, or use a linear/ease-in-out ramp), or an accent-tinted fill, so it registers as "this is the one you added". - `design-audit/interactive.tsx`: `copy-link` is a silent no-op in the specimen, which is fine. - Hide/Show chat: label and tooltip consistent; aria-label "Hide chat" / "Show chat, status" good. The `-scale-x-100` Tailwind utility on the SignInIcon is fine as a flip.
Screenshots in `/tmp/r337/` (local).
MaggieAppletonreferenced this in Add global keyboard shortcuts with hints and a shortcuts sheet #351last month on GitHub
MaggieAppletonhead-ref-force-pushed 1255493d72f8last month on GitHub
- MA
Addressed review: bundle fixed (the @chopin/icons barrel in the entry cost ~8 KB; account menu and project focus/flash now live in the lazy sidebar, 255895 B vs 256000). Interactions e2e updated. Shared NavigationNotice reuses the creation-status surface (under the header, clear of the composer). Separator uses edge tokens. Flash holds ~250 ms. Clipboard wrapped in try/catch with a failure message. Manage repository access opens in a new tab; the install route is registered in local mode too and the Add project dialog already links it, so it stays visible.
MaggieAppletonreferenced this in Move signed-out and lazy-only code out of the initial bundle #354last month on GitHub
MaggieAppletonhead-ref-force-pushed 6031aa489b4alast month on GitHub
- MA
Rebased onto main (conflict in navigation-shell.tsx resolved, main's account-menu keyboard contract kept). CI: container and e2e pass; initial JS is within budget (255485 B raw / 80552 B gzip locally). The only failing check is the design-contract exception review: document-actions-menu.tsx, navigation-shell.tsx, project-sidebar.tsx (new dynamic class: accountMenu.className, the existing presence class moved from the shell), workspace.tsx (label strings), packages/icons/src/line.tsx (two new icons, appended). No new style logic; exception files need renewing by the design-contract owner.
- MA
**Review:** Changes needed
Re-verified at 6031aa48 in a real browser (desktop 1440x900, phone 390x844). CI: container and e2e pass; "format, lint, types, tests" fails only on design-contract review hashes (35 "reviewed dynamic owner changed" lines, nothing else), which is acceptable. Initial JS is back under budget (255485 B / 80552 B gzip).
Verified good - Separator: `--color-edge`, 1px, 4px above and 4px below (measured). - Row flash: opaque for about 250ms, then fades out by about 700ms. Clearly perceptible now. - Manage repository access: `target="_blank" rel="noopener"`. - Account menu opens immediately (menu item present within about 18ms), no flash. Icons and layout look right. - Copy link is first in the document menu and is the focused item on open. Clipboard holds the correct URL. With `navigator.clipboard` removed it shows "Could not copy link" (no throw). - Notice reuses `.navigation-creation-status`, role=status, 2s auto-clear. It does not cover the composer or cards on desktop or phone.
Findings 1. Account menu arrow keys (`project-sidebar.tsx`, the moved `onKeyDown`): ArrowDown, ArrowUp, Home and End all call `querySelector("[role=menuitem]")?.focus()`, which is always the first item. With two items, pressing ArrowDown twice stays on "Manage repository access"; "Sign out" cannot be reached by arrow keys, and Tab closes the menu. It was harmless with one item and is now a keyboard regression. Fix: move focus through `querySelectorAll("[role=menuitem]")` (next, previous, first, last). 2. The notice sits over the workspace tab header: on desktop it replaces the Chat / Document / Decisions tab row (white pill on white, so it reads as a header with the text "Link copied" and the tabs vanish for 2s); on phone it covers the top of the tab bar. Cards and composer are fine, but the tabs being hidden is jarring and the shadow is too faint to read as a floating notice. Suggest anchoring it centred over the content area below the tab header, or giving it a visible edge (`--shadow-raised` / `--color-edge` border) and a narrower, content-sized width.
MaggieAppletonhead-ref-force-pushed 469dea88e878last month on GitHub
- MA
Both follow-ups addressed: account menu arrows now step through items with wrap and Home/End (unit test for menuItemTarget plus an e2e ArrowDown to Sign out). The shared .navigation-creation-status is now a content-sized, edged (--shadow-raised + edge border) pill centred just below the pane tabs, with a fade and rise in/out using existing duration tokens; it also applies to the creation status. Initial JS 255485 B raw / 80549 B gzip locally.
MaggieAppletonreferenced this in Offer Undo after archiving and label archived mode #363last month on GitHub
MaggieAppletonhead-ref-force-pushed 6d4cfb987aealast month on GitHub
MaggieAppletonreferenced this in Share one menu surface recipe across pickers and the language menu #377last month on GitHub
MaggieAppletonhead-ref-force-pushed 811886a9e4bdlast month on GitHub
MaggieAppletonhead-ref-force-pushed 17ac03d52f70last month on GitHub
MaggieAppletonhead-ref-force-pushed fca4487a5544last month on GitHub
MaggieAppletonauto-rebase-enabledlast month on GitHub
MaggieAppletonhead-ref-force-pushed 5e7cde9fb8e6last month on GitHub
MaggieAppletonmerged 7188f6e56b1elast month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub
MaggieAppletonreferenced this in Calm entry and error states #344last month on GitHub