Keep the comment card off its passage and match Chat's composer and author row #301
- MA
## Integration and validation
Restacked on #300’s validated marker groups. Comment cards choose the available gutter or space above/below the prose, use one inset send action, and preserve touch/IME input behavior and focus restoration. Reviewed actual card geometry and presence boundaries.
Formatting/lint/design checks, types, 34 focused tests and all 53 responsive-comment/sidecar browser tests pass locally. The parent merges first; final main rebase and required CI follow.
<details> <summary>Original proposal and screenshots</summary>
Stacked on #300
## Why
The open comment card docked at the document's right edge, so it usually covered the right half of the very passage being discussed. The new-comment composer also had a different shape from the reply field (× plus Cancel, and left-aligned Comment/Cancel buttons instead of the inset send arrow), and authors appeared as teal "@ana" text while Chat shows a face and a display name.
## What changed
- **Placement** (`commentCardPoint` in `comment-geometry.ts`, replaces `edgePanelPoint`): when the gutter right of the prose column can hold the card beyond the marker lane, the card sits there beside the passage's first line. Otherwise it sits directly below the passage, aligned to its left edge and kept inside the column, flipping above when there is no room below. If neither side holds the whole card, the roomier side caps its height so the card scrolls instead of covering prose. Phones keep the existing bottom sheet. - **One composer**: new comments now use the reply field pattern: a field with the round send button inside, Enter submits, Escape cancels. The duplicate Cancel and the Comment button are gone; the header × is the single close. - **Author row**: threads, the grouped list from #300 and the hover preview use Chat's author row: `Face` avatar, display name, quiet timestamp. `@handle` stays in the accessible name.
## Screenshots
Before (#300 build), desktop 1440×900, thread: card covers the passage 
After, thread opens below its passage 
Before, new comment 
After, new comment uses the reply field pattern 
Before, grouped list 
After, grouped list 
After, wide document (chat and sidebar hidden): card in the gutter 
Phone 390×844, before / after (sheet unchanged, author row updated)  
## Testing
- `bun test`: 3675 pass, 0 fail (new geometry tests, including a sweep asserting the card never overlaps a visible passage). - `bun run types`: pass. - `bun run ci`: dprint, oxlint and tokens pass; fails only on the design-contract review hash below. - E2E updated, not run locally (local E2E suspended for this run): `e2e/responsive-comments.e2e.ts` (card never covers its passage, marker lane uncovered, draft has no Cancel, Escape cancels, Enter submits, author face) and `e2e/sidecar.e2e.ts` (wide gutter placement, below-passage placement at 1024). Relying on CI's browser-integration job.
## Needs a design-contract review
- `packages/editor/src/comment-layer.tsx`, pinned twice in `scripts/design-contract/exceptions/dynamic-editor.json` (comment overlay style; motion/presence classes). New hash: `db127842206a29d6eb733356a77095b22ac618f648e01dcd72955f134a46a25d`. Safe: the reviewed `surface.style` still carries measured geometry only (now `top`/`left`/`width`/`maxHeight` from `commentCardPoint`), the only new attribute is a finite `data-side` (right/below/above), and no class logic changed. Note the base #300 is already off this pin (its file hashes to `68ce9788…`).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
</details>
anamentionedlast month on GitHub
anasubscribedlast month on GitHub
- MA
**Review:** Changes needed
Checked at 1024, 1440 (chat on and off), 1920 and 390 with a headless Playwright run against this branch. CI: only `format, lint, types, tests` fails, and only on the reviewed-owner hashes in `comment-layer.tsx` (acceptable per the audit rules); container and e2e pass.
**Works well** - The card never covered its passage in any state I tried. It sits below the passage's left edge at 1024, 1440 and 1920 (chat on). With chat hidden at 1920 it sits in the right gutter beside the first line; at 1440 with chat hidden the gutter is under 264px, so it goes below. Flip-above and clamping when the passage scrolls away behave and don't jump between frames. - Keyboard: focus lands in the new-comment field on open, Enter posts, Shift+Enter inserts a newline (`"Line one\nline two"`), and replies behave the same way. Escape from the draft or the thread returns focus to the marker, including from inside the reply field. - Names: dialog "Comments", send is "Post comment" / "Send reply", and the author row reads `Octocat (@octocat)` for screen readers, with a single "Close comment". - The author row (Face, display name, quiet timestamp) is consistent across thread, grouped list and hover preview.
**Changes needed** 1. Phone draft has no visible way to cancel (regression). `comments.tsx` `DraftCard` dropped the Cancel button, and in the compact sheet the only close button is `sr-only` (`showClose=false`). At 390 the draft is just a lone icon, the field and the send arrow. Touch users can only dismiss by dragging or tapping the backdrop, and there is no Escape. Either show the × in the sheet's draft header, or keep Cancel when `!showClose`. The header row is also an empty strip with a stray message icon in the compact sheet; drop the row there. 2. Entrance motion is not origin-aware. `styles.css` `plan-comment-enter` is still `translateX(0.375rem)` plus opacity 0.92, measured as 6px sliding in from the right, with almost no fade, whichever side the card lands on. Recommend `commentCardPoint` return `side: "right" | "below" | "above"` and the layer set `data-side` on the card. Then use a ~120–150ms ease-out fade from opacity 0 (so there is a real fade) with a 4px translate from the anchor direction: `translateY(-4px)` below the passage, `translateY(4px)` above it, `translateX(4px)` in the gutter. Use custom properties (`--enter-x`, `--enter-y`) so there is one keyframe, and keep the existing `prefers-reduced-motion` `animation: none`. Judgement: the current motion is subtle enough to be harmless, but it contradicts the placement logic, so it should change before this ships. 3. `capitalize` is inlined in `Author` (`comments.tsx`) as `handle[0]!.toUpperCase() + handle.slice(1)` and duplicates `capitalize` in `apps/web/src/chat/model.ts:31`. The editor cannot import from the web app, but `apps/web` already depends on `@chopin/editor` (it imports `AgentFace`, and `Face` lives here). Export one helper (for example `displayName(handle)`) from the editor next to `Face`, and have chat import it, so the two surfaces cannot drift. A shared package is overkill. Moving the whole author row into the editor and having the chat transcript use `Author` would be better still. 4. Smaller points. - Enter-to-send has no `event.isComposing` guard, so confirming an IME candidate sends the comment. Add `if (event.nativeEvent.isComposing) return;`. - On phones the soft keyboard has no Shift+Enter, so a multi-line comment cannot be written. This was already true of replies but now covers new comments too. Consider treating Enter as a newline when `matchMedia("(pointer: coarse)")` matches and relying on the send button. - `commentCardPoint` defaults `lane = 40`, while the layer passes `MARKER_LANE = 56`. The geometry tests only exercise 40. Either default to 56 or add a test with 56.
Screenshots are in the audit `shots` folder from my run (/tmp/r301).
- MA
Thanks. All six points are addressed in 8bddcdb6.
1. **Phone draft close.** The compact draft sheet now shows a visible × (44px, `Close comment`) in the grabber row through `Drawer.Close`, so it uses the sheet's own exit animation. The draft has no header row in the sheet anymore. Thread and list sheets keep their sr-only close.  2. **Entrance from the passage.** `commentCardPoint` returns `side: "right" | "below" | "above"` and the card carries `data-side`. There is one keyframe: opacity from 0 plus `translate(var(--enter-x), var(--enter-y))`, over `--duration-fast` (120ms) with `--ease-out`. The offset is 0.25rem: up when the card sits below, down when it sits above, from the right when it sits in the gutter. `transform-origin` follows the side too. The `prefers-reduced-motion` rule is unchanged. The frames below were captured at 0.1× speed (opacity runs 0 → 0.73 → 0.95 → 1; the translate eases from 4px to 0): Below:  Gutter:  Above:  3. **One name helper.** `displayName` lives in `packages/editor/src/display-name.ts` and is exported from `@chopin/editor`. The comment `Author` row uses it. In `apps/web/src/chat/model.ts` the only change is the import (`import { displayName as capitalize }`) plus re-exporting the existing name. I put it in its own file rather than `face.tsx` because `face.tsx` has a design-contract pin. 4. **IME.** A new `composerKey()` helper never sends while `isComposing` (or keyCode 229, matching Chat). It applies to both new comments and replies. 5. **Coarse pointers.** When the primary pointer is coarse, Enter inserts a newline and only the send button sends. This also applies to both composers. Unit tests cover it, and the phone e2e now checks that Enter leaves `"…\n"` in the field before sending with the button. 6. **Marker lane.** `lane` defaults to 56. The gutter tests now use 56, and a new test checks that a gutter card starts at `column.right + 56` and falls back to below when only a 40px lane would have fitted.
The e2e specs now also assert `data-side` (`below` at 1440, `right` in the wide sidecar case). The design-contract hash for `comment-layer.tsx` is updated in the PR body. `bun test` (editor and web), `bun run types` and `bun run fix` pass locally. CI is running.
lavaman131referenced 219816789803last month on GitHub
MaggieAppletonbase-ref-force-pushedlast month on GitHub
MaggieAppletonhead-ref-force-pushed fc51eab1393blast month on GitHub
MaggieAppletonbase-ref-force-pushedlast month on GitHub
MaggieAppletonhead-ref-force-pushed 8308c1cc47eflast month on GitHub
MaggieAppletonautomatic-base-change-succeededlast month on GitHub
MaggieAppletonhead-ref-force-pushed bcffae0d1496last month on GitHub
MaggieAppletonmerged edfe0bb77f70last month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub
MaggieAppletonreferenced this in Show decision evidence as a summary row with objections #361last month on GitHub