mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 16:02:35 +00:00
Fix stale GitHub task drawer (#5424)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -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({
|
||||
) : (
|
||||
<div className="flex-none border-b border-border/60 bg-card/80 px-4 py-3 shadow-xs backdrop-blur supports-[backdrop-filter]:bg-card/70">
|
||||
<div className="flex items-start gap-3">
|
||||
{variant === 'page' ? (
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
onClick={onClose}
|
||||
className="-ml-1 mt-0.5 shrink-0 gap-1.5"
|
||||
aria-label={backLabel}
|
||||
>
|
||||
<ChevronLeft className="size-4" />
|
||||
{backLabel}
|
||||
</Button>
|
||||
) : null}
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
onClick={onClose}
|
||||
className="-ml-1 mt-0.5 shrink-0 gap-1.5"
|
||||
aria-label={backLabel}
|
||||
>
|
||||
<ChevronLeft className="size-4" />
|
||||
{backLabel}
|
||||
</Button>
|
||||
<div className="mt-0.5 flex size-8 shrink-0 items-center justify-center rounded-md border border-border/60 bg-muted/40 text-muted-foreground">
|
||||
<Icon className="size-4" />
|
||||
</div>
|
||||
@@ -6488,26 +6480,6 @@ export default function GitHubItemDialog({
|
||||
{translate('auto.components.GitHubItemDialog.3fdf777817', 'Open on GitHub')}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
{variant === 'sheet' ? (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
onClick={onClose}
|
||||
aria-label={translate(
|
||||
'auto.components.GitHubItemDialog.45af57999b',
|
||||
'Close preview'
|
||||
)}
|
||||
>
|
||||
<X className="size-4" />
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={6}>
|
||||
{translate('auto.components.GitHubItemDialog.474c59b4b3', 'Close · Esc')}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
@@ -6780,63 +6752,9 @@ export default function GitHubItemDialog({
|
||||
</div>
|
||||
) : null
|
||||
|
||||
if (variant === 'page') {
|
||||
return (
|
||||
<div className="flex h-full min-h-0 flex-col overflow-hidden rounded-md border border-border/50 bg-background shadow-sm">
|
||||
{content}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
return (
|
||||
<Sheet open={workItem !== null} onOpenChange={(open) => !open && onClose()}>
|
||||
<SheetContent
|
||||
side="right"
|
||||
showCloseButton={false}
|
||||
className={cn(
|
||||
'flex w-full flex-col gap-0 overflow-hidden p-0 lg:max-w-[var(--github-item-dialog-max-width)]',
|
||||
// Why: native macOS traffic lights are drawn above web content, so a
|
||||
// nearly full-width right sheet must leave the titlebar's 80px
|
||||
// traffic-light pad uncovered instead of relying on z-index.
|
||||
IS_MAC
|
||||
? 'max-w-[calc(100vw-(80px/var(--ui-zoom-factor,1)))] sm:max-w-[calc(100vw-(80px/var(--ui-zoom-factor,1)))]'
|
||||
: 'max-w-[calc(100vw-1rem)] sm:max-w-[calc(100vw-1rem)]'
|
||||
)}
|
||||
style={
|
||||
{
|
||||
'--github-item-dialog-max-width': IS_MAC
|
||||
? 'min(calc(100vw - (80px / var(--ui-zoom-factor, 1))), 1600px)'
|
||||
: 'min(calc(100vw - 2rem), 1600px)'
|
||||
} as React.CSSProperties
|
||||
}
|
||||
onOpenAutoFocus={(event) => {
|
||||
// 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. */}
|
||||
<VisuallyHidden.Root asChild>
|
||||
<SheetTitle>
|
||||
{workItem?.title ??
|
||||
translate('auto.components.GitHubItemDialog.3853476a97', 'GitHub item')}
|
||||
</SheetTitle>
|
||||
</VisuallyHidden.Root>
|
||||
<VisuallyHidden.Root asChild>
|
||||
<SheetDescription>
|
||||
{translate(
|
||||
'auto.components.GitHubItemDialog.3ab6ac0fc8',
|
||||
'Preview and edit the selected GitHub issue or pull request.'
|
||||
)}
|
||||
</SheetDescription>
|
||||
</VisuallyHidden.Root>
|
||||
|
||||
{content}
|
||||
</SheetContent>
|
||||
</Sheet>
|
||||
<div className="flex h-full min-h-0 flex-col overflow-hidden rounded-md border border-border/50 bg-background shadow-sm">
|
||||
{content}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
|
||||
<GitHubItemDialog
|
||||
workItem={dialogWorkItem}
|
||||
repoPath={
|
||||
// Why: the dialog is for a single item — resolve its repoPath from the
|
||||
// item's own repoId (set when fan-out merged the list) so it works in
|
||||
// cross-repo mode too. Reusing the memoized repo map avoids an O(n)
|
||||
// scan on every render while the dialog is open.
|
||||
dialogWorkItem ? (repoMap.get(dialogWorkItem.repoId)?.path ?? null) : null
|
||||
}
|
||||
repoId={dialogWorkItem?.repoId ?? null}
|
||||
sourceContext={dialogSourceContext}
|
||||
onUse={(item) => {
|
||||
setDialogWorkItem(null)
|
||||
handleUseWorkItem(item)
|
||||
}}
|
||||
onClose={() => setDialogWorkItem(null)}
|
||||
/>
|
||||
|
||||
<GitLabItemDialog
|
||||
item={gitlabDialogItem}
|
||||
// Why: dialog's repoPath has to come from the clicked item's
|
||||
|
||||
@@ -17,6 +17,15 @@ function sourceBetween(source: string, startPattern: string, endPattern: string)
|
||||
}
|
||||
|
||||
describe('GitHubItemDialog source host boundaries', () => {
|
||||
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('<Sheet')
|
||||
expect(source).not.toContain('<SheetContent')
|
||||
expect(source).not.toContain("variant?: 'sheet'")
|
||||
})
|
||||
|
||||
it('routes reviewer metadata and reviewer mutations through the task source context', () => {
|
||||
const source = componentSource('GitHubItemDialog.tsx')
|
||||
const section = sourceBetween(source, 'function PRReviewersPanel', 'function isPRFileViewed')
|
||||
|
||||
@@ -793,6 +793,33 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J
|
||||
}
|
||||
}}
|
||||
/>
|
||||
) : visibleTable && resolvedDialogRepoItem ? (
|
||||
<GitHubItemDialog
|
||||
workItem={resolvedDialogRepoItem.workItem}
|
||||
repoPath={resolvedDialogRepoItem.repoPath}
|
||||
repoId={resolvedDialogRepoItem.repoId}
|
||||
projectOrigin={resolvedDialogRepoItem.origin}
|
||||
backLabel={translate(
|
||||
'auto.components.github.project.ProjectViewWrapper.1aa7c952b9',
|
||||
'Project view'
|
||||
)}
|
||||
onUse={(item) => {
|
||||
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 ? (
|
||||
<ProjectViewList
|
||||
table={visibleTable}
|
||||
@@ -811,36 +838,6 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J
|
||||
/>
|
||||
) : 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. */}
|
||||
<GitHubItemDialog
|
||||
workItem={resolvedDialogRepoItem?.workItem ?? null}
|
||||
repoPath={resolvedDialogRepoItem?.repoPath ?? null}
|
||||
repoId={resolvedDialogRepoItem?.repoId ?? null}
|
||||
projectOrigin={resolvedDialogRepoItem?.origin}
|
||||
onUse={(item) => {
|
||||
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
|
||||
|
||||
@@ -81,6 +81,7 @@ vi.mock('react', async () => {
|
||||
return {
|
||||
...actual,
|
||||
memo: <T>(component: T) => component,
|
||||
useCallback: <T extends (...args: never[]) => unknown>(callback: T) => callback,
|
||||
useEffect: () => {},
|
||||
useLayoutEffect: () => {},
|
||||
useCallback: <T extends (...args: never[]) => unknown>(callback: T) => callback,
|
||||
|
||||
@@ -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,
|
||||
'<Dialog\n open={newJiraIssueOpen}',
|
||||
'<GitLabItemDialog'
|
||||
)
|
||||
|
||||
expect(detailSection).toContain('<GitHubItemDialog')
|
||||
expect(detailSection).toContain('sourceContext={dialogSourceContext}')
|
||||
expect(modalSection).not.toContain('<GitHubItemDialog')
|
||||
})
|
||||
|
||||
it('switches task source without mutating the focused run host', () => {
|
||||
const section = sourceBetween(
|
||||
TASK_PAGE_SOURCE,
|
||||
|
||||
Reference in New Issue
Block a user