From f53dddeb780bf421ad9c26557342ab8eb4a232b5 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:51:58 -0700 Subject: [PATCH] Fix stale GitHub task drawer (#5424) Co-authored-by: Orca --- .../src/components/GitHubItemDialog.tsx | 112 +++--------------- src/renderer/src/components/TaskPage.tsx | 20 +--- ...github-item-dialog-source-boundary.test.ts | 9 ++ .../github-project/ProjectViewWrapper.tsx | 57 +++++---- .../TabBar.windows-shell-launch.test.ts | 1 + .../task-page-source-switch-boundary.test.ts | 17 +++ 6 files changed, 70 insertions(+), 146 deletions(-) diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 360e272486e..7e636618d77 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -51,8 +51,6 @@ import { ButtonGroup } from '@/components/ui/button-group' import { Input } from '@/components/ui/input' import { useMountedRef } from '@/hooks/useMountedRef' import { useConfirmationDialog } from '@/components/confirmation-dialog' -import { Sheet, SheetContent, SheetDescription, SheetTitle } from '@/components/ui/sheet' -import { VisuallyHidden } from 'radix-ui' import { Accordion, AccordionContent, @@ -186,8 +184,6 @@ import { PER_REPO_FETCH_LIMIT } from '../../../shared/work-items' import { translate } from '@/i18n/i18n' import { getSettingsForRepoRuntimeOwner } from '@/lib/repo-runtime-owner' -const IS_MAC = navigator.userAgent.includes('Mac') - // Why: the GH item dialog can be opened from any work-item list surface and // doesn't have the full owner/repo context the list's cache entry carries. // Parsing the canonical `https://github.com/{owner}/{repo}/...` URL is the @@ -283,7 +279,6 @@ type GitHubItemDialogProps = { repoId?: string | null sourceContext?: TaskSourceContext | null initialTab?: ItemDialogTab - variant?: 'sheet' | 'page' backLabel?: string /** Called when the user clicks the primary CTA to start work from this item. */ onUse: (item: GitHubWorkItem) => void @@ -5695,7 +5690,6 @@ export default function GitHubItemDialog({ repoId, sourceContext, initialTab, - variant = 'sheet', backLabel = 'Back', projectOrigin, onUse, @@ -6161,7 +6155,7 @@ export default function GitHubItemDialog({ [details?.pullRequestId, detailsCacheKey, repoPath, sourceContext, workItem] ) - const isIssuePage = variant === 'page' && workItem?.type === 'issue' + const isIssuePage = workItem?.type === 'issue' const ownerRepo = workItem ? parseOwnerRepoFromItemUrl(workItem.url) : null const issueStateBadgeTone = localState === 'closed' ? 'bg-rose-600 text-white' : 'bg-emerald-600 text-white' @@ -6367,19 +6361,17 @@ export default function GitHubItemDialog({ ) : (
- {variant === 'page' ? ( - - ) : null} +
@@ -6488,26 +6480,6 @@ export default function GitHubItemDialog({ {translate('auto.components.GitHubItemDialog.3fdf777817', 'Open on GitHub')} - {variant === 'sheet' ? ( - - - - - - {translate('auto.components.GitHubItemDialog.474c59b4b3', 'Close · Esc')} - - - ) : null}
@@ -6780,63 +6752,9 @@ export default function GitHubItemDialog({ ) : null - if (variant === 'page') { - return ( -
- {content} -
- ) - } - return ( - !open && onClose()}> - { - // Why: focusing the first actionable element inside the drawer - // causes the "Start workspace" action to receive focus and - // get visually highlighted on open. Preventing auto-focus keeps the - // drawer feeling like a passive preview until the user acts. - event.preventDefault() - }} - > - {/* Why: SheetTitle/Description are required by Radix Dialog for a11y, - but the visible header carries the same info. Wrap each with - `asChild` so the VisuallyHidden span wraps the element cleanly. */} - - - {workItem?.title ?? - translate('auto.components.GitHubItemDialog.3853476a97', 'GitHub item')} - - - - - {translate( - 'auto.components.GitHubItemDialog.3ab6ac0fc8', - 'Preview and edit the selected GitHub issue or pull request.' - )} - - - - {content} - - +
+ {content} +
) } diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index a52c55b3a0b..aa2749fce65 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -8586,7 +8586,7 @@ export default function TaskPage(): React.JSX.Element { initialTab={dialogInitialTab} repoPath={dialogRepoPath} repoId={dialogWorkItem.repoId} - variant="page" + sourceContext={dialogSourceContext} backLabel="GitHub list" onUse={(item) => { setDialogWorkItem(null) @@ -12074,24 +12074,6 @@ export default function TaskPage(): React.JSX.Element { - { - setDialogWorkItem(null) - handleUseWorkItem(item) - }} - onClose={() => setDialogWorkItem(null)} - /> - { + it('does not keep the stale right-side sheet owner', () => { + const source = componentSource('GitHubItemDialog.tsx') + + expect(source).not.toContain('@/components/ui/sheet') + expect(source).not.toContain(' { const source = componentSource('GitHubItemDialog.tsx') const section = sourceBetween(source, 'function PRReviewersPanel', 'function isPRFileViewed') diff --git a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx index c46d8120aba..f76361d3654 100644 --- a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx +++ b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx @@ -793,6 +793,33 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J } }} /> + ) : visibleTable && resolvedDialogRepoItem ? ( + { + const current = resolvedDialogRepoItem + setDialogRepoItem(null) + void launchWorkItemDirect({ + item, + repoId: current.workItem.repoId, + launchSource: 'task_page', + telemetrySource: 'sidebar', + openModalFallback: () => { + if (item.url) { + void window.api.shell.openUrl(item.url) + } + } + }) + }} + onClose={() => setDialogRepoItem(null)} + /> ) : visibleTable ? ( ) : null} - {/* Full repo-backed dialog — writes still go through slug-addressed - mutation helpers (see design §Dialog editing from Project rows, line - 707) so a row from another repo cannot accidentally edit the active - workspace. */} - { - const current = resolvedDialogRepoItem - setDialogRepoItem(null) - if (!current) { - return - } - void launchWorkItemDirect({ - item, - repoId: current.workItem.repoId, - launchSource: 'task_page', - telemetrySource: 'sidebar', - openModalFallback: () => { - if (item.url) { - void window.api.shell.openUrl(item.url) - } - } - }) - }} - onClose={() => setDialogRepoItem(null)} - /> - {/* Slug-only simplified dialog for rows whose repo isn't added to Orca. Why: no Start-work affordance lives inside the slug dialog — the parent's `handleStartWork`/`repoNotInOrca` modal owns that flow, so diff --git a/src/renderer/src/components/tab-bar/TabBar.windows-shell-launch.test.ts b/src/renderer/src/components/tab-bar/TabBar.windows-shell-launch.test.ts index 343a22b5e7b..83d50f864fa 100644 --- a/src/renderer/src/components/tab-bar/TabBar.windows-shell-launch.test.ts +++ b/src/renderer/src/components/tab-bar/TabBar.windows-shell-launch.test.ts @@ -81,6 +81,7 @@ vi.mock('react', async () => { return { ...actual, memo: (component: T) => component, + useCallback: unknown>(callback: T) => callback, useEffect: () => {}, useLayoutEffect: () => {}, useCallback: unknown>(callback: T) => callback, diff --git a/src/renderer/src/components/task-page-source-switch-boundary.test.ts b/src/renderer/src/components/task-page-source-switch-boundary.test.ts index 8207399c98b..b2dee55ce00 100644 --- a/src/renderer/src/components/task-page-source-switch-boundary.test.ts +++ b/src/renderer/src/components/task-page-source-switch-boundary.test.ts @@ -13,6 +13,23 @@ function sourceBetween(source: string, startPattern: string, endPattern: string) } describe('TaskPage source switching host boundary', () => { + it('renders GitHub item details from the task-detail page owner only', () => { + const detailSection = sourceBetween( + TASK_PAGE_SOURCE, + "{taskSource === 'github' && dialogWorkItem ?", + ") : taskSource === 'github' && githubMode === 'project' ?" + ) + const modalSection = sourceBetween( + TASK_PAGE_SOURCE, + ' { const section = sourceBetween( TASK_PAGE_SOURCE,