Skip to content

Size the orphaned comments popover against the document, not its pill - #511

Merged
MaggieAppleton merged 2 commits into
mainfrom
design-orphan-comments-popover
Oct 12, 2026
Merged

MaggieAppleton merged 2 commits into
mainfrom
design-orphan-comments-popover

Conversation

@MaggieAppleton

@MaggieAppleton MaggieAppleton commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Clicking N comment(s) without prose opened a ~14px sliver under the pill — only the top of an avatar showed.

Root cause. The orphan card reuses .plan-comment-card, whose width: min(20rem, calc(100% - 1.5rem)) and max-height: calc(100% - 1rem) resolve against the containing block. For anchored cards that is the full-document comment layer, but for the orphan card it was .plan-comment-orphans — the pill's own absolutely positioned wrapper. So the card was about the pill's width minus 24px, and its height was the pill's height minus 16px.

Fix (CSS only). The wrapper now stretches to the layer's height (bottom: 0.5rem) and stops taking pointer events itself (the pill and card still do). The orphan card takes width: min(20rem, calc(100cqi - 1rem)) from the document's inline-size container and max-height: calc(100% - 2.5rem), so it ends 0.5rem above the document's bottom edge and scrolls when there are many. It stays right-aligned to the pill. Escape and outside-click dismissal are unchanged. The phone sheet uses a different path and already worked. Header layout is untouched (see #507).

flowchart LR
  L[".plan-comment-layer (document)"] --> W[".plan-comment-orphans<br/>before: pill-sized<br/>after: full height"]
  W --> B["pill button"]
  W --> C[".plan-comment-orphan-card<br/>% sizes resolve against W"]
Loading
Before After
Before: one orphaned comment After: one orphaned comment
Before: narrow, four orphaned After: narrow, four orphaned

Short window with four orphaned comments: the card is bounded by the document and scrolls.

After: short viewport, many

Phone (sheet path, unchanged):

Phone sheet

Tests. A new e2e test in e2e/sidecar.e2e.ts opens the popover at 1280×800 and checks the following: the quote and the Reply field are fully in view, the card does not overflow, it is at least 280px wide, it sits below the pill, it is right-aligned to the pill, and Escape returns focus to the pill. On main this test fails at the Reply visibility check. With the fix, all four orphan tests pass locally. bun run types passes.

There is no app dark theme, so I only checked light mode.

🤖 Generated with Claude Code

@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Review: Looks good

I checked this in a production build against the fake server on port 8827, using my own headless Playwright script. The test documents had 6 orphaned comments, and one had 1 orphan plus anchored comments.

What I verified

  • Clicks pass through the stretched wrapper. The wrapper has pointer-events: none. With the popover closed, elementFromPoint returned the prose at 9 sample points down the wrapper's column, never the wrapper. Real clicks put the caret in the editor, double-clicks selected a word, and hover reached the text under the column in the root, narrow (820px) and Lift child layouts. With the popover open and 1 orphan, points below the card hit prose.
  • 100cqi sizes against .plan-document. The comment layer is a direct child of .plan-document, the nearest inline-size container, and sits outside the [data-plan-scroll] container. The card was 320px wide and right-aligned to the pill. Its bottom edge sat at document bottom − 8px in every layout: root (1440×900), root expanded, narrow 820×700, short 1280×520, Lift child, and Zoom child (expanded).
  • Scrolling stays inside the card. With 6 comments it is 774px tall over 1637px of content. A wheel over the card scrolled the card by 400px and the document did not move.
  • Stacking order is right. Points inside the card hit card content, including where a margin chip sits beneath it. Hovering the table leaves the card open and on top.
  • Keyboard works. Enter on the pill opens the card. Tab moves into it (More actions, Resolve, Reply). Escape closes it and puts focus back on the pill.
  • Phone is unchanged. At 390×844 the bottom sheet opens full width as before.
  • The diff is tidy. It is two CSS rules and one e2e test, and the test fails on main.

Non-blocking

  1. This predates the PR. The fixed plan-formatting-toolbar (z-50) stays up when a comment card opens over a live selection, and anchored cards do the same. Now that the orphan card is full size, the toolbar can cover its quote and Reply field: select a word near the right edge, then click the pill. A possible follow-up is to hide the toolbar while a comment surface is open, or blur the selection when one opens.
  2. In packages/editor/src/styles.css, .plan-comment-orphan-card's max-height: calc(100% - 2.5rem) repeats the top: 2.5rem offset. A shared custom property would keep the two from drifting apart. This is optional.

CI: container passed, format/lint/types/tests passed, e2e passed.

MaggieAppleton and others added 2 commits October 12, 2026 11:48
The orphaned comments card shares `.plan-comment-card`, whose width and
max-height are percentages of the containing block. Its containing block was
the pill's own absolutely positioned wrapper, so the card resolved to roughly
the pill's width and less than its height, showing only a sliver of the first
avatar. The wrapper now spans the layer's height, pointer-transparent, and the
card takes its width from the document's inline-size container.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A selection's fixed formatting toolbar stayed above an open orphan or
anchored comment card and covered it. The orphan card's offset is now one
custom property shared by its top and max-height.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaggieAppleton
MaggieAppleton force-pushed the design-orphan-comments-popover branch from 342eb7a to f0000c1 Compare October 12, 2026 10:48
@MaggieAppleton
MaggieAppleton merged commit 472522a into main Oct 12, 2026
3 checks passed
@MaggieAppleton
MaggieAppleton deleted the design-orphan-comments-popover branch October 12, 2026 11:07
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