diff --git a/src/renderer/src/components/editor/CheckRunAnnotations.tsx b/src/renderer/src/components/editor/CheckRunAnnotations.tsx index eae9460c781..b3acc274152 100644 --- a/src/renderer/src/components/editor/CheckRunAnnotations.tsx +++ b/src/renderer/src/components/editor/CheckRunAnnotations.tsx @@ -2,11 +2,7 @@ import React from 'react' import { ExternalLink } from 'lucide-react' import { translate } from '@/i18n/i18n' import type { PRCheckAnnotation } from '../../../../shared/github/check-types' -import { - cancelAnnotationRevealFrame, - getOpenableAnnotationLine, - openAnnotationLocation -} from './check-annotation-open' +import { getOpenableAnnotationLine, openAnnotationLocation } from './check-annotation-open' import { formatAnnotationsForClipboard } from './check-run-clipboard-text' import { CheckRunCopyButton } from './CheckRunCopyButton' @@ -17,21 +13,12 @@ export function CheckRunAnnotations({ annotations: PRCheckAnnotation[] worktreeId: string | null }): React.JSX.Element { - const revealRafRef = React.useRef(null) - const revealInnerRafRef = React.useRef(null) - React.useEffect( - () => () => { - cancelAnnotationRevealFrame(revealRafRef) - cancelAnnotationRevealFrame(revealInnerRafRef) - }, - [] - ) const openAnnotation = React.useCallback( (path: string, line: number) => { if (!worktreeId) { return } - openAnnotationLocation({ worktreeId, path, line, revealRafRef, revealInnerRafRef }) + openAnnotationLocation({ worktreeId, path, line }) }, [worktreeId] ) diff --git a/src/renderer/src/components/editor/check-annotation-open.test.ts b/src/renderer/src/components/editor/check-annotation-open.test.ts index 97f0fbf09b2..fe25110f0e5 100644 --- a/src/renderer/src/components/editor/check-annotation-open.test.ts +++ b/src/renderer/src/components/editor/check-annotation-open.test.ts @@ -3,7 +3,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ activateAndRevealWorktree: vi.fn(), state: { - worktreesByRepo: { 'repo-1': [{ id: 'wt-1', repoId: 'repo-1', path: '/repo/wt' }] }, + getKnownWorktreeById: vi.fn((id: string) => + id === 'wt-1' ? { id: 'wt-1', repoId: 'repo-1', path: '/repo/wt' } : undefined + ), openFile: vi.fn(), setPendingEditorReveal: vi.fn() } @@ -23,22 +25,24 @@ afterEach(() => { }) describe('openAnnotationLocation', () => { - it('activates as a surface-providing caller before opening the editor', () => { + it('activates the given host as a surface-providing caller before opening the editor', () => { vi.stubGlobal('requestAnimationFrame', vi.fn().mockReturnValue(1)) vi.stubGlobal('cancelAnimationFrame', vi.fn()) openAnnotationLocation({ worktreeId: 'wt-1', + executionHostId: 'ssh:devbox', path: 'src/a.ts', - line: 3, - revealRafRef: { current: null }, - revealInnerRafRef: { current: null } + line: 3 }) // Why: the annotation's editor file is the surface — the jump must not re-seed - // a shell into a workspace whose last terminal the user closed. + // a shell into a workspace whose last terminal the user closed. The host keeps a + // remote workspace from switching to its local twin with the same id. + expect(mocks.state.getKnownWorktreeById).toHaveBeenCalledWith('wt-1', 'ssh:devbox') expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', { - providesInitialSurface: true + providesInitialSurface: true, + executionHostId: 'ssh:devbox' }) expect(mocks.state.openFile).toHaveBeenCalledWith( expect.objectContaining({ worktreeId: 'wt-1', relativePath: 'src/a.ts' }), @@ -50,9 +54,7 @@ describe('openAnnotationLocation', () => { openAnnotationLocation({ worktreeId: 'missing', path: 'src/a.ts', - line: 3, - revealRafRef: { current: null }, - revealInnerRafRef: { current: null } + line: 3 }) expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled() diff --git a/src/renderer/src/components/editor/check-annotation-open.ts b/src/renderer/src/components/editor/check-annotation-open.ts index 2f4c71a8367..a5749252ff2 100644 --- a/src/renderer/src/components/editor/check-annotation-open.ts +++ b/src/renderer/src/components/editor/check-annotation-open.ts @@ -1,12 +1,12 @@ import { detectLanguage } from '@/lib/language-detect' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { useAppStore } from '@/store' -import { findWorktreeById } from '@/store/slices/worktree-helpers' +import { scheduleEditorLineReveal } from '@/store/slices/editor/focus/editor-focus-reveal' import { getOpenableAnnotationLine, resolveAnnotationPathInsideWorktree } from './check-annotation-path' -import { selectWorktreesByRepo } from '@/store/workspace-record-selectors' +import type { ExecutionHostId } from '../../../../shared/execution-host' export { getOpenableAnnotationLine } @@ -14,12 +14,11 @@ export function openAnnotationLocation(params: { worktreeId: string path: string line: number - revealRafRef: React.RefObject - revealInnerRafRef: React.RefObject + executionHostId?: ExecutionHostId }): void { - const { worktreeId, path, line, revealRafRef, revealInnerRafRef } = params + const { worktreeId, path, line, executionHostId } = params const store = useAppStore.getState() - const worktree = findWorktreeById(selectWorktreesByRepo(store), worktreeId) + const worktree = store.getKnownWorktreeById(worktreeId, executionHostId) if (!worktree) { return } @@ -28,41 +27,18 @@ export function openAnnotationLocation(params: { return } const { absolutePath, relativePath } = resolvedPath + const language = detectLanguage(relativePath) // Why: reuse the shared activation path so an annotation jump lands in the // same history stack as sidebar, palette, and terminal-link navigation. - activateAndRevealWorktree(worktreeId, { providesInitialSurface: true }) + activateAndRevealWorktree(worktreeId, { providesInitialSurface: true, executionHostId }) - store.openFile( - { - filePath: absolutePath, - relativePath, - worktreeId, - language: detectLanguage(relativePath), - mode: 'edit' - }, + const fileId = store.openFile( + { filePath: absolutePath, relativePath, worktreeId, language, mode: 'edit' }, { forceContentReload: true } ) - - cancelAnnotationRevealFrame(revealRafRef) - cancelAnnotationRevealFrame(revealInnerRafRef) - store.setPendingEditorReveal(null) - - // Why: opening can replace the active tab and mount Monaco asynchronously. - // Matching search and terminal-link navigation, wait two frames so the - // destination editor owns layout before we ask it to reveal the line. - revealRafRef.current = requestAnimationFrame(() => { - revealInnerRafRef.current = requestAnimationFrame(() => { - store.setPendingEditorReveal({ filePath: absolutePath, line, column: 1, matchLength: 0 }) - cancelAnnotationRevealFrame(revealRafRef) - cancelAnnotationRevealFrame(revealInnerRafRef) - }) - }) -} - -export function cancelAnnotationRevealFrame(frameRef: React.RefObject): void { - if (frameRef.current !== null) { - cancelAnimationFrame(frameRef.current) - frameRef.current = null + if (language === 'markdown') { + store.setMarkdownViewMode(fileId, 'source') } + scheduleEditorLineReveal(useAppStore.getState, absolutePath, line, 1, fileId) } diff --git a/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx b/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx index 28232208121..53170bd0204 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx @@ -17,6 +17,7 @@ import { PRTriageStrip } from './triage-strip' import { ConflictingFilesSection, MergeConflictNotice } from './conflict-summary' import { ChecksList } from './checks-list' import { PRCommentsList } from './comments-list' +import { openAnnotationLocation } from '@/components/editor/check-annotation-open' import { translate } from '@/i18n/i18n' import type { ChecksPanelActiveContentModel } from './active-content-props' import { ChecksPanelReviewHeader } from './review-header' @@ -259,6 +260,17 @@ export function ChecksPanelActiveContent({ onEditComment={pr ? handleEditComment : undefined} onDeleteComment={pr ? handleDeleteComment : undefined} onSetReaction={canTargetPRComments ? handleSetReaction : undefined} + onOpenLocation={ + activeWorktree + ? (path, line) => + openAnnotationLocation({ + worktreeId: activeWorktree.id, + executionHostId: activeWorktree.hostId, + path, + line + }) + : undefined + } /> void +}): React.JSX.Element | null { + const { path, line } = comment + if (!path) { + return null + } + const lineRange = formatLineRange(comment) + const label = `${basename(path)}${lineRange ? `:${lineRange}` : ''}` + if (!onOpenLocation || !line) { + return ( + + {label} + + ) + } + return ( + + ) +} + /** True for top-level PR conversation comments the viewer can edit or delete. */ export function isMutablePRConversationComment(comment: PRComment): boolean { if (comment.threadId || comment.path) { diff --git a/src/renderer/src/components/right-sidebar/checks-panel/comment-group.tsx b/src/renderer/src/components/right-sidebar/checks-panel/comment-group.tsx index ebf15a2dc6e..ad87c185ae4 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/comment-group.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/comment-group.tsx @@ -45,7 +45,8 @@ export function PRCommentGroupView({ onEditComment, onDeleteComment, onSetReaction, - onQueueForAgent + onQueueForAgent, + onOpenLocation }: { group: PRCommentGroup botAuthorOverrides: ReadonlySet @@ -69,6 +70,7 @@ export function PRCommentGroupView({ reacted: boolean ) => Promise onQueueForAgent?: () => void + onOpenLocation?: (path: string, line: number) => void }): React.JSX.Element { // Reply targets a specific comment id so any comment in a thread — root or // nested reply — can be replied to, not just the thread root. @@ -116,7 +118,8 @@ export function PRCommentGroupView({ onEditComment, onDeleteComment, onSetReaction, - onQueueForAgent + onQueueForAgent, + onOpenLocation } const content = @@ -201,7 +204,8 @@ export function ResolvedCommentGroupsSection({ onReply, onEditComment, onDeleteComment, - onSetReaction + onSetReaction, + onOpenLocation }: { groups: PRCommentGroup[] botAuthorOverrides: ReadonlySet @@ -221,6 +225,7 @@ export function ResolvedCommentGroupsSection({ content: GitHubReactionContent, reacted: boolean ) => Promise + onOpenLocation?: (path: string, line: number) => void }): React.JSX.Element | null { if (groups.length === 0) { return null @@ -258,6 +263,7 @@ export function ResolvedCommentGroupsSection({ onEditComment={onEditComment} onDeleteComment={onDeleteComment} onSetReaction={onSetReaction} + onOpenLocation={onOpenLocation} /> ))} diff --git a/src/renderer/src/components/right-sidebar/checks-panel/comment-row.test.tsx b/src/renderer/src/components/right-sidebar/checks-panel/comment-row.test.tsx new file mode 100644 index 00000000000..cc54cabd7f2 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/checks-panel/comment-row.test.tsx @@ -0,0 +1,42 @@ +// @vitest-environment happy-dom + +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { getPRCommentPresentationClasses } from '../pr-comment-presentation' +import { CommentRow } from './comment-row' + +afterEach(cleanup) + +describe('CommentRow location', () => { + it('opens the full repo path at the first line of the commented range', () => { + const onOpenLocation = vi.fn() + render( + + ) + + screen.getByRole('button', { name: 'doctor.ts:L370-L380' }).click() + + expect(onOpenLocation).toHaveBeenCalledWith('src/cli/doctor.ts', 370) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/checks-panel/comment-row.tsx b/src/renderer/src/components/right-sidebar/checks-panel/comment-row.tsx index aeee8cc3eaf..731b834e9c5 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/comment-row.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/comment-row.tsx @@ -11,13 +11,13 @@ import { formatPrCommentRelativeTime } from '../../../../../shared/pr-comment-ti import { translate } from '@/i18n/i18n' import { buildCopyText, + CommentLocationLabel, CommentMoreMenu, CopyButton, isMutablePRConversationComment, PRCommentActionBadge, QueueForAgentButton, - ResolveButton, - formatLineRange + ResolveButton } from './comment-controls' /** A single comment row — used for both root and reply comments. */ @@ -39,7 +39,8 @@ export function CommentRow({ onEditComment, onDeleteComment, onSetReaction, - onQueueForAgent + onQueueForAgent, + onOpenLocation }: { comment: PRComment botAuthorOverrides: ReadonlySet @@ -63,6 +64,7 @@ export function CommentRow({ reacted: boolean ) => Promise onQueueForAgent?: () => void + onOpenLocation?: (path: string, line: number) => void }): React.JSX.Element { const automated = isBotPRComment(comment, botAuthorOverrides) const canMutateComment = isMutablePRConversationComment(comment) @@ -199,12 +201,11 @@ export function CommentRow({ {translate('auto.components.right.sidebar.checks.panel.content.2ba0a32bdd', 'bot')} ) : null} - {comment.path ? ( - - {comment.path.split('/').pop()} - {formatLineRange(comment) && `:${formatLineRange(comment)}`} - - ) : null} + )} - {!isReply && comment.path && ( - - {comment.path.split('/').pop()} - {formatLineRange(comment) && `:${formatLineRange(comment)}`} - + {!isReply && ( + )} {!isReply ? ( )} diff --git a/src/renderer/src/components/right-sidebar/checks-panel/use-comments-list-state.tsx b/src/renderer/src/components/right-sidebar/checks-panel/use-comments-list-state.tsx index 38c9df9730c..7dba50cc85e 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/use-comments-list-state.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/use-comments-list-state.tsx @@ -64,6 +64,7 @@ export type PRCommentsListProps = { content: GitHubReactionContent, reacted: boolean ) => Promise + onOpenLocation?: (path: string, line: number) => void } export function useCommentsListState({ @@ -78,7 +79,8 @@ export function useCommentsListState({ onEditComment, onDeleteComment, onSetReaction, - onResolveSelectedCommentsWithAI + onResolveSelectedCommentsWithAI, + onOpenLocation }: PRCommentsListProps) { function findVerticalScrollParent(element: HTMLElement): HTMLElement | null { let parent = element.parentElement @@ -250,6 +252,7 @@ export function useCommentsListState({ onDeleteComment={onDeleteComment} onSetReaction={onSetReaction} onQueueForAgent={canQueue ? () => addGroupToSelection(groupId) : undefined} + onOpenLocation={onOpenLocation} /> ) }