diff --git a/semantic_code_review/viewer/assets/comments.ts b/semantic_code_review/viewer/assets/comments.ts index 0fe08df..e0d7b09 100644 --- a/semantic_code_review/viewer/assets/comments.ts +++ b/semantic_code_review/viewer/assets/comments.ts @@ -562,6 +562,10 @@ interface EntryActions { onEdit: () => void; onDelete: () => void; onSend: () => void; + /** Set on the thread's last entry alone: Reply belongs to the thread, + * and sits in one row with the entry's own buttons rather than under + * a rule of its own. */ + onReply?: () => void; } function _buildEntry(c: ReviewerComment, isReply: boolean, actions: EntryActions): HTMLElement { @@ -578,8 +582,8 @@ function _buildEntry(c: ReviewerComment, isReply: boolean, actions: EntryActions if (header) entry.appendChild(header); entry.appendChild(_buildEntryBody(c)); + const bar = _el("div", "comment-actions"); if (!_isIngested(c)) { - const bar = _el("div", "comment-actions"); if (_counterpart === "claude") { // The lifecycle chrome: where the comment stands towards Claude, // and Send while it is a draft. The badge sits first so the state @@ -603,8 +607,13 @@ function _buildEntry(c: ReviewerComment, isReply: boolean, actions: EntryActions bar.appendChild(delBtn); editBtn.addEventListener("click", (e) => { e.stopPropagation(); actions.onEdit(); }); delBtn.addEventListener("click", (e) => { e.stopPropagation(); actions.onDelete(); }); - entry.appendChild(bar); } + if (actions.onReply) { + const reply = _el("button", "comment-btn comment-btn-reply", "Reply"); + reply.addEventListener("click", (e) => { e.stopPropagation(); actions.onReply!(); }); + bar.appendChild(reply); + } + if (bar.childElementCount) entry.appendChild(bar); return entry; } @@ -650,7 +659,20 @@ function _buildThreadRow( } if (expanded) { + // A reply is how the reviewer follows up: on an ingested thread it + // nests on GitHub; with Claude as the counterpart it is an ordinary + // draft they Send, delivered as `reply`. It sits on the last entry's + // row of buttons. + const canReply = ingestedThread || _counterpart === "claude"; + const onReply = (): void => { + handle?.remove(); + _openEditor({ + rowEl: anchorRowEl, side: anchor.side, line: anchor.line, + file: anchor.file, replyTo: replyTarget, + }); + }; thread.entries.forEach((c, idx) => { + const last = idx === thread.entries.length - 1; const entry = _buildEntry(c, idx > 0, { onEdit: () => { handle?.remove(); @@ -661,27 +683,10 @@ function _buildThreadRow( }, onDelete: () => _store.delete(c.id).then(() => { refresh(); _onChange?.(); }), onSend: () => _store.send(c.id).then(() => { refresh(); _onChange?.(); }), + onReply: canReply && last ? onReply : undefined, }); container.appendChild(entry); }); - - // A reply is how the reviewer follows up: on an ingested thread it - // nests on GitHub; with Claude as the counterpart it is an ordinary - // draft they Send, delivered as `reply`. - if (ingestedThread || _counterpart === "claude") { - const actions = _el("div", "comment-thread-actions"); - const reply = _el("button", "comment-btn comment-btn-reply", "Reply"); - reply.addEventListener("click", (e) => { - e.stopPropagation(); - handle?.remove(); - _openEditor({ - rowEl: anchorRowEl, side: anchor.side, line: anchor.line, - file: anchor.file, replyTo: replyTarget, - }); - }); - actions.appendChild(reply); - container.appendChild(actions); - } } handle = Annotations.attach({ diff --git a/semantic_code_review/viewer/assets/viewer.css b/semantic_code_review/viewer/assets/viewer.css index 84dfcf3..9d4eb45 100644 --- a/semantic_code_review/viewer/assets/viewer.css +++ b/semantic_code_review/viewer/assets/viewer.css @@ -1287,15 +1287,6 @@ html.dragging-divider { cursor: col-resize; user-select: none; } * the divider between entries is enough. (The .comment-thread-reply * class still attaches for any future treatment that wants to target * replies specifically.) */ -.comment-thread-actions { - display: flex; - justify-content: flex-end; - gap: 6px; - margin-top: 4px; - padding-top: 6px; - border-top: 1px solid var(--border); -} -.comment-btn-reply { padding: 2px 10px; } /* Smell pills that are click-to-promote get a subtle hover cue — * pointer cursor + a hint underline. Non-promotable smells (the @@ -1459,7 +1450,9 @@ html.dragging-divider { cursor: col-resize; user-select: none; } } .comment-badge-draft { border-style: dashed; } .comment-badge-resend { border-style: dashed; border-color: var(--warn); color: var(--fg); } -.comment-badge-sent { border-style: solid; border-color: var(--fg-muted); } +/* One border colour across badge and buttons: a brighter border reads + * as a taller box. Draft dashed, sent solid, delivered filled. */ +.comment-badge-sent { border-style: solid; } .comment-badge-delivered { border-style: solid; border-color: var(--ok); background: var(--ok); color: #fff; } diff --git a/tests/js/viewer.test.ts b/tests/js/viewer.test.ts index 3354105..c699363 100644 --- a/tests/js/viewer.test.ts +++ b/tests/js/viewer.test.ts @@ -6035,11 +6035,17 @@ describe("comment lifecycle (ADR 0009)", () => { expect(reply.classList.contains("comment-thread-reply")).toBe(true); expect(reply.querySelector(".comment-author")!.textContent).toBe("claude"); expect(reply.querySelector(".comment-body")!.textContent).toBe("Fixed in 3f2a."); - expect(reply.querySelector(".comment-actions")).toBeNull(); + // Read-only: no edit, delete, Send or badge of its own. The one button + // on its row is the thread's Reply — the reviewer's follow-up is a + // reply they Send — which sits on the last entry's row, not under a + // rule of its own. + expect(reply.querySelector(".comment-btn-edit, .comment-btn-del, .comment-btn-send")).toBeNull(); expect(reply.querySelector(".comment-badge")).toBeNull(); - // The reviewer's follow-up is a reply they Send: the thread offers Reply. + expect(Array.from(reply.querySelectorAll(".comment-actions > *")).map((b) => b.className)) + .toEqual(["comment-btn comment-btn-reply"]); const thread = reply.closest(".comment-thread")!; - expect(thread.querySelector(".comment-btn-reply")).not.toBeNull(); + expect(thread.querySelector(".comment-thread-actions")).toBeNull(); + expect(thread.querySelectorAll(".comment-btn-reply").length).toBe(1); // The root keeps its own chrome. expect(badgeOf("c1")).toBe("delivered"); });