refactor(github): move inline PR comment line placement to src/shared (#36904)

This commit is contained in:
Kelvin Amoaba
2026-11-01 05:43:51 +00:00
committed by GitHub
parent c0190ff9fa
commit fddfd7b024
4 changed files with 54 additions and 35 deletions
@@ -1,7 +1,6 @@
import React, { useCallback, useLayoutEffect, useMemo, useRef, useState } from 'react'
import { useVirtualizer } from '@tanstack/react-virtual'
import type { editor as monacoEditor } from 'monaco-editor'
import type { DecoratedDiffComment } from '@/components/diff-comments/decorated-diff-comment'
import { useCombinedDiffSectionIndexMap } from '../../editor/combined-diff/resolve-changes/use-combined-diff-section-index-map'
import { handleCombinedDiffFileTreeNavigation } from '../../editor/combined-diff/browse-files/combined-diff-file-tree-navigation'
import { getDiffSectionRowEstimatedHeight } from '@/components/editor/diff-section-layout'
@@ -18,7 +17,7 @@ import {
gitHubPRFileToBranchEntry,
type PRFilesCombinedDiffViewerProps
} from '@/components/github/pr-file-diff-mapping'
import { formatRelativeTime } from '@/components/github/work-item-state-presentation'
import { buildInlineReviewComments } from '@/components/pull-request-page/files/inline-comments'
import { PRViewedCheckbox } from '@/components/github/PRViewedCheckbox'
import { PRFilesCombinedDiffBody } from './pr-files-combined-diff-body'
import { getPRFilesCombinedDiffSignature } from './pr-files-combined-diff-signature'
@@ -123,34 +122,8 @@ function PRFilesCombinedDiffSections({
fileByPath.get(section.path)?.reviewCommentLineNumbers,
[fileByPath]
)
const inlineReviewComments = useMemo<DecoratedDiffComment[]>(
() =>
comments.flatMap((comment): DecoratedDiffComment[] => {
// Why: outdated threads' line number can attach the comment to unrelated current code, so skip them inline.
if (comment.isOutdated || !comment.path || typeof comment.line !== 'number') {
return []
}
const createdAtMs = new Date(comment.createdAt).getTime()
return [
{
id: `github-pr-comment:${comment.id}`,
worktreeId: `github-pr:${repoId}:${prNumber}`,
filePath: comment.path,
source: 'diff',
startLine: comment.startLine,
lineNumber: comment.line,
body: comment.body,
createdAt: Number.isFinite(createdAtMs) ? createdAtMs : Date.now(),
side: 'modified',
author: comment.author,
authorAvatarUrl: comment.authorAvatarUrl,
createdAtLabel: formatRelativeTime(comment.createdAt),
url: comment.url,
canDelete: false,
canEdit: false
}
]
}),
const inlineReviewComments = useMemo(
() => buildInlineReviewComments(comments, repoId, prNumber),
[comments, prNumber, repoId]
)
const [sectionHeights, setSectionHeights] = useState<Record<number, number>>({})
@@ -1,6 +1,7 @@
import type { DecoratedDiffComment } from '@/components/diff-comments/decorated-diff-comment'
import { formatRelativeTime } from '@/components/github/work-item-state-presentation'
import type { PRComment } from '../../../../../shared/github/comment-types'
import { getPRInlineCommentPlacement } from '../../../../../shared/github/pr-inline-comment-placement'
export function buildInlineReviewComments(
comments: PRComment[],
@@ -8,8 +9,8 @@ export function buildInlineReviewComments(
prNumber: number
): DecoratedDiffComment[] {
return comments.flatMap((comment): DecoratedDiffComment[] => {
// Why: outdated threads keep originalLine for the sidebar, but rendering it inline can attach the comment to unrelated current code.
if (comment.isOutdated || !comment.path || typeof comment.line !== 'number') {
const placement = getPRInlineCommentPlacement(comment)
if (!placement) {
return []
}
const createdAtMs = new Date(comment.createdAt).getTime()
@@ -17,10 +18,10 @@ export function buildInlineReviewComments(
{
id: `github-pr-comment:${comment.id}`,
worktreeId: `github-pr:${repoId}:${prNumber}`,
filePath: comment.path,
filePath: placement.path,
source: 'diff',
startLine: comment.startLine,
lineNumber: comment.line,
startLine: placement.startLine,
lineNumber: placement.line,
body: comment.body,
createdAt: Number.isFinite(createdAtMs) ? createdAtMs : Date.now(),
side: 'modified',
@@ -0,0 +1,29 @@
import { describe, expect, it } from 'vitest'
import type { PRComment } from './comment-types'
import { getPRInlineCommentPlacement } from './pr-inline-comment-placement'
const comment = (overrides: Partial<PRComment>): PRComment => ({
id: 1,
author: 'octocat',
authorAvatarUrl: '',
body: 'body',
createdAt: '2026-11-01T00:00:00Z',
url: 'https://github.com/o/r/pull/1#discussion_r1',
...overrides
})
describe('getPRInlineCommentPlacement', () => {
it('places a current review comment on its range', () => {
expect(
getPRInlineCommentPlacement(comment({ path: 'src/a.ts', line: 12, startLine: 10 }))
).toEqual({ path: 'src/a.ts', line: 12, startLine: 10 })
})
it('keeps outdated threads and conversation comments out of the diff', () => {
expect(
getPRInlineCommentPlacement(comment({ path: 'src/a.ts', line: 12, isOutdated: true }))
).toBeNull()
expect(getPRInlineCommentPlacement(comment({ path: 'src/a.ts' }))).toBeNull()
expect(getPRInlineCommentPlacement(comment({}))).toBeNull()
})
})
@@ -0,0 +1,16 @@
import type { PRComment } from './comment-types'
export type PRInlineCommentPlacement = {
path: string
line: number
startLine: number | undefined
}
/** Where a PR review comment sits in the current diff, or null when it should not render inline. */
export function getPRInlineCommentPlacement(comment: PRComment): PRInlineCommentPlacement | null {
// Why: outdated threads keep originalLine for the sidebar, but rendering it inline can attach the comment to unrelated current code.
if (comment.isOutdated || !comment.path || typeof comment.line !== 'number') {
return null
}
return { path: comment.path, line: comment.line, startLine: comment.startLine }
}