From ad9cc78f007fa2445207ac1175f40ee1f29e11a5 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Wed, 1 Jul 2026 03:51:40 +0300 Subject: [PATCH] fix(#268): don't open an empty hover card; align flip height estimate (F1,F2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1: gate the card on rows-WITH-text (`thread.some(row => row.text.length > 0)`) instead of thread length. A text-less root whose only reply is also text-less would otherwise open an empty (the render already filters empty rows). New test locks it (parent + reply both empty → no card). - F2: ESTIMATED_CARD_HEIGHT 200 -> 300 (= CARD_MAX_HEIGHT) so the flip-above decision reserves the real worst-case height and a tall thread near the viewport bottom flips up instead of overflowing off-screen. vitest 19/19, tsc 0, eslint 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/comment-hover-preview.test.tsx | 27 +++++++++++++++++++ .../components/comment-hover-preview.tsx | 13 +++++---- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/apps/client/src/features/comment/components/comment-hover-preview.test.tsx b/apps/client/src/features/comment/components/comment-hover-preview.test.tsx index 6f7289e8..235fc141 100644 --- a/apps/client/src/features/comment/components/comment-hover-preview.test.tsx +++ b/apps/client/src/features/comment/components/comment-hover-preview.test.tsx @@ -268,6 +268,33 @@ describe("CommentHoverPreview — hover behaviour", () => { expect(card.textContent).toContain("Bob: A reply"); }); + it("shows nothing when neither the parent nor its reply has any text", () => { + // The card is gated on rows-with-text (not thread length), so a text-less + // root whose only reply is also text-less must NOT open an empty card. + const emptyDoc = JSON.stringify({ type: "doc", content: [] }); + setComments([ + comment({ + id: "c-1", + content: emptyDoc, + creator: { id: "u-1", name: "Alice", avatarUrl: null } as any, + }), + comment({ + id: "c-2", + content: emptyDoc, + parentCommentId: "c-1", + createdAt: new Date(), + creator: { id: "u-2", name: "Bob", avatarUrl: null } as any, + }), + ]); + render(); + + hoverMark(); + act(() => { + vi.advanceTimersByTime(350); + }); + expect(screen.queryByTestId("comment-hover-preview")).toBeNull(); + }); + it("hides on mouseout", () => { setComments([comment()]); render(); diff --git a/apps/client/src/features/comment/components/comment-hover-preview.tsx b/apps/client/src/features/comment/components/comment-hover-preview.tsx index 5f68f5eb..7ebcc066 100644 --- a/apps/client/src/features/comment/components/comment-hover-preview.tsx +++ b/apps/client/src/features/comment/components/comment-hover-preview.tsx @@ -18,7 +18,10 @@ const CARD_MAX_WIDTH = 360; const CARD_MAX_HEIGHT = 300; const GAP = 6; // Reserve roughly this much room below the span; flip above when it doesn't fit. -const ESTIMATED_CARD_HEIGHT = 200; +// Match CARD_MAX_HEIGHT so the flip-above decision reserves the real worst-case +// height — otherwise a tall thread placed below near the viewport bottom passes +// the "fits below" check and then overflows off-screen (clipped, no scroll). +const ESTIMATED_CARD_HEIGHT = 300; // One rendered line of the thread: the author and the comment's plain text, // pre-computed at hover time so render stays cheap. Shown as "Author: text". @@ -140,10 +143,10 @@ export default function CommentHoverPreview({ if (span === activeSpanRef.current) return; const thread = buildThread(commentMapRef.current, comment); - // Show the card when the root has text OR it has at least one reply. - // A thread of a single empty-text root carries nothing worth showing. - const hasContent = - thread.length > 1 || thread.some((row) => row.text.length > 0); + // Show the card only when SOME comment has text. Gating on thread length + // could open an empty card (a text-less root whose only reply is also + // text-less), since the render filters out empty-text rows. + const hasContent = thread.some((row) => row.text.length > 0); if (!hasContent) return; activeSpanRef.current = span;