fix(#268): don't open an empty hover card; align flip height estimate (F1,F2)
- 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 <Paper> (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) <noreply@anthropic.com>
This commit is contained in:
@@ -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(<Harness />);
|
||||
|
||||
hoverMark();
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(350);
|
||||
});
|
||||
expect(screen.queryByTestId("comment-hover-preview")).toBeNull();
|
||||
});
|
||||
|
||||
it("hides on mouseout", () => {
|
||||
setComments([comment()]);
|
||||
render(<Harness />);
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user