Compare commits

...

1 Commits

Author SHA1 Message Date
Naiyuan Qing
cc286d85f6 fix(issues): make comment highlight background-only 2026-07-01 15:59:03 +08:00
2 changed files with 194 additions and 116 deletions

View File

@@ -46,6 +46,8 @@ import { deriveThreadResolution } from "./thread-utils";
const highlightedCommentBackgroundClass =
"bg-[color-mix(in_srgb,var(--card)_95%,var(--brand)_5%)]";
const stickyHeaderFadeClass =
"after:pointer-events-none after:absolute after:inset-x-0 after:top-full after:h-1 after:bg-[inherit] after:[mask-image:linear-gradient(to_bottom,#000,transparent)] after:[-webkit-mask-image:linear-gradient(to_bottom,#000,transparent)]";
function StickyHeaderShell({
className,
@@ -59,19 +61,23 @@ function StickyHeaderShell({
children: ReactNode;
}) {
if (!sticky) {
return <div className={className}>{children}</div>;
return (
<div className={cn(highlighted && highlightedCommentBackgroundClass, className)}>
{children}
</div>
);
}
return (
<div
className={cn(
"sticky top-0 z-10 transition-colors duration-700 after:pointer-events-none after:absolute after:inset-x-0 after:top-full after:h-1 after:bg-[inherit] after:[mask-image:linear-gradient(to_bottom,#000,transparent)] after:[-webkit-mask-image:linear-gradient(to_bottom,#000,transparent)]",
"sticky top-0 z-10 transition-colors duration-700",
!highlighted && stickyHeaderFadeClass,
highlighted ? highlightedCommentBackgroundClass : "bg-card",
className,
)}
>
<div className={className}>
{children}
</div>
{children}
</div>
);
}
@@ -510,7 +516,8 @@ function CommentRow({
{/* Header pins to the timeline's scroll parent within this reply's own
row box, so a LONG reply keeps its
author + actions visible while you scroll its body, then releases once
this reply ends. bg-card occludes the body scrolling underneath. */}
this reply ends. The header stays opaque and matches this comment's
highlight state while it occludes the body scrolling underneath. */}
<StickyHeaderShell
highlighted={isHighlighted}
className="flex items-center gap-2.5 px-4 pt-1 pb-1.5"
@@ -760,7 +767,7 @@ function CommentCardImpl({
// overflow-clip (not -hidden) clips the rounded corners WITHOUT creating a
// scroll container, so the sticky collapse affordances below resolve to the
// timeline's scroll parent instead of this card. See PR #3623.
<Card className={cn("!py-0 !gap-0 overflow-clip transition-colors duration-700", isHighlighted && "ring-2 ring-brand/50", isHighlighted && highlightedCommentBackgroundClass)}>
<Card className="!py-0 !gap-0 overflow-clip transition-colors duration-700">
{onCollapseResolved && (
<button
type="button"
@@ -779,112 +786,112 @@ function CommentCardImpl({
That is what keeps exactly one header pinned at a time: without this
wrapper the header's containing block is the whole thread and it
stays stuck behind every reply. */}
<div>
{/* Header — always visible, acts as toggle */}
<StickyHeaderShell
sticky={stickyHeader}
highlighted={isHighlighted}
className="px-4 py-3"
>
<div className="flex items-center gap-2.5">
<CollapsibleTrigger className="shrink-0 rounded p-0.5 text-muted-foreground hover:bg-muted hover:text-foreground transition-colors">
<ChevronRight className={cn("h-3.5 w-3.5 transition-transform", open && "rotate-90")} />
</CollapsibleTrigger>
<ActorAvatar actorType={entry.actor_type} actorId={entry.actor_id} size={24} enableHoverCard showStatusDot />
<span className="shrink-0 cursor-pointer text-sm font-medium">
{getActorName(entry.actor_type, entry.actor_id)}
</span>
<Tooltip>
<TooltipTrigger
render={
<span className="shrink-0 text-xs text-muted-foreground cursor-default">
{timeAgo(entry.created_at)}
</span>
}
/>
<TooltipContent side="top">
{new Date(entry.created_at).toLocaleString()}
</TooltipContent>
</Tooltip>
{!open && contentPreview && (
<span className="min-w-0 flex-1 truncate text-xs text-muted-foreground">
{contentPreview}
<div className={cn("transition-colors duration-700", isHighlighted && highlightedCommentBackgroundClass)}>
{/* Header — always visible, acts as toggle */}
<StickyHeaderShell
sticky={stickyHeader}
highlighted={isHighlighted}
className="px-4 py-3"
>
<div className="flex items-center gap-2.5">
<CollapsibleTrigger className="shrink-0 rounded p-0.5 text-muted-foreground hover:bg-muted hover:text-foreground transition-colors">
<ChevronRight className={cn("h-3.5 w-3.5 transition-transform", open && "rotate-90")} />
</CollapsibleTrigger>
<ActorAvatar actorType={entry.actor_type} actorId={entry.actor_id} size={24} enableHoverCard showStatusDot />
<span className="shrink-0 cursor-pointer text-sm font-medium">
{getActorName(entry.actor_type, entry.actor_id)}
</span>
)}
{!open && replyCount > 0 && (
<span className="shrink-0 text-xs text-muted-foreground">
{t(($) => $.comment.reply_count, { count: replyCount })}
</span>
)}
{open && (
<div className="ml-auto flex items-center gap-0.5">
<DropdownMenu>
<DropdownMenuTrigger
<Tooltip>
<TooltipTrigger
render={
<Button variant="ghost" size="icon-sm" className="text-muted-foreground">
<MoreHorizontal className="h-4 w-4" />
</Button>
<span className="shrink-0 text-xs text-muted-foreground cursor-default">
{timeAgo(entry.created_at)}
</span>
}
/>
<DropdownMenuContent align="end">
<DropdownMenuItem onClick={() => {
void copyText(entry.content ?? "").then((ok) => {
if (ok) toast.success(t(($) => $.comment.copied_toast));
});
}}>
<Copy className="h-3.5 w-3.5" />
{t(($) => $.comment.copy_action)}
</DropdownMenuItem>
{onResolveToggle && (
<>
<DropdownMenuSeparator />
<DropdownMenuItem onClick={() => onResolveToggle(entry.id, !entry.resolved_at)}>
{entry.resolved_at ? (
<>
<RotateCcw className="h-3.5 w-3.5" />
{t(($) => $.comment.resolve.unresolve_thread_action)}
</>
) : (
<>
<CheckCircle2 className="h-3.5 w-3.5" />
{t(($) => $.comment.resolve.resolve_thread_action)}
</>
)}
<TooltipContent side="top">
{new Date(entry.created_at).toLocaleString()}
</TooltipContent>
</Tooltip>
{!open && contentPreview && (
<span className="min-w-0 flex-1 truncate text-xs text-muted-foreground">
{contentPreview}
</span>
)}
{!open && replyCount > 0 && (
<span className="shrink-0 text-xs text-muted-foreground">
{t(($) => $.comment.reply_count, { count: replyCount })}
</span>
)}
{open && (
<div className="ml-auto flex items-center gap-0.5">
<DropdownMenu>
<DropdownMenuTrigger
render={
<Button variant="ghost" size="icon-sm" className="text-muted-foreground">
<MoreHorizontal className="h-4 w-4" />
</Button>
}
/>
<DropdownMenuContent align="end">
<DropdownMenuItem onClick={() => {
void copyText(entry.content ?? "").then((ok) => {
if (ok) toast.success(t(($) => $.comment.copied_toast));
});
}}>
<Copy className="h-3.5 w-3.5" />
{t(($) => $.comment.copy_action)}
</DropdownMenuItem>
</>
)}
{(canEditEntry || canDeleteEntry) && (
<>
<DropdownMenuSeparator />
{canEditEntry && (
<DropdownMenuItem onClick={edit.startEdit}>
<Pencil className="h-3.5 w-3.5" />
{t(($) => $.comment.edit_action)}
</DropdownMenuItem>
{onResolveToggle && (
<>
<DropdownMenuSeparator />
<DropdownMenuItem onClick={() => onResolveToggle(entry.id, !entry.resolved_at)}>
{entry.resolved_at ? (
<>
<RotateCcw className="h-3.5 w-3.5" />
{t(($) => $.comment.resolve.unresolve_thread_action)}
</>
) : (
<>
<CheckCircle2 className="h-3.5 w-3.5" />
{t(($) => $.comment.resolve.resolve_thread_action)}
</>
)}
</DropdownMenuItem>
</>
)}
{canEditEntry && canDeleteEntry && <DropdownMenuSeparator />}
{canDeleteEntry && (
<DropdownMenuItem onClick={() => setConfirmDelete(true)} variant="destructive">
<Trash2 className="h-3.5 w-3.5" />
{t(($) => $.comment.delete_action)}
</DropdownMenuItem>
{(canEditEntry || canDeleteEntry) && (
<>
<DropdownMenuSeparator />
{canEditEntry && (
<DropdownMenuItem onClick={edit.startEdit}>
<Pencil className="h-3.5 w-3.5" />
{t(($) => $.comment.edit_action)}
</DropdownMenuItem>
)}
{canEditEntry && canDeleteEntry && <DropdownMenuSeparator />}
{canDeleteEntry && (
<DropdownMenuItem onClick={() => setConfirmDelete(true)} variant="destructive">
<Trash2 className="h-3.5 w-3.5" />
{t(($) => $.comment.delete_action)}
</DropdownMenuItem>
)}
</>
)}
</>
)}
</DropdownMenuContent>
</DropdownMenu>
<DeleteCommentDialog
open={confirmDelete}
onOpenChange={setConfirmDelete}
onConfirm={() => onDelete(entry.id)}
hasReplies
/>
</div>
)}
</div>
</StickyHeaderShell>
</DropdownMenuContent>
</DropdownMenu>
<DeleteCommentDialog
open={confirmDelete}
onOpenChange={setConfirmDelete}
onConfirm={() => onDelete(entry.id)}
hasReplies
/>
</div>
)}
</div>
</StickyHeaderShell>
{/* Collapsible body */}
<CollapsibleContent>
@@ -992,7 +999,13 @@ function CommentCardImpl({
</div>
)}
{resolutionReply && (
<div id={`comment-${resolutionReply.id}`} className={cn("border-t border-border/50 transition-colors duration-700", highlightedCommentId === resolutionReply.id && highlightedCommentBackgroundClass)}>
<div
id={`comment-${resolutionReply.id}`}
className={cn(
"border-t border-border/50 transition-colors duration-700",
highlightedCommentId === resolutionReply.id && highlightedCommentBackgroundClass,
)}
>
<CommentRow
issueId={issueId}
entry={resolutionReply}
@@ -1024,7 +1037,14 @@ function CommentCardImpl({
)}
{/* Replies — chronological; the resolution keeps its place with a badge */}
{allNestedReplies.map((reply) => (
<div key={reply.id} id={`comment-${reply.id}`} className={cn("border-t border-border/50 transition-colors duration-700", highlightedCommentId === reply.id && highlightedCommentBackgroundClass)}>
<div
key={reply.id}
id={`comment-${reply.id}`}
className={cn(
"border-t border-border/50 transition-colors duration-700",
highlightedCommentId === reply.id && highlightedCommentBackgroundClass,
)}
>
<CommentRow
issueId={issueId}
entry={reply}

View File

@@ -311,8 +311,9 @@ vi.mock("@multica/core/issues/stores", () => ({
// scrollIntoView (it drives the timeline container's scrollTop directly to
// avoid scrolling ancestor overflow:hidden boxes — see issue-detail.tsx). We
// keep a no-op stub on the prototype so any stray scrollIntoView call from
// other components doesn't throw; deep-link tests assert the highlight ring
// instead, which is mechanism-independent and observable without layout.
// other components doesn't throw; deep-link tests assert the highlight
// background instead, which is mechanism-independent and observable without
// layout.
const scrollIntoViewSpy = vi.hoisted(() => vi.fn());
vi.mock("react-virtuoso", () => ({
@@ -491,6 +492,21 @@ function renderIssueDetailWithHighlight(
return { ...result, queryClient };
}
const highlightedCommentBackgroundClass =
"bg-[color-mix(in_srgb,var(--card)_95%,var(--brand)_5%)]";
function hasHighlightedCommentBackground(root: ParentNode | null): boolean {
if (!root) return false;
const elements = root instanceof Element
? [root, ...Array.from(root.querySelectorAll("[class]"))]
: Array.from(root.querySelectorAll("[class]"));
return elements.some(
(el) => typeof el.className === "string" && el.className.includes(highlightedCommentBackgroundClass),
);
}
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
@@ -1112,14 +1128,56 @@ describe("IssueDetail (shared)", () => {
// The deep-link effect lands on AND highlights the target comment: it
// drives the timeline container's scrollTop directly (jsdom has no
// layout, so the scroll itself isn't observable here) and applies the
// brand highlight ring. Assert the user-facing highlight.
// brand highlight background. Assert the user-facing highlight.
await waitFor(() => {
expect(
document.getElementById("comment-comment-2")?.querySelector(".ring-2"),
).not.toBeNull();
hasHighlightedCommentBackground(document.getElementById("comment-comment-2")),
).toBe(true);
});
});
it("highlights only the target root comment, not the whole thread", async () => {
mockApiObj.listTimeline.mockResolvedValue([
{
type: "comment",
id: "comment-root",
actor_type: "member",
actor_id: "user-1",
content: "Root target",
parent_id: null,
created_at: "2026-01-18T00:00:00Z",
updated_at: "2026-01-18T00:00:00Z",
comment_type: "comment",
} as TimelineEntry,
{
type: "comment",
id: "reply-under-root",
actor_type: "member",
actor_id: "user-1",
content: "Reply should stay neutral",
parent_id: "comment-root",
created_at: "2026-01-18T01:00:00Z",
updated_at: "2026-01-18T01:00:00Z",
comment_type: "comment",
} as TimelineEntry,
]);
renderIssueDetailWithHighlight("comment-root");
await waitFor(() => {
expect(document.getElementById("comment-comment-root")).not.toBeNull();
});
await waitFor(() => {
expect(
hasHighlightedCommentBackground(document.getElementById("comment-comment-root")),
).toBe(true);
});
const reply = document.getElementById("comment-reply-under-root");
expect(reply).not.toBeNull();
expect(hasHighlightedCommentBackground(reply)).toBe(false);
});
it("still scrolls when the timeline is ready before the issue (regression for inbox click)", async () => {
// Reproduces the inbox-click race: timeline data is in the cache
// before the issue resolves. While loading is true, IssueDetail
@@ -1138,7 +1196,7 @@ describe("IssueDetail (shared)", () => {
document.getElementById("comment-comment-2"),
).toBeNull();
// Nothing highlighted while the loading skeleton is up.
expect(document.querySelector(".ring-2")).toBeNull();
expect(hasHighlightedCommentBackground(document)).toBe(false);
resolveIssue(mockIssue);
@@ -1149,8 +1207,8 @@ describe("IssueDetail (shared)", () => {
});
await waitFor(() => {
expect(
document.getElementById("comment-comment-2")?.querySelector(".ring-2"),
).not.toBeNull();
hasHighlightedCommentBackground(document.getElementById("comment-comment-2")),
).toBe(true);
});
});