Skip to content

Open a comment card under its marker with Resolve in view - #543

Merged
MaggieAppleton merged 6 commits into
mainfrom
design-comment-card-resolve
Oct 13, 2026
Merged

MaggieAppleton merged 6 commits into
mainfrom
design-comment-card-resolve

Conversation

@MaggieAppleton

@MaggieAppleton MaggieAppleton commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Resolving a comment was laborious: clicking a margin marker opened the card at the passage's left edge, across the page from the marker, and its Resolve check only appeared after pointing at the card.

  • Card under its marker. When the right gutter can't hold the card (chat open, ~1100px, split document), it now opens below the passage (or flips above near the viewport bottom) right-aligned to the prose column, directly under the marker. Its entrance origin moves to the right corner to match. The wide-gutter "right" placement, the orphaned-comments card's placement (Size the orphaned comments popover against the document, not its pill #511) and the phone sheet's placement are unchanged.
  • Resolve always visible. The check shows whenever a card is open, by click or keyboard, including each thread in the orphaned-comments card. More actions (…) keeps its hover/focus reveal (still always shown on touch).
  • New-comment composer stays by the selection. A draft has no marker yet, so commentCardPoint takes align: "passage" and the composer keeps to the selection's left edge (inside the column), as before.
  • Long threads stay in view. Unfolding "Show N replies" used to grow the card past the document because its height was measured only when placed. The card now observes its content, is re-placed for the new height, and caps to the room it has (also in the wide gutter), scrolling within itself with the reply box reachable.

The hover preview already sits beside the marker, flush with the column edge, so preview and clicked card now open on the same side.

Before / after (1440, chat open)

Before: card at left, Resolve hidden
After: card right-aligned under marker, Resolve visible

Several threads in one paragraph, table cell, near the bottom (flips above), phone

Two threads in one paragraph
Table cell
Near the viewport bottom flips above
Phone sheet keeps Resolve

Draft composer by a left-side selection; unfolded long thread scrolls inside the viewport

Draft beside selection
Long thread unfolded, reply reachable

Tests

  • comment-geometry.test.ts: below/above cards right-align to the column wherever the passage starts.
  • sidecar.e2e.ts (narrow split): card's right edge within 2px of the paragraph's right edge after click; Resolve fully opaque without hovering the card.
  • comment-sheet.e2e.ts: a desktop card that unfolds a 9-note thread stays inside a 640px viewport, scrolls internally, and its reply box is reachable.
  • comment-geometry.test.ts: drafts align to the passage; a tall gutter card is capped.
  • bun test packages/editor, bun run types, design contract, design check, dprint, oxlint pass locally. Local e2e skipped (shared ports held by other sessions); relying on CI.

🤖 Generated with Claude Code

A clicked comment card that cannot sit in the right gutter now opens below
(or above) its passage right-aligned to the prose column, beneath the marker
that raised it, instead of at the passage's left edge across the page.
Resolve stays visible whenever a card is open; the More actions menu keeps
its hover and focus reveal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Review: Looks good

I checked this in a real browser on a fake-GitHub server, with a heading thread of 8 replies, a table-cell thread and a thread on a low paragraph.

Verified

  • 1440, chat open: the card is below the passage, its right edge exactly on the column edge (1372), just under the marker (1380). The marker is never covered. The hover preview has the same right edge (1084–1372), so the preview and the clicked card are on the same side.
  • 1440, document expanded: the card is still below and right-aligned to the column (r1170, marker at 1178). At 1440 the gutter is about 200px, below the 264px minimum, so this case does not reach the wide-gutter placement.
  • 1920, expanded (wide gutter): right placement is unchanged. The card is at x1466, beside the marker (r1452), with the preview next to it.
  • 1100 and 900: the card is right-aligned under the marker at both widths. At 900 it clears a heading that wraps onto two lines.
  • Near the viewport bottom: the card flips above (b827 over the passage at 833) and stays right-aligned. In the table cell it sits below the row and stays right-aligned.
  • Resolve: fully opaque (computed product of opacities = 1) whenever the card is open, opened by mouse or by keyboard. ⋯ still appears on hover or focus.
  • Keyboard: Enter on the marker opens the card. Esc closes it and returns focus to the marker.
  • Phone 390×844: the sheet keeps Resolve and More as 44px targets.
  • Entrance motion: the frame sequence grows out of the top-right corner, which reads as coming from the marker.
  • Size the orphaned comments popover against the document, not its pill #511 toolbar rule: the formatting toolbar shows while text is selected and hides while a card is open.

Notes (non-blocking)

  1. The new-comment draft moves too. comment-layer.tsx:1397 places the draft with the same cardPlacement, so the "Add a comment" composer is now right-aligned as well. If you select a few words at the left of a paragraph ("Filler paragraph" at x818), the composer opens at x1052–1372, about 230px from the selection. The description doesn't mention this. It fits where the thread card will appear once posted, but it partly brings back the "far from the trigger" problem for drafts. Either keep it on purpose and mention it in the description, or add a placement option such as align: "passage" | "column" to commentCardPoint so drafts keep aligning to the passage.
  2. Orphan card (Size the orphaned comments popover against the document, not its pill #511): Actions is shared, so every entry in the orphaned-comments card now shows its Resolve check all the time instead of only on hover. This is consistent with the change, but it is a visible change to Size the orphaned comments popover against the document, not its pill #511, which the description says is unchanged. I couldn't seed orphans locally to look at it.
  3. Older bug, not caused by this PR: at 1100×700, choosing "Show 7 replies" grows the card to about 988px. It runs past the bottom of the document with no internal scroll, so the reply box is out of view. The height cap only uses the measured height from opening, and this PR doesn't touch that logic. Worth a separate issue.

The code is minimal, uses tokens and follows conventions. The geometry unit tests and the e2e assertion (card right edge within 2px, Resolve opacity 1) cover the change.

CI: all green (format/lint/types/tests, e2e, container, CLI, CLI Windows).

MaggieAppleton and others added 5 commits October 13, 2026 09:52
A draft has no marker to open under, so commentCardPoint takes an align
option and the draft composer keeps to the selection's left edge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Unfolding a long thread grew the card past the document because its height
was measured only when placed. The card now observes its content, is
re-placed for the new height, and caps to the room it has so it scrolls
within itself with the reply box reachable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reviewed dynamic owners in comment-layer.tsx keep the same data flow:
PreviewSurface and CommentSurface still take style and className from their
placed values and presence lifecycle, and hit rectangles are unchanged.
CommentSurface only adds a ref and a ResizeObserver that re-run the
surface's own onMeasure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaggieAppleton
MaggieAppleton merged commit 4e9a850 into main Oct 13, 2026
@MaggieAppleton
MaggieAppleton deleted the design-comment-card-resolve branch October 13, 2026 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant