Skip to content

bd-bpt089zw: resolving the last comment no longer leaves an empty pill or a stuck glow (q2-preview) - #666

Merged
cscheid merged 4 commits into
mainfrom
bugfix/bd-bpt089zw-q2-preview-comments-resolving
Sep 9, 2026
Merged

bd-bpt089zw: resolving the last comment no longer leaves an empty pill or a stuck glow (q2-preview)#666
cscheid merged 4 commits into
mainfrom
bugfix/bd-bpt089zw-q2-preview-comments-resolving

Conversation

@cscheid

@cscheid cscheid commented Sep 9, 2026

Copy link
Copy Markdown
Member

Summary

In hub-client's q2-preview, resolving the last comment on a block (the in the comment bubble) left two artifacts behind: a small empty pill where the bubble was, and a block "glow" that never cleared — not after moving the mouse away, not after clicking elsewhere.

Both are state-lifecycle bugs in CommentWrapper (ts-packages/preview-renderer/src/q2-preview/custom/CommentBlock.tsx). Blocks are keyed by index, so the bubble component instance and its state survive the commit round-trip, and nothing reset it:

  1. Empty pill. selfExpanded outlived the last comment, so the bubble rendered its expanded branch with zero rows. Fix: a layout effect collapses the self-expanded state when the comment count reaches zero with no inline input open (the one legitimate empty-and-expanded state: + just clicked). Layout, so the pill is never painted.
  2. Stuck glow. The block-wrapper glow was set from the bubble's own onMouseEnter/onMouseLeave. The resolve re-render unmounts the button under the pointer; Chrome re-evaluates :hover after layout without boundary events, and the next mouseout comes from the new hovered node, which the bubble is not an ancestor of — so the bubble's leave never fired. Fix: derive the glow on the wrapper's mousemove (containment of e.target) and clear it on the wrapper's mouseleave; a layout effect re-derives it from geometry (last pointer position vs the re-measured bubble rect) whenever the bubble changes shape, so a stationary pointer gets the right answer in both directions.

Both fields date from Comments v1 (#441); not a regression from the rich-bubble work.

Tests

CommentBlock.resolveLast.integration.test.tsx — 8 jsdom tests (T1–T7 + T6b), each verified failing before the fix (T7 against the clear-only variant of the geometric re-check). Full preview-renderer integration (56 files) and unit (43 files) suites green; cargo xtask verify green.

End-to-end

Verified in Chrome against a local vite dev server: after with the pointer stationary, the chrome collapses to the hover-only + and the glow is on only because the pointer actually rests inside that +; after a real move away, wrapper box-shadow is none and no chrome remains. DOM probe output is recorded in the plan.

Plan: claude-notes/plans/2026-09-09-preview-comment-resolve-artifacts.md · Strand: bd-bpt089zw

🤖 Generated with Claude Code

https://claude.ai/code/session_01AwSEXztTeiRxA2NMhud9pe

cscheid and others added 4 commits September 9, 2026 08:47
…w (plan)

Repro'd in Chrome against a local hub-client dev server; both artifacts are
state-lifecycle bugs in CommentWrapper (selfExpanded outlives the last
comment; bubbleHovered's mouseleave is lost when the resolve re-render
removes the button under the pointer). Plan doc records the DOM probes and
proposes a TDD fix; no code changes yet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AwSEXztTeiRxA2NMhud9pe
…ered ok, changelog yes)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AwSEXztTeiRxA2NMhud9pe
…l or a stuck glow

Two state-lifecycle bugs in CommentWrapper (q2-preview comment bubbles),
both surfacing after the ✓ (Resolve) click removes a block's last comment
and the commit round-trip re-renders the index-keyed, hence preserved,
bubble component:

1. `selfExpanded` outlived the last comment, so the bubble rendered its
   expanded branch with zero rows — a bordered, empty pill. A layout
   effect now collapses the self-expanded state when the comment count
   reaches zero with no inline input open (the one legitimate
   empty-and-expanded state); layout so the pill is never painted.

2. The block-wrapper glow (`bubbleHovered`) was driven by the bubble's own
   onMouseEnter/onMouseLeave. The resolve re-render unmounts the button
   under the pointer; Chrome re-evaluates :hover after layout without
   boundary events and the next mouseout comes from the NEW hovered node,
   which the bubble is not an ancestor of, so the leave never fired and
   the glow stuck until a genuine enter/leave of the bubble. The glow is
   now derived on the wrapper's mousemove (containment of e.target) and
   cleared on the wrapper's mouseleave, and a layout effect re-derives it
   from geometry (last pointer position vs the re-measured bubble rect)
   whenever the bubble changes shape, so a stationary pointer gets the
   right answer in both directions.

Tests: CommentBlock.resolveLast.integration.test.tsx (T1–T7 + T6b), each
verified failing before the fix (T7 against the clear-only variant of the
geometric re-check). Verified end-to-end in Chrome against a local vite
dev server; `cargo xtask verify` green.

Plan: claude-notes/plans/2026-09-09-preview-comment-resolve-artifacts.md

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AwSEXztTeiRxA2NMhud9pe
@posit-snyk-bot

posit-snyk-bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@cscheid
cscheid merged commit 3ecabd2 into main Sep 9, 2026
10 checks passed
@cscheid
cscheid deleted the bugfix/bd-bpt089zw-q2-preview-comments-resolving branch September 9, 2026 15:04
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.

2 participants