githubnext/chopinPublic

Let's compose rich plans together

Animate sidebar document rows in and out #336

MergedMaggieAppletonmerged 2 commits intomainfromrefs/pull/336/head
  1. MA

    ## Why Sidebar document rows popped in on New document and vanished on Archive, so every row below jumped in a single frame.

    ## What changed - New `DocumentRow` wraps each top-level row in the existing `MotionDisclosure` (`.motion-collapse`, `useTransitionPresence`): inserts expand in, removals collapse and fade out. - `useSidebarRowPresence` retains a removed row until its exit finishes. Rows present on initial load, load-more, archive-mode switch or while loading are baseline and never animate. - A row inserted while not current gets a brief selected tint (`--duration-linger`, opacity only). - Archiving from the actions menu (keyboard or pointer) moves focus to the next row, the previous one, or the project toggle. The target is computed before the mutation and reclaimed after the portalled menu restores focus. - Row transition logic is a pure, unit-tested `nextTrack`; a refetch mid-exit lets the row finish. - Reduced motion / keyboard input is instant (existing `motionImmediately`).

    ## Screenshots Top row of each strip is before, bottom row after (frames about 50ms apart).

    New document:

    ![New document before/after](./filmstrip-new.png)

    Archive:

    ![Archive before/after](./filmstrip-archive.png)

    ## Testing - `bun run types`, `bun run fix`, `bun run build`: pass - `bun run ci`: fails only the design-contract hash check (see below) - `bun test apps/web/src/sidebar-row-presence.test.ts`: 9 pass; keyboard-archive e2e added to e2e/document-navigation.e2e.ts (verified focus by script; CI runs the spec) - Checked in headless Chromium against a fake-GitHub server.

    ## Needs a design-contract review - `apps/web/src/project-sidebar.tsx`, exception `scripts/design-contract/exceptions/dynamic-web.json`: new file sha256 `306f3942de231593779f914dec4aef45858aebf87bb25ec9c777cdd7b7c31b95`. The change adds a wrapper component and one conditional class; no new class-forwarding logic.

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

    ![New document filmstrip](https://github.com/user-attachments/assets/37af74b2-45b7-4099-9a93-54729880d0ca)

    ![Archive filmstrip](https://github.com/user-attachments/assets/dc81f992-44b8-4030-bdfc-9199d69d3687)

  2. MA
    Author

    **Review:** Changes needed

    Checked in headless Chromium (1440x900, fake-GitHub on 8833) with a rAF row-height sampler plus ~30ms screenshot frames.

    **Works** - Create: row expands 0 to 30px over about 250ms, rows below slide down. Archive: row collapses over about 250ms and fades. Keyed by id, no ghost rows or duplicate-key warnings after create then archive x2 in quick succession. - No animation on initial load or reload, project collapse/expand, archive-mode switch, or restore (rows swap in a single frame). Load-more is baseline by code (status goes loading, so `baseline` resets); I could not exercise it with seeded data. - `bun test apps/web`: 499 pass, 1 timeout failure (5000ms, slow machine; unrelated file). `bun run ci`: only the design-contract line (`project-sidebar.tsx:25 reviewed dynamic owner changed`); the PR's stated sha256 matches the file. CI: e2e and container pass; format/lint/types/tests fails on that same single line, which is acceptable under the hash rule.

    **Findings** 1. Focus fallback does not work for the real removal paths (`project-sidebar.tsx`, `DocumentRow` layout effect). Archiving through the row's actions menu, with mouse or keyboard, leaves `document.activeElement` on `<body>` at 50ms and at 1s. The menu is portalled, so `row.contains(active)` is false when `exiting` flips. For keyboard input `motionImmediately()` makes `steady` false, so the row is never retained and the effect never runs at all. As written the fallback only fires if a link inside the row is focused when another client or the Planner removes it. Fix: have the archive handler (or `onDocumentAction`) compute the fallback target before the mutation and focus it after the menu closes. The same code is needed for the immediate path. Add an e2e that archives by keyboard and asserts `document.activeElement`. 2. No unit test for `useSidebarRowPresence` (`sidebar-row-presence.ts`). The transition logic is pure but sits inside `useState`. Please extract a pure `nextTrack(track, groups, ready, scope, immediately)` and cover: baseline on load, loading to ready not animating, scope change, insert, remove, re-insert while exiting, `immediately`, exiting index clamp. 3. Minor: `DocumentRow` passes a new `onExited` closure every render, and the timer effect depends on it, so each parent render during the exit restarts the 50ms-padded timeout. A stable ref would avoid delaying removal under frequent re-renders. 4. Minor: a project refetch that sets `ready` false mid-exit resets `exiting`, so a collapsing row would vanish abruptly. Probably acceptable, but worth a note. 5. Not exercised: new-row tint for a document inserted by someone else (creating locally navigates to the row, so it is current and tint is suppressed by design).

    The `--duration-linger`, `--motion-move` and `--acc-expand` tokens, the `prefers-reduced-motion` handling and the conventions look fine.

  3. MA
    Author

    Addressed in the latest push: (1) focus target is computed in the archive handler before the mutation and reclaimed after the menu closes (works for keyboard/immediate and pointer; verified by script) plus a keyboard e2e; (2) pure `nextTrack` extracted with 9 unit tests; (3) `onExited` is held in a ref so the exit timer no longer restarts; (4) a refetch (`ready` false) now keeps rows already exiting so they finish. Design-contract hash in the body updated.

  4. MaggieAppletonhead-ref-force-pushed 341ecff5562alast month on GitHub

  5. MaggieAppletonmerged 85cb6db8e83elast month on GitHub

  6. MaggieAppletonclosedlast month on GitHub

  7. MaggieAppletonhead-ref-deletedlast month on GitHub

  8. MaggieAppletonreferenced this in Offer Undo after archiving and label archived mode #363last month on GitHub