mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
fix(ui): address reply-UI review — author labels, tombstone id, copy
- formatReplyAuthorName resolves the current viewer vs. other users instead of labeling every non-agent target "You"; passes the profile map through the live thread's quote (real names) and currentUserId through the comment thread. A reply to another human now shows their name/"User", not "You". - Reply chip seeding in the comment thread uses the same resolver, so replying to another user no longer mislabels the chip "Replying to You". - Tombstone detection falls back to message.id when a deleted message has no explicit custom.commentId, matching the render-time id so its replies still show "Original comment deleted". - Drop instance-local ticket ids from the UX lab copy and code comments. - Extend comment-reply unit tests for the current-user / other-user / board label cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
dfe62783b9
commit
70f3363dce
6 files changed
+100
-22
No files matched your search
@@ -387,15 +387,17 @@ function ReplyButton({ onClick }: { onClick: () => void }) {
|
||||
function ReplyQuote({
|
||||
replyTo,
|
||||
agentMap,
|
||||
currentUserId,
|
||||
targetDeleted,
|
||||
onClick,
|
||||
}: {
|
||||
replyTo: IssueCommentReplyToMetadata;
|
||||
agentMap?: Map<string, Agent>;
|
||||
currentUserId?: string | null;
|
||||
targetDeleted: boolean;
|
||||
onClick?: () => void;
|
||||
}) {
|
||||
const authorName = formatReplyAuthorName(replyTo, agentMap);
|
||||
const authorName = formatReplyAuthorName(replyTo, { agentMap, currentUserId });
|
||||
const body = (
|
||||
<span className="flex min-w-0 items-stretch gap-2">
|
||||
<span aria-hidden className="w-0.5 shrink-0 rounded-full bg-primary/40" />
|
||||
@@ -434,6 +436,7 @@ function ReplyQuote({
|
||||
function CommentCard({
|
||||
comment,
|
||||
agentMap,
|
||||
currentUserId,
|
||||
companyId,
|
||||
projectId,
|
||||
feedbackVote = null,
|
||||
@@ -450,6 +453,7 @@ function CommentCard({
|
||||
}: {
|
||||
comment: CommentWithRunMeta;
|
||||
agentMap?: Map<string, Agent>;
|
||||
currentUserId?: string | null;
|
||||
companyId?: string | null;
|
||||
projectId?: string | null;
|
||||
feedbackVote?: FeedbackVoteValue | null;
|
||||
@@ -548,6 +552,7 @@ function CommentCard({
|
||||
<ReplyQuote
|
||||
replyTo={replyTo}
|
||||
agentMap={agentMap}
|
||||
currentUserId={currentUserId}
|
||||
targetDeleted={replyTargetDeleted}
|
||||
onClick={
|
||||
onScrollToComment && !replyTargetDeleted
|
||||
@@ -863,6 +868,7 @@ const TimelineList = memo(function TimelineList({
|
||||
key={comment.id}
|
||||
comment={comment}
|
||||
agentMap={agentMap}
|
||||
currentUserId={currentUserId}
|
||||
companyId={companyId}
|
||||
projectId={projectId}
|
||||
feedbackVote={feedbackVoteByTargetId?.get(comment.id) ?? null}
|
||||
@@ -1026,13 +1032,18 @@ export function CommentThread({
|
||||
const handleReply = useCallback(
|
||||
(comment: CommentWithRunMeta) => {
|
||||
const { excerpt, truncated } = buildReplyExcerpt(comment.body);
|
||||
const authorName = comment.authorAgentId
|
||||
? agentMap?.get(comment.authorAgentId)?.name ?? comment.authorAgentId.slice(0, 8)
|
||||
: "You";
|
||||
const authorName = formatReplyAuthorName(
|
||||
{
|
||||
authorType: comment.authorType,
|
||||
authorAgentId: comment.authorAgentId,
|
||||
authorUserId: comment.authorUserId,
|
||||
},
|
||||
{ agentMap, currentUserId },
|
||||
);
|
||||
setReplyingTo({ commentId: comment.id, authorName, excerpt, excerptTruncated: truncated });
|
||||
editorRef.current?.focus();
|
||||
},
|
||||
[agentMap],
|
||||
[agentMap, currentUserId],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -1224,6 +1235,7 @@ export function CommentThread({
|
||||
key={comment.id}
|
||||
comment={comment}
|
||||
agentMap={agentMap}
|
||||
currentUserId={currentUserId}
|
||||
companyId={companyId}
|
||||
projectId={projectId}
|
||||
highlightCommentId={highlightCommentId}
|
||||
|
||||
@@ -863,17 +863,21 @@ function IssueChatReplyButton({ onClick }: { onClick: () => void }) {
|
||||
function IssueChatReplyQuote({
|
||||
replyTo,
|
||||
agentMap,
|
||||
currentUserId,
|
||||
userProfileMap,
|
||||
targetDeleted,
|
||||
onAccent,
|
||||
onClick,
|
||||
}: {
|
||||
replyTo: IssueCommentReplyToMetadata;
|
||||
agentMap?: Map<string, Agent>;
|
||||
currentUserId?: string | null;
|
||||
userProfileMap?: ReadonlyMap<string, CompanyUserProfile> | null;
|
||||
targetDeleted: boolean;
|
||||
onAccent?: boolean;
|
||||
onClick?: () => void;
|
||||
}) {
|
||||
const authorName = formatReplyAuthorName(replyTo, agentMap);
|
||||
const authorName = formatReplyAuthorName(replyTo, { agentMap, currentUserId, userProfileMap });
|
||||
const barClass = onAccent ? "bg-white/60" : "bg-primary/40";
|
||||
const authorClass = onAccent ? "text-white" : "text-foreground";
|
||||
const excerptClass = onAccent ? "text-white/80" : "text-muted-foreground";
|
||||
@@ -1692,6 +1696,8 @@ function IssueChatUserMessage({
|
||||
<IssueChatReplyQuote
|
||||
replyTo={replyTo}
|
||||
agentMap={agentMap}
|
||||
currentUserId={currentUserId}
|
||||
userProfileMap={userProfileMap}
|
||||
targetDeleted={deletedCommentIds?.has(replyTo.commentId) ?? false}
|
||||
onAccent={isCurrentUser && !queued}
|
||||
onClick={onScrollToComment ? () => onScrollToComment(replyTo.commentId) : undefined}
|
||||
@@ -1835,6 +1841,8 @@ function IssueChatAssistantMessage({
|
||||
stoppingRunLabel = "Stopping...",
|
||||
stopRunVariant = "stop",
|
||||
runFinalizationActions = [],
|
||||
currentUserId,
|
||||
userProfileMap,
|
||||
onReply,
|
||||
onScrollToComment,
|
||||
highlightCommentId,
|
||||
@@ -2089,6 +2097,8 @@ function IssueChatAssistantMessage({
|
||||
<IssueChatReplyQuote
|
||||
replyTo={replyTo}
|
||||
agentMap={agentMap}
|
||||
currentUserId={currentUserId}
|
||||
userProfileMap={userProfileMap}
|
||||
targetDeleted={deletedCommentIds?.has(replyTo.commentId) ?? false}
|
||||
onClick={onScrollToComment ? () => onScrollToComment(replyTo.commentId) : undefined}
|
||||
/>
|
||||
@@ -4739,7 +4749,7 @@ export function IssueChatThread({
|
||||
return true;
|
||||
}
|
||||
|
||||
// --- Reply-to-comment composer state (P3, PAP-13802) ---
|
||||
// --- Reply-to-comment composer state ---
|
||||
const [replyingTo, setReplyingTo] = useState<ReplyDraftTarget | null>(null);
|
||||
const [highlightCommentId, setHighlightCommentId] = useState<string | null>(null);
|
||||
const highlightTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
@@ -4749,8 +4759,11 @@ export function IssueChatThread({
|
||||
const ids = new Set<string>();
|
||||
for (const message of messages) {
|
||||
const custom = message.metadata?.custom as Record<string, unknown> | undefined;
|
||||
if (custom?.deletedAt && typeof custom.commentId === "string") {
|
||||
ids.add(custom.commentId);
|
||||
if (custom?.deletedAt) {
|
||||
// Match the comment-id fallback used when rendering the message, so a deleted source
|
||||
// without an explicit custom.commentId still marks its replies as tombstoned.
|
||||
const commentId = typeof custom.commentId === "string" ? custom.commentId : message.id;
|
||||
ids.add(commentId);
|
||||
}
|
||||
}
|
||||
return ids;
|
||||
|
||||
@@ -339,7 +339,7 @@ export const issueChatUxReviewEvents: IssueTimelineEvent[] = [
|
||||
];
|
||||
|
||||
/**
|
||||
* Reply-to-comment (PAP-13802 P3) scenario: a sent reply with a quoted block, a reply whose
|
||||
* Reply-to-comment scenario: a sent reply with a quoted block, a reply whose
|
||||
* source was deleted (tombstone), and the source comments they point back to. Seed
|
||||
* {@link issueChatUxReplyDraftKey} in localStorage to also render the composer reply chip.
|
||||
*/
|
||||
|
||||
@@ -96,21 +96,59 @@ describe("formatReplyAuthorName", () => {
|
||||
|
||||
it("uses the agent name when the target is an agent in the map", () => {
|
||||
expect(
|
||||
formatReplyAuthorName({ authorType: "agent", authorAgentId: "agent-1" }, agentMap),
|
||||
formatReplyAuthorName({ authorType: "agent", authorAgentId: "agent-1", authorUserId: null }, { agentMap }),
|
||||
).toBe("CodexCoder");
|
||||
});
|
||||
|
||||
it("falls back to a short agent id when the agent is unknown", () => {
|
||||
expect(
|
||||
formatReplyAuthorName({ authorType: "agent", authorAgentId: "agent-abcdef123" }, agentMap),
|
||||
formatReplyAuthorName(
|
||||
{ authorType: "agent", authorAgentId: "agent-abcdef123", authorUserId: null },
|
||||
{ agentMap },
|
||||
),
|
||||
).toBe("agent-ab");
|
||||
});
|
||||
|
||||
it("labels system authors as System", () => {
|
||||
expect(formatReplyAuthorName({ authorType: "system", authorAgentId: null })).toBe("System");
|
||||
expect(
|
||||
formatReplyAuthorName({ authorType: "system", authorAgentId: null, authorUserId: null }),
|
||||
).toBe("System");
|
||||
});
|
||||
|
||||
it("labels user authors as You", () => {
|
||||
expect(formatReplyAuthorName({ authorType: "user", authorAgentId: null })).toBe("You");
|
||||
it("labels the current user's own comment as You", () => {
|
||||
expect(
|
||||
formatReplyAuthorName(
|
||||
{ authorType: "user", authorAgentId: null, authorUserId: "user-1" },
|
||||
{ currentUserId: "user-1" },
|
||||
),
|
||||
).toBe("You");
|
||||
});
|
||||
|
||||
it("resolves another user's profile label instead of mislabeling them You", () => {
|
||||
const userProfileMap = new Map([["user-2", { label: "Ada Lovelace", image: null }]]);
|
||||
expect(
|
||||
formatReplyAuthorName(
|
||||
{ authorType: "user", authorAgentId: null, authorUserId: "user-2" },
|
||||
{ currentUserId: "user-1", userProfileMap },
|
||||
),
|
||||
).toBe("Ada Lovelace");
|
||||
});
|
||||
|
||||
it("labels another user without a profile as User (never You)", () => {
|
||||
expect(
|
||||
formatReplyAuthorName(
|
||||
{ authorType: "user", authorAgentId: null, authorUserId: "user-2" },
|
||||
{ currentUserId: "user-1" },
|
||||
),
|
||||
).toBe("User");
|
||||
});
|
||||
|
||||
it("labels the board principal as Board", () => {
|
||||
expect(
|
||||
formatReplyAuthorName(
|
||||
{ authorType: "user", authorAgentId: null, authorUserId: "local-board" },
|
||||
{ currentUserId: "user-1" },
|
||||
),
|
||||
).toBe("Board");
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { Agent, IssueComment, IssueCommentReplyToMetadata } from "@paperclipai/shared";
|
||||
import type { CompanyUserProfile } from "./company-members";
|
||||
|
||||
/**
|
||||
* Max characters shown in a reply excerpt (composer chip + sent quoted header).
|
||||
@@ -32,16 +33,30 @@ export function buildOptimisticReplyTo(target: IssueComment): IssueCommentReplyT
|
||||
}
|
||||
|
||||
/**
|
||||
* Human-readable author label for a reply target. Mirrors {@link CommentCard}'s author rendering:
|
||||
* agent comments show the agent name, everything else collapses to "You".
|
||||
* Human-readable author label for a reply target. Mirrors the comment bubble's own author
|
||||
* rendering: agent comments show the agent name; a user comment shows "You" only when it is the
|
||||
* current viewer's, otherwise the resolved profile label (or "Board"/"User" when no profile is
|
||||
* available). Resolving the current user matters in multi-user threads, where every non-current
|
||||
* user would otherwise be mislabeled "You".
|
||||
*/
|
||||
export function formatReplyAuthorName(
|
||||
replyTo: Pick<IssueCommentReplyToMetadata, "authorType" | "authorAgentId">,
|
||||
agentMap?: Map<string, Agent>,
|
||||
replyTo: Pick<IssueCommentReplyToMetadata, "authorType" | "authorAgentId" | "authorUserId">,
|
||||
options: {
|
||||
agentMap?: Map<string, Agent> | null;
|
||||
currentUserId?: string | null;
|
||||
userProfileMap?: ReadonlyMap<string, CompanyUserProfile> | null;
|
||||
} = {},
|
||||
): string {
|
||||
if (replyTo.authorAgentId) {
|
||||
return agentMap?.get(replyTo.authorAgentId)?.name ?? replyTo.authorAgentId.slice(0, 8);
|
||||
return options.agentMap?.get(replyTo.authorAgentId)?.name ?? replyTo.authorAgentId.slice(0, 8);
|
||||
}
|
||||
if (replyTo.authorType === "system") return "System";
|
||||
return "You";
|
||||
const authorUserId = replyTo.authorUserId ?? null;
|
||||
if (authorUserId && options.currentUserId && authorUserId === options.currentUserId) {
|
||||
return "You";
|
||||
}
|
||||
const profileLabel = authorUserId ? options.userProfileMap?.get(authorUserId)?.label?.trim() : undefined;
|
||||
if (profileLabel) return profileLabel;
|
||||
if (authorUserId === "local-board") return "Board";
|
||||
return "User";
|
||||
}
|
||||
@@ -320,7 +320,7 @@ export function IssueChatUxLab() {
|
||||
id="reply-to-comment"
|
||||
eyebrow="Reply to comment"
|
||||
title="Reply chip, quoted block, and tombstone"
|
||||
description="PAP-13802 P3: hover a bubble to reveal Reply, the composer shows a quoted/clamped source chip (click to jump to the original, X or Escape to clear), sent replies render a quoted header above the body, and a reply whose source was deleted falls back to an 'Original comment deleted' tombstone."
|
||||
description="Hover a bubble to reveal Reply, the composer shows a quoted/clamped source chip (click to jump to the original, X or Escape to clear), sent replies render a quoted header above the body, and a reply whose source was deleted falls back to an 'Original comment deleted' tombstone."
|
||||
accentClassName="bg-[linear-gradient(180deg,rgba(16,185,129,0.06),transparent_28%),var(--background)]"
|
||||
>
|
||||
<IssueChatThread
|
||||
|
||||
Reference in new issue
Block a user