镜像站点 · 本页由第三方 GitHub 只读镜像提供，非 GitHub 官方站点，不接受任何登录或凭据输入。前往 github.com
Skip to content

feat(github): reveal the PR comment a link points at - #947

Merged
kalvinnchau merged 4 commits into
mainfrom
highlight-linked-pr-comment
Oct 15, 2026
Merged

feat(github): reveal the PR comment a link points at#947
kalvinnchau merged 4 commits into
mainfrom
highlight-linked-pr-comment

Conversation

@matt2e

@matt2e matt2e commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Opening a /pull/N#issuecomment-<id> or #pullrequestreview-<id> link opened the GitHub pane at the top of the PR, with nothing marking the comment the link points at. The pane now scrolls to that comment, opens it and highlights it.

Depends on #920. The first commit (2627c26d) is #920's fix cherry-picked in. It stops the pane from claiming PR links whose fragment it can't show (#discussion_r…, timeline events, and any fragment on an issue). Review the last three commits here. Once #920 merges, a rebase drops the duplicate.

Demo

before-after.mp4

Left: main — the #issuecomment link opens the PR at the top. Right: this branch — the run opens and the linked comment lands at the bottom of the pane, focused and highlighted; a link to a comment outside the loaded data shows the "Open it on GitHub" notice.

Changes

  • references.ts: parses the fragment into a GitHubAnchor (discussion / review plus an id) whose kind-id matches the conversation event keys. GitHubReference.url no longer includes the fragment, so when only the anchor changes the pane keeps its request and loaded content. gitHubAnchorUrl rebuilds comment links.
  • GitHubConversation.tsx:
    • Each comment row carries data-event-key.
    • A one-shot useAnchorReveal (MutationObserver + rAF) waits for the linked row to mount, scrolls it into view and focuses it. The row lands at the bottom of the scroller (block: "end") so the comments before it stay visible. A row taller than the scroller aligns to start instead.
    • If the linked comment is inside a folded history run, the run opens without animation. The anchored message also opens.
    • If the comment isn't in the loaded data (a later page, or an empty review the pane leaves out), a status notice links to it on GitHub once that comment's own source has finished loading.
  • index.tsx: a new anchor switches the PR back to the Discussion tab. The title and error links keep the fragment.
  • GitHub.module.css: the data-anchored row gets the same accent wash as the channel feed's return highlight (a row ::before with --radius-row), with the keyboard focus ring drawn on that same shape.

Known gaps (still a draft)

  • Reveal race: a run is keyed by its first event. If issues/N/comments resolves before pulls/N/reviews, the run key can flip (e.g. discussion-20 → review-9). History then remounts closed, and the one-shot reveal doesn't run again: focus falls to body and scrollTop stays 0. This happens every time when reviews arrive 400 ms late.
  • Double focus ring: under data-keyboard-navigation the row itself still reports outline-style: auto, so WebKit draws its blue UA ring inside the designed ring. It needs outline: none on the row.
  • Two tabs for one PR: two comment links to the same PR still open two channel tabs. Tab identity compares raw targets, and the FOUNDATION Panel contract has no identity hook, so fixing this needs escalation first.

Tests

  • Vitest coverage in references.test.ts, conversation.test.tsx and GitHubPanel.test.tsx: fragment gating, the anchor round-trip, the reveal into a folded run, the missing-comment notice, and an anchor-only change reusing loaded content.
  • tests/browser/external-links.spec.mjs is updated for the fragment gating.
  • Deferred: human testing in the app, both themes, narrow/wide and enlarged-text checks.

🤖 Generated with Claude Code

@matt2e
matt2e force-pushed the highlight-linked-pr-comment branch from f9c320d to 2b0ce00 Compare October 14, 2026 00:56
@matt2e
matt2e marked this pull request as ready for review October 14, 2026 02:15
@matt2e
matt2e requested a review from a team as a code owner October 14, 2026 02:15
kalvinnchau
kalvinnchau reviewed Oct 14, 2026
View reviewed changes

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 One correctness finding in the anchor reveal lifecycle. Static review only; no tests or app execution.

Comment thread src/bundled/github/GitHubConversation.tsx
...container.querySelectorAll<HTMLElement>("[data-event-key]"),
].find((element) => element.dataset.eventKey === key);
if (!row) return; // A folded run mounts its rows after opening; observe the mount.
observer.disconnect();

@kalvinnchau kalvinnchau Oct 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [P2] Preserve the reveal across asynchronous regrouping

On a merged PR, let discussion comments 20 and 21 load and reveal #issuecomment-21, then let the reviews request return an earlier nonempty COMMENTED review 9. groupConversationEvents changes the history key from discussion-20 to review-9, so React removes the focused row and mounts a new history subtree. This observer has already disconnected, and the effect dependencies remain the same anchor key/ref, so the replacement row never gets scrolled to or focused. The new History opens via its own layout effect, but keyboard focus is lost and the linked comment is no longer guaranteed to remain in view. Preserve the row/reveal through regrouping or defer the initial reveal until both initial sources settle, and cover this ordering with a gated reviews response.

kalvinnchau
kalvinnchau reviewed Oct 14, 2026
View reviewed changes

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 No new findings in the delta from 2b0ce00 to 3b415cb. The initial source-order race is addressed. Code-only review; tests and browser behavior were not executed.

matt2e and others added 4 commits October 15, 2026 09:48
@matt2e @claude
feat(github): reveal the comment a PR link points at
804c7d5 
Opening a `/pull/N#issuecomment-<id>` or `#pullrequestreview-<id>` link
showed the PR top with no cue for the referenced comment. Parse the
fragment once into a structured anchor whose `kind-id` matches the
conversation event keys, and keep the reference URL hash-less so an
anchor-only target change reuses the PR's request and loaded content
instead of refetching.

Each rendered comment row now carries `data-event-key`. A one-shot
reveal effect (MutationObserver + rAF, after use-message-reveal) opens
the folded history run holding the linked comment without animation,
scrolls the row into view, focuses it and expands it; `data-anchored`
paints a persistent accent wash with a keyboard focus ring. A new
anchor on an already-open PR returns to Discussion and reveals again.

When the comment is outside the loaded data (later page, or an empty
comment-only review the pane omits), a notice offers the comment on
GitHub once its own source has answered.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e @claude
fix(github): land the linked comment at the scroller's bottom
0ca0eb0 
The reveal aligned the anchored row to the top of the pane, hiding the
conversation that led to it and, as the before/after recording showed,
clipping the accent wash and focus ring against the scroller edge.

Align with `block: "end"` so the row's bottom edge meets the visible
bottom with the preceding comments above it; native alignment clamps to
`scrollHeight`, so a comment near the end never overscrolls. A row
taller than the scroller keeps its top visible with `block: "start"`
instead. The scroller is the nearest `overflow-y` ancestor (the pane
root today), found with the same walk ChannelTimeline uses.
`scroll-margin-block: var(--space-3)` on `[data-anchored]` keeps the
wash (which extends `--space-2` past the row) and the ring clear of the
edge.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e @claude
style(github): paint the anchored comment like the feed's return wash
f625d53 
The anchored PR-comment highlight deviated from the thread-return cue it
mirrors: its wash covered only the content column with a `--border-accent`
inset ring no other state fill carries, while the keyboard focus ring drew
on the full row box with square corners, so ring and wash described two
different shapes.

Drop the ring, move the wash pseudo-element to the row with the same
`inset-block: 0; inset-inline: calc(-1 * var(--space-2))` extension as
`.returnHighlight`, and draw the focus outline on that pseudo-element with
`--border-focus` so both share one `--radius-row` box. The marker and
event icon already sit at `z-index: 1`, so they stay above the wash in
both the grid row and the stacked ≤18rem layout. `scroll-margin-block`
stays because the ring is still offset outward.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e @claude
fix(github): reveal the linked comment only once both sources answer
0961d6f 
Folded runs are keyed by their first event, so a review arriving after
the discussion (or the reverse) can move the run's start and remount the
linked comment's row. The one-shot reveal had already disconnected its
observer, leaving the replacement row unscrolled and unfocused.

Withhold the reveal key while either source is loading, and remember
the last revealed key so a later page load toggling that gate does not
scroll or steal focus again. A gated reviews response covers the
reordering.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e force-pushed the highlight-linked-pr-comment branch from 3b415cb to 0961d6f Compare October 14, 2026 22:48
@kalvinnchau
kalvinnchau merged commit 56bd9db into main Oct 15, 2026
32 checks passed
@kalvinnchau
kalvinnchau deleted the highlight-linked-pr-comment branch October 15, 2026 00:52
johnmatthewtennant added a commit that referenced this pull request Oct 15, 2026
@johnmatthewtennant
Merge remote-tracking branch 'origin/main' into claude/me-tab-section…
d67ee3a 
…-drag

* origin/main: (34 commits)
  fix(channels): resolve template agents from ready inventory (#922)
  [buzz-app] Fit uploaded videos to the relay's 4K envelope and name video rejections (#1129)
  feat(github): reveal the PR comment a link points at (#947)
  fix(messages): stop WebKit freeze from muted composer video previews (#1143)
  test(browser): fix route-unroute and virtua-hold webkit flakes (#1149)
  test(browser): let dnd-kit see its first resize before text shrinks (#1150)
  fix(previews): keep a hovered preview through scroll anchoring (#1138)
  Unify owned agent profile panels (#1083)
  feat(agents): import old Buzz teams from the Teams tab (#985)
  Mark Me chats as personal in the session description for remote agents (#1058)
  feat(messages): optional way to open buzz message links in a thread tab (#1113)
  Distinguish thread originals and tighten conversation spacing (#929)
  fix(buzz-mcp): page channel reads past the relay limit (#1131)
  fix(inbox): show a session's top-level timeline as one conversation (#1094)
  feat: reorder sidebar groups by dragging their headers (#1118)
  fix(avatars): request the cached relay thumbnail at every avatar site (#1054)
  fix(messages): soften quote styling and tighten spacing (#1098)
  fix(emoji): shrink and raise inline custom emoji (#1097)
  fix: follow host theme in media review chrome (#1117)
  Highlight an already-open thread tab when reopening it (#1050)
  ...

Signed-off-by: John Tennant <jtennant@block.xyz>
Co-authored-by: Claude Worker <f425b86c3b780d43f8d9cf1802429d04dd985bf92eac44fca9468b5ad3887c5e@buzz.block.builderlab.xyz>

# Conflicts:
#	src/features/channel-navigation/ChannelSidebarDnd.tsx
johnmatthewtennant added a commit that referenced this pull request Oct 15, 2026
@johnmatthewtennant
Merge remote-tracking branch 'origin/main' into HEAD
a998270 
* origin/main:
  fix(navigation): share one abort reason so Settings → Back stops leaking DOM on V8 (#1161)
  Unify sidebar resizing and polish Solo navigation (#1132)
  Organize Builderlab agent identity and runtime settings (#1153)
  fix(channels): keep consecutive tab closes in the closing pane (#1158)
  feat(goose): add the goose agent type (#1125)
  feat: edit the last message with Cmd+Up (#1042)
  fix(channels): make Enter press the prominent lifecycle button (#1152)
  feat(me): drag chats between projects and Chats (#1112)
  fix(search): find open channels past the first 500 in Command-K (#1128)
  Stop in-flight request mocks from failing finished browser tests (#987)
  fix(channels): resolve template agents from ready inventory (#922)
  [buzz-app] Fit uploaded videos to the relay's 4K envelope and name video rejections (#1129)
  feat(github): reveal the PR comment a link points at (#947)
  fix(messages): stop WebKit freeze from muted composer video previews (#1143)

Signed-off-by: John Tennant <jtennant@block.xyz>

# Conflicts:
#	docs/sessions/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kalvinnchau kalvinnchau kalvinnchau approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matt2e @kalvinnchau