Review UI: collapsible identity-aware comments rail, clean dialogs, editor spacing fixes#8
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… chips (attn-d7y)
The review right rail reserved a fixed 320px whenever the panel was open,
leaving a nearly vacant column when the margin held only resolved threads —
a single full-width dashed strip whose click just moved the cursor and
never showed the resolved content.
- New pure rail-mode derivation (closed/slim/full) in review/rail-mode.ts,
exposed as reviewStore.railMode: resolved-only margins slim the aside to
a 48px gutter of icon chips; the document reclaims the width.
- Resolved strips become content-sized chips ("✓ author · resolved" in the
full rail, icon-only in the gutter). Clicking a chip expands it in place
to the full read-only ReviewMarginCard (resolved badge, quote, body,
replies) with a Collapse button; Escape collapses too.
- Unified the active-card and resolved-chip collision layout into one
layoutCards pass, fixing the pre-existing overlap between the two
independent passes and letting the expanded resolved card push its
neighbors.
- locallyDismissed moved from ReviewMargin into the store so the rail
slims the same tick the last active card is resolved (UI-only Reject now
survives a panel remount for the session — intended).
- Resolved threads always get an anchor-Y (fallback 0) so the slim gutter
never renders empty when positions can't resolve.
- Tests: rail-mode.test.ts (new), margin-integration cases for the unified
pass/chip pitch/visibility rule, and a hard E2E suite in
test-review-e2e.sh (slim → expand → collapse → mixed) seeded through
window.__attn_review_store__. Design doc §3 amended.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The share-link and name-prompt dialogs opened glitchily: a zoom+fade keyframe entry scaled the dialog while it appeared (tw-animate keyframes also have two documented WKWebView failure modes in this app), the 50%/ translate centering re-centered the whole dialog whenever its height changed, and ShareDialog roughly doubled in height when ShareReady swapped minting → ready. - dialog-content/dialog-overlay: entry is now a pure opacity fade via a CSS transition + @starting-style (starting:opacity-0) — no keyframes, so it can't strand the dialog invisible and bits-ui still unmounts instantly on close. Content is anchored at top-[14vh] (translate-x centering only) with max-h-[80vh] + scroll, so late-arriving content grows the dialog downward instead of re-centering it. - ShareDialog: minting and ready render the same rows — the command row swaps "Minting room…" for the npx command in place, the Direct-link card is present from open with a placeholder row, and the copy buttons enable when ready. Verified in the running app: rect identical (top 101, height 572) across the minting → ready transition. The share-invite-url hidden input stays gated on ready — E2E scripts use its appearance as the share-ready signal. - app.css: dropped the now-dead entry-animation workaround (its keyframes are gone); kept and re-documented the exit-animation unmount fix for the surfaces that still use animate-out (sheet, command palette). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…anner (attn-42y) Four user-reported UI fixes plus the findings of an adversarial review pass over the diff. Rail (always-collapsible, differentiated chrome): - rail-mode.ts reworked to hidden/collapsed/expanded: in a review room the rail is always at least a 48px gutter; the ReviewBar dock gains a panel toggle (mirrors Cmd+J); auto-expand fires only for UNRESOLVED feedback, one-shot per room. Aside gets bg-sidebar + border; card slots inset 12px so cards no longer touch the window edge; cards are fluid-width. Collapsed gutter renders author-avatar chips for unresolved threads and ✓ chips for resolved ones (clamped below the dock, virtualized past 50), each expanding the rail onto its thread. - Removed the in-card Collapse button: clicking the read-only resolved card (guarded against text-selection drags) or Escape shrinks it back to its chip; collapsing the rail clears expansion + manual-reanchor state, and both window-capture key handlers are gated off while collapsed (they otherwise survived invisibly, swallowing Escape and confirming reanchors). Identity (names + colors): - Rust: share() now announces the owner via a self ParticipantJoined (display name) on fresh mint AND re-Share (backfills legacy rooms, refreshes renames) — owner-authored comments no longer degrade to a raw participant id. Frontend displayNameFor adds an owner fallback (profile name on the owner window, 'Owner' elsewhere) and room-scopes participantNames; new memoized participantKinds map. - Cards carry the author's presence color (left border + 16px monogram avatar) via --peer-avatar-bg-* tokens, matching carets and peer chips; tokens darkened so white monograms pass WCAG AA 4.5:1. - Clicking an inline mark now expands a collapsed rail. Editor spacing: - prosemirror.css gains the upstream img.ProseMirror-separator guard: Tailwind preflight + base.css image rules were styling ProseMirror's zero-size cursor shim as a block image with 1rem margins, inflating every paragraph ending in a suggest-changes pilcrow by ~57px — the "huge gap on Enter" bug. Layout: - The shared-document banner is now a snippet rendered above a new content+rail row in SidebarInset, spanning the full window width with the rail starting beneath it; reviewChrome moved into the row so the ReviewBar keeps its breadcrumb alignment. Breadcrumb right-inset is rail-width aware. Orphan tray clears the floating dock. Verified: svelte-check 0 errors, 32/32 web test files, 413 cargo review tests, scripts/test-review-e2e.sh 35 PASS / 0 FAIL (new collapsed/ expand/avatar/toggle assertions + screenshots), binary-size gate 30.21 of 32 MiB. The editorial relay E2E fails identically on the clean tree (pre-existing). CLAUDE.md dev:collab join description corrected (daemon-routed, not --as-agent). Follow-ups filed: participantId init plumbing for resumed-room owner names; ReviewBar dock width tuning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The margin computed anchor positions from view.coordsAtPos (viewport- relative) but listened for scrolls on its own aside — an element that never scrolls, since the rail is a SIBLING of the editor's scroll container. Cards were positioned once on store changes and then froze while the text moved underneath them, drifting away from the comments' anchor text immediately on scroll. - ReviewMargin now tracks the EDITOR's scroll viewport (found from view.dom, robust to layout changes) and recomputes positions per scroll tick, plus a ResizeObserver on the editor DOM for typing- induced reflows. Cards and gutter chips follow their anchors 1:1, Google-Docs style; off-screen anchors clip out of view with their text (collapsed chips keep position-less orphans pinned below the dock, but scrolled-past anchors pass through negative). - The aside is overflow-hidden: the document scroll is the single source of vertical movement, so the rail can never desync by scrolling independently. Sticky bottom elements (resolved pill, reanchor overlay) became absolute; the orphan tray is plain flow. - Dropped the rail width transition: WebKit pauses CSS transitions in occluded windows, which left the rail stuck mid-transition at ~1px (caught because every E2E screenshot went stale); width changes are instant and the E2E now asserts exact widths (48/320) instead of a ≤60 band that masked the frozen state. - E2E: new scroll-tracking suite — scrolls the editor viewport 150px, asserts the card's on-screen top moves 1:1 (±4px) and returns to its anchor on scroll-back. 39 PASS / 0 FAIL. Verified visually in a live window: cards at y 237/664 moved to -13/414 after a 250px scroll, landing beside the exact text they annotate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eader (attn-z46) Three pieces of user feedback on the previous rounds: - Dialogs are viewport-centered again (top-50%/translate-y-50%). The top-[14vh] anchor from the dialog-glitch fix read as "not centered"; centering is safe now because dialogs render their final structure from open (ShareDialog's minting → ready swap is height-stable). Verified: dialog center == window center exactly. - ShareDialog right padding no longer collapses on narrow windows: the unbreakable font-mono file path drove the dialog's grid track wider than its padding box (overflow-x-hidden then clipped every w-full child flush to the border). The path now break-alls. Verified: 24px/24px symmetric padding. - The comments rail starts at the underside of the header strip (mt-12) instead of running the full window height, and owns its toggle in a rail-header row — centered in the collapsed gutter (was the off-center dock button, now removed from ReviewBar), right-aligned when expanded. With the dock no longer overlapping the rail, the chip clearance dropped 56 → 8 and became viewport-aware: chips whose anchors are still on screen (including the header band above the rail) pin into the gutter; only anchors scrolled above the window clip away with their text. Verified: rail top 47px, toggle/chip centered ±1px; E2E 40 PASS / 0 FAIL including a new toggle-centering assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… dismissal, rail padding (attn-2aj) - Resolved chips carry the thread author's presence color: border + 16px monogram avatar on the labeled variant; the gutter icon variant takes the color via a CSS custom property so its hover foreground flip still works. - New shared composer-keys helper: plain Enter submits, Shift/Alt+Enter inserts a newline, Cmd/Ctrl+Enter keeps working. IME-safe for the engine we actually ship on: WebKit fires the candidate-confirm Enter AFTER compositionend with isComposing=false, so the helper also bails on keyCode 229 (blocker caught by the adversarial review pass — the isComposing-only guard would have submitted half-composed CJK text). Auto-repeat Enter is rejected, and both composers gained an in-flight submitting flag so duplicates can't fire once IPC gains real acks. - All authoring textareas (margin reply, comment composer, suggestion replacement/insert/note): rows=4 minimum, resize handle removed. The three-way apply editor drops its resize handle too but keeps Enter as newline (it edits multi-line replacement text). - SuggestionComposer no longer dead-keys Enter while the form is incomplete (gate hoisted above preventDefault so it falls through to a newline); CommentComposer regained dialog-level Cmd+Enter parity; Escape during IME composition no longer wipes drafts. - The Comment|Suggest selection toolbar dismisses after a comment or suggestion is CREATED: a submit-only callback collapses the editor selection (the load-bearing part — toolbar state alone gets re-derived from the still-live selection) and is guarded against a mid-compose file switch dispatching into the wrong editor view. Cancel/Escape keep the selection for a retry. - Rail breathing room: cards/chips whose anchors are on screen clamp 8px below the rail header instead of sitting flush against it; anchors scrolled above the window still clip away 1:1 with their text. E2E re-anchored its scroll-tracking thread deeper to stay out of the pinning band; 41 PASS / 0 FAIL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
angusbezzina
force-pushed
the
angus/ui-tweaks
branch
from
ad9a263 to
b00fc2e
Compare
The update nudge ("attn X is available") and other toasts had no
dismiss affordance — users had to wait out the 12s timeout. sonner's
closeButton renders an ✕ on every toast.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three rounds of review-mode UI work:
Comments rail (attn-d7y → attn-42y)
--peer-avatar-bg-*tokens (darkened to pass WCAG AA for white monograms), matching carets and peer chips.share()now announces the owner via a selfParticipantJoined(fresh mint and re-Share), so owner-authored comments resolve to a real display name on every window instead of a raw participant id; frontend gains owner-name fallbacks and room-scoped name/kind maps.Dialogs (attn-0sv)
@starting-style(no more zoom keyframes with their two documented WKWebView failure modes), are top-anchored so async content growth never re-centers them, and ShareDialog renders its final structure from open (minting → ready swaps in place; rect verified pixel-identical).Editor spacing (attn-42y)
img.ProseMirror-separatorguard in prosemirror.css: Tailwind preflight + content-image rules were styling ProseMirror's zero-size cursor shim as a block image with 1rem margins, inflating paragraphs ending in suggest-changes pilcrows by ~57px (the "huge gap on Enter" bug).Layout (attn-42y)
An adversarial multi-agent review pass over the diff surfaced 16 findings (4 bug-class, all fixed in-branch; 2 polish items deferred to beads attn-* follow-ups).
Verification
scripts/test-review-e2e.sh: 35 PASS / 0 FAIL (new collapsed/expand/avatar/toggle suites + screenshots)🤖 Generated with Claude Code