githubnext/chopinPublic

Let's compose rich plans together

Compact the Projects sidebar into single-line rows #284

MergedMaggieAppletonmerged 5 commits intomainfromrefs/pull/284/head
  1. MA

    Sidebar documents use single-line 30px rows, with descriptions available beside the rail on hover or keyboard focus. Touch targets stay 44px and tapping does not show a tooltip.

    Integration review: the sidebar icon still forwards the same static caller classes. Renewed its exact source hash after reviewing the row markup and producers; no new exception was added.

    Local validation passes: checks, types, 481 web tests (the 13 socket tests rerun with local networking), and all 24 navigation browser tests, including main’s current archive/menu behavior. Rebased onto merged #283, retaining main’s empty-state copy and compact shared spacing. Final required GitHub CI is running.

    <details><summary>Original proposal and screenshots</summary>

    ## Why

    Every document in the Projects sidebar was a two-line, ~50px row: the name plus a truncated generated description ("Plan for server-side canvas …"). With a dozen documents the rail turned into a wall of cut-off grey text that was rarely readable anyway.

    ## What changed

    - Document rows are single-line and 30px tall: name only, `text-sm`, weight 400. The current document keeps its selected background, primary ink, and weight 500. Research children keep their indent guide and ancestor treatment. - The description moves into a card beside the rail. It appears after the standard tooltip delay on hover or keyboard focus, wraps, and is at most 280px wide. It reuses the shared `IconTooltip` surface through a new `data-tooltip-side="right"` variant, anchored to the rail edge (`data-tooltip-edge`). Touch never shows it. Focus opens a tooltip only when it is `:focus-visible`, using the exact guard `if (!(target instanceof Element) || !target.matches(":focus-visible")) return;` at the top of `focusIn`. The touch-ergonomics PR adds the identical guard, so the rebase should be trivial. The card's top is clamped to the window, so a long description on a bottom row stays on screen. After Escape hides a focused control's card, it stays hidden until focus moves, rather than coming back on unrelated pointer moves. - Research child rows intentionally have no description card. Children never showed a description, and only parents carry a generated one. - The link's accessible name is still the title alone. The description stays available to screen readers through `aria-description`. - One rhythm for the whole rail: New document, Search, project rows, document rows, children, Archived chats, and the account row now share `--project-sidebar-row` (`calc(var(--spacing) * 7.5)`), plus the same inset, 8px gap, and 14px icon column (the avatar is now 14px too). Document names, loading, empty, and Load more text line up with the icon-row labels. - The per-project "New document" pencil appears only on hover or focus of its own row, not while hovering any document in the project. It stays visible on coarse pointers, and coarse-pointer rows keep their 44px targets. - The design-audit Navigation specimen matches: 30px rows, current at weight 500, ancestor in primary ink, and one row with a description card.

    ## Screenshots

    Before (1440, 12 documents + one research child):

    ![Before desktop](https://github.com/user-attachments/assets/04fa8d04-0619-4114-be5d-98cba19c63cf)

    After (1440):

    ![After desktop](https://github.com/user-attachments/assets/f01bb8ce-3536-415a-b146-266f9fbc9cb2)

    After, hovering a row shows its description:

    ![After hover](https://github.com/user-attachments/assets/dd266da8-db93-4a0f-bc21-44d8474a0983)

    Phone drawer, before and after (coarse-pointer rows stay 44px):

    ![Before phone](https://github.com/user-attachments/assets/797b71b6-7dcb-4612-8fd3-9473ce710ecc) ![After phone](https://github.com/user-attachments/assets/9e69bb15-762f-4bfe-a1bc-a75226af61e4)

    ## Testing

    - `bun test apps/web` — 470 pass. I updated `research-sidebar.test.ts` (new link markup) and `theme.test.ts` (shared row height replaces the old `gap: 10px` check). - `bun run types` — pass. - `bun run ci` — dprint, oxlint, tokens, design record, and `design:check` all pass. The only failure is the design-contract review hash below. - New e2e tests in `e2e/document-navigation.e2e.ts`: a tapped row on a 1180px touch layout shows no card, and a long description card on the bottom row of a 420px-tall window stays inside it. - New e2e test in `e2e/document-navigation.e2e.ts` ("sidebar rows stay single-line and reveal descriptions beside the rail"): it checks the 30px row, the accessible description, the card position beside the rail, and the pencil appearing only on project-row hover. Local e2e is suspended, so it ran in CI. The CI e2e job passes, including this test and the updated pagination test, which now checks the description through `toHaveAccessibleDescription`. The validation job fails only on the design-contract hash below, which stops it before CI's unit and type steps; I ran those locally and they pass. - Checked in a real browser against a fake-GitHub server with 12 seeded documents, at 1440×900 and 390×844.

    ## Needs a design-contract review

    - `apps/web/src/project-sidebar.tsx`, covered by `scripts/design-contract/exceptions/dynamic-web.json` (`class > NavigationIcon > <img>`). - New source hash: `b12634c312087e7874a64baef40310b3d98e0e135b82c14c73f89f17543f799f` - Why it's safe: the reviewed dynamic owner (`NavigationIcon`, line 24) is unchanged. It still forwards only the caller's static class names. The hash moved because other markup in the file changed: the document link classes, the tooltip data attributes, the 14px avatar, and the `aside` attributes. No new runtime class values were introduced.

    🤖 Generated with [Claude Code](https://claude.com/claude-code)

    </details>

  2. MA
    Author

    **Review:** Changes needed (small, none blocking the design direction)

    Built the branch, ran it against a fake-GitHub server with 25 documents (one 100+ character name, one research child) and looked at it at 1440x900, 1180x820 touch, and 390x844 touch. The change looks right: every rail row measures exactly 30px (44px on coarse pointers), the document names line up with the icon-row labels, the card sits flush beside the rail after 400ms, wraps at 280px, and the New document pencil only shows on its own project row (opacity 0 on document hover, 1 on project-row hover and on focus). Long names truncate cleanly. The Navigation specimen matches (30px rows, card beside the plate). The link keeps its title-only name with the description exposed as `aria-description`.

    **Findings**

    1. **Touch still shows the card** (`apps/web/src/icon-tooltip.tsx`, `focusIn`). The touch guard only exists in `pointerOver`. On a touch device that keeps the persistent rail (tablet, 1180px with `hasTouch`, `pointer: coarse` true), tapping a document row focuses the link and, 400ms later, the description card appears beside the rail and stays. That contradicts "touch input never shows it". Suggested fix: remember the last `pointerType` (set it in a capturing `pointerdown`) and skip `focusIn`/`enter` when it was `touch`, or only honour focus when the focus came from the keyboard (`:focus-visible`, `target.matches(":focus-visible")`). A phone drawer tap doesn't show it only because navigation closes the drawer. 2. **No vertical clamp on the card** (`icon-tooltip.tsx`, right-side branch). `top` is the row centre with `translateY(-50%)`. Realistic one-line descriptions are fine near the bottom (card bottom 854 of 900 for a 5-line card), but a long description on one of the last rows overflows: a 120-word description on the bottom row ran from y=626 to y=976 in a 900px window, and in a 300px-tall window the card is pushed to the top edge. The brief asked about flip near the window bottom: right placement never flips vertically. Suggested fix: after measuring, clamp `top` to `[8 + h/2, innerHeight - 8 - h/2]` (or switch to `translateY(0)` and clamp the top). 3. **Keyboard focus lingers as a hover fallback** (`icon-tooltip.tsx`, `pointerOver` -> `enter(hovered ?? focused)`). After Escape hides a focus-triggered card, moving the mouse over any non-tooltip element brings the focused row's card back after 400ms (reproduced: Tab to a row, Esc, move the pointer to the project row, card reappears). The fallback already existed for icon buttons but is much more noticeable now that document links are permanent focus holders. Low severity; consider ignoring the focused fallback once dismissed with Escape until focus moves. 4. **Research child rows get no card** (`project-sidebar.tsx`, child link unchanged). Intentional or not, children have no description attributes, so only parents reveal a description. Fine for v1 (children previously had none), but worth stating in the PR so it is not read as a gap. 5. Minor: `design-audit/surfaces.css` uses raw `1.875rem` and `0.5rem` where the rail now has `--project-sidebar-row`/`-inset` tokens (the specimen is outside the rail so the variables are not in scope; consider hoisting them to the audit scope). The `icon-tooltip.css` right variant uses a raw `17.5rem`; consistent with the existing `20rem` but not a token.

    **Phone drawer:** rows stay 44px, descriptions are not shown, the per-project pencil stays visible, and indent/inset look consistent. A stale "Close Sidebar" tooltip is visible in my after-tap screenshot; that is existing focus-tooltip behaviour (same root cause as finding 1), not new.

    **Conflicts with other open PRs touching `project-sidebar.tsx` / `navigation.css`** - **#281 (empty states): real textual conflict in `apps/web/src/navigation.css`** around `.project-sidebar-empty`. #281 sets `padding: 0 8px 4px 32px` and drops `.project-sidebar-empty-action`; this PR moves that padding onto `--project-sidebar-label`/`--project-sidebar-inset`. Whoever lands second should rebase and resolve to the tokenised padding with #281's line-height and action removal. - **#280 (archived labels)** and **#283 (copy)**: merge cleanly with this branch (`git merge-tree` reports no conflicts). #280/#283 rename "Archived chats" to "Archived" and "Back to active docs" to "All documents" and rename refs in `project-sidebar.tsx`; this PR's e2e/tests and the `.project-sidebar-primary-action` height rule don't depend on those strings. #283 also edits `icon-tooltip.tsx` (and its test); no overlap with the new `data-tooltip-side` branch, but re-run `icon-tooltip.test.ts` after the second one lands. - **#286 (navigation dialogs)** also touches `project-sidebar.tsx` and `navigation.css`; merges cleanly.

    **Tests:** `theme.test.ts` and the e2e test cover height, description, card-beside-rail, and pencil hover well. Missing: a touch/coarse-pointer case (would have caught finding 1) and a bottom-of-window case. Both need Playwright, per AGENTS.md.

    **Design-contract hash:** the only `bun run ci` failure is the listed review hash for `project-sidebar.tsx`; the change is safe.

  3. MaggieAppletonreferenced this in Fix touch ergonomics on phones #299last month on GitHub

  4. MA
    Author

    Thanks for the review. All five findings are addressed:

    1. **Touch shows the card:** fixed. `focusIn` now starts with exactly `if (!(target instanceof Element) || !target.matches(":focus-visible")) return;`. That is the same guard the touch-ergonomics PR adds, so the rebase should be trivial. New e2e: on a 1180px touch layout, tapping a row focuses it and no card appears. 2. **No vertical clamp:** fixed. The right-side card's centre is clamped to `[8 + h/2, innerHeight - 8 - h/2]`. New e2e: a long description on the bottom row of a 420px-tall window stays inside the window. 3. **Focus fallback after Escape:** fixed. Escape keeps the focused control's tooltip dismissed until focus moves. Pointer moves and scroll no longer bring it back. 4. **Research child rows:** intentional. Child documents have no generated description, and they never showed one. This is now stated in the PR body. 5. **Raw sizes:** `design-audit/surfaces.css` now uses `calc(var(--spacing) * 7.5)` and `calc(var(--spacing) * 2)`. The tooltip's right variant uses `calc(var(--spacing) * 70)`.

    CI: e2e passes, and so does the container build. The validation job fails only on the listed design-contract review hash for `project-sidebar.tsx`; that file is unchanged since the first push, so the hash is the same. One run hit a flake in `decision-marker.e2e.ts` ("typing with a visible decision card selected…"). That test is unrelated to this change and passed on the rerun.

  5. lavaman131referenced b5d08681087flast month on GitHub

  6. MaggieAppletonhead-ref-force-pushed 7606043fe1f9last month on GitHub

  7. MaggieAppletonhead-ref-force-pushed 346f85b420fblast month on GitHub

  8. MA
    Author

    CI is still failing in `Format, lint, types, tests` on the recorded head. The first meaningful failure is `bun scripts/check-design-contract.ts`, which reports `apps/web/src/project-sidebar.tsx:24 reviewed dynamic owner changed; inspect its data flow and renew the exact exception`.

    I did not queue a guarded `[ci-fix]` commit because this runner does not have Bun installed, so I could not run the required reproduction and verification commands (`bun install --frozen-lockfile`, `bun run fix`, `bun run ci`). I also could not fully disambiguate the exact renewal from static inspection alone: the branch still has the reviewed `NavigationIcon` boundary at line 24, and a raw file-hash probe here matched the checked-in `dynamic-web.json` entry, so this needs one real local `check-design-contract.ts` run on the branch rather than a blind hash edit.

    Next action for a human: on a machine with Bun 1.4.2, run those commands on `346f85b420fb7b50be1db0d135882c3cc43f1102`, inspect the `project-sidebar.tsx` dynamic-owner report, renew the exact exception only if that boundary is still safe, and rerun CI.

    `babysit-head:346f85b420fb7b50be1db0d135882c3cc43f1102`

    > Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37618989481) · gpt54 · 72.4 AIC · ⌖ 4.97 AIC · ⊞ 16.4K · [◷](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: 37618989481, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37618989481 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->

  9. MaggieAppletonhead-ref-force-pushed a8a8fc36c28blast month on GitHub

  10. MaggieAppletonhead-ref-force-pushed 07845321b026last month on GitHub

  11. MaggieAppletonhead-ref-force-pushed d73e4bf47e00last month on GitHub

  12. MA
    Author

    CI babysitter rechecked the current failed `ci` run for `d73e4bf47e007fd6228a53740f04f214ddf4fff1`. The head still matches the recorded candidate, and the failure still appears to be the reviewed design-contract hash path rather than a product-behavior regression: the checked-in exception for `apps/web/src/project-sidebar.tsx` on this head does not match the file content, which hashes to `b12634c312087e7874a64baef40310b3d98e0e135b82c14c73f89f17543f799f` while the exception file still carries the older reviewed hash.

    I did not queue a guarded `[ci-fix]` 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, inspect the exact `check-design-contract.ts` report, renew the `project-sidebar.tsx` exception only if that `NavigationIcon` ownership boundary is still safe, and rerun CI.

    `<!-- pr-ci-fix:d73e4bf47e007fd6228a53740f04f214ddf4fff1 -->`

    > Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37627276943) · gpt54 · 52.4 AIC · ⌖ 4.51 AIC · ⊞ 16.4K · [◷](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: 37627276943, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37627276943 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->

  13. MaggieAppletonhead-ref-force-pushed 5e4d2191b6balast month on GitHub

  14. MaggieAppletonhead-ref-force-pushed 173c9903bb55last month on GitHub

  15. MaggieAppletonmerged 2bd6387ff2e4last month on GitHub

  16. MaggieAppletonclosedlast month on GitHub

  17. MaggieAppletonhead-ref-deletedlast month on GitHub