Skip to content

Commit a705668

Browse files
fix(canvas): correct comment Build links and metadata (#28)
<!-- Use a conventional commit-style title, such as "fix(hub): retain prototype history". --> ## Why <!-- Explain the problem and why this change matters in a short, plain-English paragraph. Link the issue when applicable. --> Comment Build links used a relative Hub path inside the separately hosted canvas. Clicking one reached the canvas server's JSON 404 instead of the Build page. This affects canvases connected to local or deployed Hubs. ## What changes <!-- Describe what becomes different for a first-time reader. Explain necessary terms. Include accurate Mermaid diagrams when they clarify the proposal, using before/after diagrams when helpful. Add screenshots for meaningful UI changes. --> <!-- Omit validation sections, checklists, and routine test/CI summaries such as "CI is green" or "tests passed". Mention only known verification gaps that affect the change, not disclaimers about drafting this description. --> Canvas comment links now open the original Build on the selected Hub in a separate tab. Earlier Build links appear in parentheses immediately after the author's name, with muted text and an underline only on hover. The shared Hub comment row uses the same layout and keeps its existing relative navigation. Compact avatars and unread dots share a fixed left gutter. Each dot sits below its avatar, vertically centered against the comment text. Author names, comment bodies, and reply counts align beside the gutter, giving the body more room while retaining its three-line limit. Read comments keep the same positions, with muted author and body text and regular-weight names. The wider hover background includes the gutter. Remove the confusing “Checking” badge as requested. It described a pending attempt to locate the comment's target in the preview, not whether the comment was saved. Confirmed target statuses and resolved-conversation badges remain unchanged. Row selection retains its disabled-state guard, unread focus announcements, and timestamp and unread-indicator tooltips. Conversation menus now start with **Mark as unread**, using a solid dot icon, with a divider before **Delete**. Any repository reader can mark a conversation unread for themselves in Hub or the canvas, without changing other reviewers' state or inbox notifications. The conversation and drafts stay open; reopening marks the displayed activity read again. Replying, editing, reacting, and changing resolution retain their existing mark-as-read behavior. Failed unread changes show an error and can be retried from the menu. A full-canvas Storybook preview shows earlier and current Build comments with zero, one, and two replies, using only in-memory fixtures. Its read and unread actions also stay in memory. The bundled plugin assets are updated with the source. ![Canvas sidebar with aligned avatar and unread-dot gutters, an earlier Build link, reply counts, and muted read comments](https://github.com/user-attachments/assets/1557c496-10f9-4dc0-b7dd-f10296fca2f3) The screenshot uses synthetic Storybook data. The original reference URL redirected to another canvas, so the reported authenticated Build click could not be replayed. Authenticated destination loading and pointer-hover interaction remain unverified; the browser canvas has no hover action and the configured isolated Playwright MCP is unavailable in this runtime. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 616a126c-672b-420e-a62f-e54cdce52001
1 parent 2944cc5 commit a705668

43 files changed

Lines changed: 1096 additions & 169 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/extensions/replica-prototype/assets/renderer-vendor.js‎

Lines changed: 10 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/extensions/replica-prototype/assets/renderer.css‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/extensions/replica-prototype/assets/renderer.js‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/extensions/replica-prototype/start-canvas-server/index.mjs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -567,16 +567,16 @@ async function routeRequest(
567567
const method = request.method
568568
const valid = !operation
569569
? ["PATCH", "DELETE"].includes(method)
570-
: operation === "replies" || operation === "read"
570+
: operation === "replies"
571571
? method === "POST" && !content
572-
: operation === "resolution"
572+
: operation === "resolution" || operation === "read"
573573
? ["POST", "DELETE"].includes(method) && !content
574574
: (method === "POST" && !content) || (method === "DELETE" && !!content)
575575
if (!valid) throw httpError(405, "Review action not supported.")
576576
const body =
577577
(!operation && method === "PATCH") ||
578578
operation === "replies" ||
579-
operation === "read" ||
579+
(operation === "read" && method === "POST") ||
580580
operation === "reactions"
581581
? await readJson(request)
582582
: undefined
@@ -587,7 +587,7 @@ async function routeRequest(
587587
await hub.request(`${await reviewPath(hub)}/${id}${suffix}`, {
588588
method,
589589
body,
590-
interactive: operation !== "read",
590+
interactive: operation !== "read" || method === "DELETE",
591591
}),
592592
)
593593
return

‎.github/extensions/replica-prototype/start-canvas-server/index.test.mjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,20 @@ esac
10671067
.every(({ activity }) => activity === undefined),
10681068
)
10691069
assert.equal((await send("/api/hub/comments/9/resolution", "POST")).status, 200)
1070+
assert.equal(
1071+
(
1072+
await send("/api/hub/comments/9/read", "POST", {
1073+
review_thread_read: { activity_version: 1 },
1074+
})
1075+
).status,
1076+
200,
1077+
)
1078+
assert.equal(calls.at(-1).activity, undefined)
1079+
assert.equal((await send("/api/hub/comments/9/read", "DELETE")).status, 200)
1080+
assert.ok(calls.at(-1).path.endsWith("/review_comments/9/read"))
1081+
assert.equal(calls.at(-1).method, "DELETE")
1082+
assert.equal(calls.at(-1).body, "")
1083+
assert.equal(calls.at(-1).activity, "interactive")
10701084
assert.equal(
10711085
(
10721086
await send("/api/hub/comments/9/reactions/%2B1", "DELETE", {

‎hub/app/controllers/canvas_api/review_thread_reads_controller.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,12 @@ def create
1111
root.thread.record_read_by! Current.user, activity_version: positive_activity_version(value)
1212
render_review_comment_change root, broadcast: false
1313
end
14+
15+
# Clears only the paired reviewer's read state.
16+
# @route DELETE /api/canvas/repositories/:repository_github_id/prototypes/:prototype_id/review_comments/:review_comment_id/read (canvas_api_review_comment_read)
17+
def destroy
18+
root = @prototype.review_comments.roots.find(params[:review_comment_id])
19+
root.thread.record_unread_by! Current.user
20+
render_review_comment_change root, broadcast: false
21+
end
1422
end

‎hub/app/controllers/review_thread_reads_controller.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ def create
1414
render_review_comment_change root, broadcast: false
1515
end
1616

17+
# Clears only the current reviewer's read state.
18+
# @route DELETE /repositories/:repository_github_id/prototypes/:prototype_id/review_comments/:review_comment_id/read (repository_prototype_review_comment_read)
19+
def destroy
20+
root = @prototype.review_comments.roots.find(params[:review_comment_id])
21+
root.thread.record_unread_by! Current.user
22+
render_review_comment_change root, broadcast: false
23+
end
24+
1725
private
1826

1927
# Requires the browser to name the positive activity version it displayed.

‎hub/app/javascript/application/prototype-canvas/stories.test.tsx‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,30 @@ afterEach(() => {
1010
})
1111

1212
describe("PublicationDemo()", () => {
13+
it("shows the updated comments in the complete canvas sidebar without contacting Hub", async () => {
14+
const externalFetch = vi.fn(() => Promise.reject(new Error("Unexpected external request.")))
15+
vi.stubGlobal("fetch", externalFetch)
16+
const Demo = story.component
17+
const view = render(
18+
<Demo scenario="Published" prototypeUrl="http://localhost:4321/" withComments />,
19+
{ wrapper: HubThemeProvider },
20+
)
21+
fireEvent.click(await screen.findByRole("button", { name: /^Comments, 3/ }, { timeout: 5_000 }))
22+
const sidebar = screen.getByRole("complementary", { name: "Comments" })
23+
expect(sidebar).toHaveClass("hub-sidebar")
24+
expect(within(sidebar).getByRole("link", { name: "build 1" })).toHaveAttribute(
25+
"href",
26+
"https://hub.example.com/repositories/2001/prototypes/1?build=1",
27+
)
28+
expect(within(sidebar).getByText("1 reply")).toBeVisible()
29+
expect(within(sidebar).getByText("2 replies")).toBeVisible()
30+
expect(within(sidebar).getAllByRole("button", { pressed: false })).toHaveLength(3)
31+
expect(within(sidebar).queryByRole("link", { name: "build 2" })).not.toBeInTheDocument()
32+
expect(within(sidebar).queryByText("Checking")).not.toBeInTheDocument()
33+
expect(externalFetch).not.toHaveBeenCalled()
34+
view.unmount()
35+
})
36+
1337
it("shows the Publish/Share group and opens fixture management without contacting Hub", async () => {
1438
const externalFetch = vi.fn(() => Promise.reject(new Error("Unexpected external request.")))
1539
vi.stubGlobal("fetch", externalFetch)

‎hub/app/javascript/application/prototype-canvas/stories.tsx‎

Lines changed: 127 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,23 @@ import type { Meta, StoryObj } from "@storybook/react-vite"
33

44
import { App } from "."
55
import { publicationExamples } from "@utils/resolve-prototype-publication/fixtures"
6+
import { repliedReviewThread } from "@/storybook/fixtures/review/comments"
67
import type { ShareLink } from "@hooks/use-canvas-share-links/types"
78
import type { HubStatus, PrototypeState, PublishPreparation } from "@utils/prototype-canvas/types"
9+
import type { ReviewComment } from "@utils/review-comment/types"
810

911
import "./stories.css"
1012

1113
function PublicationDemo({
1214
scenario,
1315
prototypeUrl,
1416
withShareLinks = false,
17+
withComments = false,
1518
}: {
1619
scenario: string
1720
prototypeUrl: string
1821
withShareLinks?: boolean
22+
withComments?: boolean
1923
}) {
2024
const [ready, setReady] = useState(false)
2125
useEffect(() => {
@@ -26,6 +30,50 @@ function PublicationDemo({
2630
let remember = false
2731
if (withShareLinks && hub?.prototype)
2832
hub.prototype = { ...hub.prototype, canManageShareLinks: true }
33+
if (withComments && hub?.prototype?.build)
34+
hub.prototype = {
35+
...hub.prototype,
36+
build: {
37+
...hub.prototype.build,
38+
number: 2,
39+
url: "https://hub.example.com/repositories/2001/prototypes/1?build=2",
40+
},
41+
}
42+
let comments: ReviewComment[] = withComments
43+
? [
44+
repliedReviewThread.root,
45+
...repliedReviewThread.replies,
46+
{
47+
...repliedReviewThread.root,
48+
id: 3,
49+
buildNumber: 2,
50+
buildPath: "/repositories/2001/prototypes/1?build=2",
51+
body: "The updated button spacing looks good.",
52+
author: repliedReviewThread.replies[0]!.author,
53+
viewerLastSeenActivityVersion: 2,
54+
},
55+
{
56+
...repliedReviewThread.root,
57+
id: 4,
58+
buildNumber: 2,
59+
buildPath: "/repositories/2001/prototypes/1?build=2",
60+
body: "Can we keep the heading aligned?",
61+
},
62+
{
63+
...repliedReviewThread.replies[0]!,
64+
id: 5,
65+
rootId: 4,
66+
body: "I checked the narrow layout.",
67+
},
68+
{
69+
...repliedReviewThread.replies[0]!,
70+
id: 6,
71+
rootId: 4,
72+
body: "Thanks, the heading now stays aligned.",
73+
author: repliedReviewThread.root.author,
74+
},
75+
]
76+
: []
2977
let links: ShareLink[] = ["Usability review", "Design review"].map((label, index) => ({
3078
id: index + 1,
3179
label,
@@ -131,8 +179,25 @@ function PublicationDemo({
131179
hub = { ...hub, prototype: { ...hub.prototype, registered: true, published: true } }
132180
data = hub.prototype
133181
} else if (path === "/api/hub/comments")
134-
data = { comments: [], hasMore: false, nextCursor: null }
135-
else if (path === "/api/hub/events") {
182+
data = {
183+
comments,
184+
hasMore: false,
185+
nextCursor: null,
186+
}
187+
else if (withComments && /^\/api\/hub\/comments\/\d+\/read$/.test(path)) {
188+
const id = Number(path.split("/").at(-2))
189+
const root = comments.find((comment) => comment.id === id && comment.kind === "root")
190+
if (!root || root.kind !== "root") throw new Error("Unknown demo conversation.")
191+
if (init?.method !== "POST" && init?.method !== "DELETE")
192+
throw new Error("Unsupported demo read-state action.")
193+
194+
const updated = {
195+
...root,
196+
viewerLastSeenActivityVersion: init.method === "DELETE" ? null : root.activityVersion,
197+
}
198+
comments = comments.map((comment) => (comment.id === root.id ? updated : comment))
199+
data = { comments: [updated] }
200+
} else if (path === "/api/hub/events") {
136201
return new Response(
137202
new ReadableStream({
138203
start(controller) {
@@ -156,7 +221,7 @@ function PublicationDemo({
156221
window.clearTimeout(timer)
157222
window.fetch = originalFetch
158223
}
159-
}, [scenario, withShareLinks])
224+
}, [scenario, withShareLinks, withComments])
160225
return (
161226
<div className="publication-demo">
162227
<div className="publication-demo-canvas">
@@ -189,7 +254,12 @@ const meta = {
189254
: "http://127.0.0.1:38763/",
190255
},
191256
argTypes: { scenario: { control: "select", options: Object.keys(publicationExamples) } },
192-
render: (args) => <PublicationDemo key={`${args.scenario}/${args.withShareLinks}`} {...args} />,
257+
render: (args) => (
258+
<PublicationDemo
259+
key={`${args.scenario}/${args.withShareLinks}/${args.withComments}`}
260+
{...args}
261+
/>
262+
),
193263
} satisfies Meta<typeof PublicationDemo>
194264

195265
export default meta
@@ -230,6 +300,59 @@ export const UnknownRegistrationReason: Story = {
230300
}
231301
export const Failed: Story = { args: { scenario: "Build failed" } }
232302
export const Ready: Story = { args: { scenario: "Published" } }
303+
export const CommentsSidebar: Story = {
304+
args: {
305+
scenario: "Published",
306+
withComments: true,
307+
},
308+
render: (args) => (
309+
<PublicationDemo
310+
{...args}
311+
prototypeUrl={new URL("/prototype-preview/index.html", window.location.href).href}
312+
/>
313+
),
314+
play: async ({ canvas, userEvent }) => {
315+
await userEvent.click(
316+
await canvas.findByRole("button", { name: /^Comments, 3/ }, { timeout: 5_000 }),
317+
)
318+
await canvas.findByRole("complementary", { name: "Comments" })
319+
await canvas.findByRole("link", { name: "build 1" })
320+
await canvas.findByText("1 reply")
321+
await canvas.findByText("2 replies")
322+
const link = await canvas.findByRole("link", { name: "build 1" })
323+
const row = link.closest("article")
324+
const unread = row?.querySelector(".review-comment-row__unread")
325+
if (!row || !unread) throw new Error("The unread comment fixture is missing.")
326+
const bounds = row.getBoundingClientRect()
327+
const surface = getComputedStyle(row, "::before")
328+
const surfaceLeft = bounds.left + parseFloat(surface.left)
329+
const surfaceRight = bounds.right - parseFloat(surface.right)
330+
const scroller = row.closest(".review-comments-sidebar__body")?.getBoundingClientRect()
331+
if (
332+
!scroller ||
333+
!Number.isFinite(surfaceLeft) ||
334+
!Number.isFinite(surfaceRight) ||
335+
surfaceLeft < scroller.left - 0.5 ||
336+
surfaceRight > scroller.right + 0.5
337+
)
338+
throw new Error("The wider hover surface must remain inside the comment scroll area.")
339+
const body = row.querySelector(".review-comment-row__body")?.getBoundingClientRect()
340+
const avatar = row.querySelector(".review-comment-row__avatar")?.getBoundingClientRect()
341+
const author = row.querySelector(".review-comment-row__author")?.getBoundingClientRect()
342+
const dot = unread.getBoundingClientRect()
343+
if (!body || Math.abs(dot.top + dot.height / 2 - body.top - body.height / 2) > 0.5)
344+
throw new Error("The unread dot must be vertically centered beside the comment text.")
345+
if (
346+
!avatar ||
347+
!author ||
348+
Math.abs(avatar.width - 20) > 0.5 ||
349+
Math.abs(avatar.left - surfaceLeft - 8) > 0.5 ||
350+
Math.abs(avatar.left + avatar.width / 2 - dot.left - dot.width / 2) > 0.5 ||
351+
Math.abs(avatar.top + avatar.height / 2 - author.top - author.height / 2) > 0.5
352+
)
353+
throw new Error("The compact avatar must align above the unread dot beside the author.")
354+
},
355+
}
233356
export const ShareManagement: Story = {
234357
args: {
235358
scenario: "Published",

0 commit comments

Comments
 (0)