Use overlay sheet for GitHub item drawer on new-workspace page (#785)

This commit is contained in:
Neil 2026-04-17 18:20:54 -07:00 committed by GitHub
parent 818210f3e2
commit 3d40d4a3db
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 370 additions and 223 deletions

View file

@ -14,10 +14,11 @@ import {
X
} from 'lucide-react'
import { Button } from '@/components/ui/button'
import { Sheet, SheetContent, SheetDescription, SheetTitle } from '@/components/ui/sheet'
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { VisuallyHidden } from 'radix-ui'
import CommentMarkdown from '@/components/sidebar/CommentMarkdown'
import { useSidebarResize } from '@/hooks/useSidebarResize'
import { detectLanguage } from '@/lib/language-detect'
import { cn } from '@/lib/utils'
import { CHECK_COLOR, CHECK_ICON } from '@/components/right-sidebar/checks-helpers'
@ -33,10 +34,6 @@ import type {
// pulled into the drawer's bundle until the user actually opens the Files tab.
const DiffViewer = lazy(() => import('@/components/editor/DiffViewer'))
const DRAWER_MIN_WIDTH = 420
const DRAWER_MAX_WIDTH = 920
const DRAWER_DEFAULT_WIDTH = 560
type DrawerTab = 'conversation' | 'files' | 'checks'
type GitHubItemDrawerProps = {
@ -441,22 +438,12 @@ export default function GitHubItemDrawer({
repoPath,
onUse,
onClose
}: GitHubItemDrawerProps): React.JSX.Element | null {
const [width, setWidth] = useState(DRAWER_DEFAULT_WIDTH)
}: GitHubItemDrawerProps): React.JSX.Element {
const [tab, setTab] = useState<DrawerTab>('conversation')
const [details, setDetails] = useState<GitHubWorkItemDetails | null>(null)
const [loading, setLoading] = useState(false)
const [error, setError] = useState<string | null>(null)
const { containerRef, isResizing, onResizeStart } = useSidebarResize<HTMLDivElement>({
isOpen: workItem !== null,
width,
minWidth: DRAWER_MIN_WIDTH,
maxWidth: DRAWER_MAX_WIDTH,
deltaSign: -1,
setWidth
})
const requestIdRef = useRef(0)
useEffect(() => {
@ -497,225 +484,228 @@ export default function GitHubItemDrawer({
})
}, [repoPath, workItem])
useEffect(() => {
if (!workItem) {
return
}
const handler = (event: KeyboardEvent): void => {
if (event.key === 'Escape') {
event.preventDefault()
onClose()
}
}
window.addEventListener('keydown', handler)
return () => window.removeEventListener('keydown', handler)
}, [onClose, workItem])
if (!workItem) {
return null
}
const Icon = workItem.type === 'pr' ? GitPullRequest : CircleDot
const Icon = workItem?.type === 'pr' ? GitPullRequest : CircleDot
const body = details?.body ?? ''
const comments = details?.comments ?? []
const files = details?.files ?? []
const checks = details?.checks ?? []
return (
<div
ref={containerRef}
style={{ width: `${width}px` }}
className={cn(
'relative flex h-full shrink-0 flex-col border-l border-border/60 bg-card shadow-xl',
isResizing && 'select-none'
)}
>
{/* Left-edge resize handle */}
<div
onMouseDown={onResizeStart}
className="absolute left-0 top-0 z-10 h-full w-1 cursor-col-resize bg-transparent hover:bg-primary/40"
role="separator"
aria-orientation="vertical"
aria-label="Resize drawer"
/>
// Why: the overlay sheet pops over page content rather than docking a
// right column — Radix's Dialog handles focus trap, Esc-to-close, and
// overlay click-outside, and the `slide-in-from-right` animation gives it
// the Mantine-style drawer feel while keeping us on shadcn primitives.
<Sheet open={workItem !== null} onOpenChange={(open) => !open && onClose()}>
<SheetContent
side="right"
showCloseButton={false}
className="w-full p-0 sm:max-w-[640px]"
onOpenAutoFocus={(event) => {
// Why: focusing the first actionable element inside the drawer
// causes the "Start workspace" footer button 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 already carries the same info. Wrap each
individually with `asChild` so the visually-hidden span wraps the
element cleanly nesting a <h2>/<p> inside a single <span> would
be invalid HTML. */}
<VisuallyHidden.Root asChild>
<SheetTitle>{workItem?.title ?? 'GitHub item'}</SheetTitle>
</VisuallyHidden.Root>
<VisuallyHidden.Root asChild>
<SheetDescription>
Read-only preview of the selected GitHub issue or pull request.
</SheetDescription>
</VisuallyHidden.Root>
{/* Header */}
<div className="flex-none border-b border-border/60 px-4 py-3">
<div className="flex items-start gap-2">
<Icon className="mt-1 size-4 shrink-0 text-muted-foreground" />
<div className="min-w-0 flex-1">
<div className="flex items-center gap-2">
<span
className={cn(
'rounded-full border px-2 py-0.5 text-[11px] font-medium',
getStateTone(workItem)
)}
>
{getStateLabel(workItem)}
</span>
<span className="font-mono text-[12px] text-muted-foreground">
#{workItem.number}
</span>
</div>
<h2 className="mt-1 text-[15px] font-semibold leading-tight text-foreground">
{workItem.title}
</h2>
<div className="mt-1 flex flex-wrap items-center gap-x-2 gap-y-1 text-[11px] text-muted-foreground">
<span>{workItem.author ?? 'unknown'}</span>
<span>· {formatRelativeTime(workItem.updatedAt)}</span>
{workItem.branchName && (
<span className="font-mono text-[10px] text-muted-foreground/80">
· {workItem.branchName}
</span>
)}
</div>
{workItem.labels.length > 0 && (
<div className="mt-2 flex flex-wrap gap-1">
{workItem.labels.map((label) => (
<span
key={label}
className="rounded-full border border-border/50 bg-background/60 px-2 py-0.5 text-[10px] text-muted-foreground"
>
{label}
</span>
))}
</div>
)}
</div>
<div className="flex shrink-0 items-center gap-1">
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
className="size-7"
onClick={() => window.api.shell.openUrl(workItem.url)}
aria-label="Open on GitHub"
>
<ExternalLink className="size-4" />
</Button>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={6}>
Open on GitHub
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
className="size-7"
onClick={onClose}
aria-label="Close drawer"
>
<X className="size-4" />
</Button>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={6}>
Close · Esc
</TooltipContent>
</Tooltip>
</div>
</div>
</div>
{/* Tabs + body */}
<div className="min-h-0 flex-1">
{error ? (
<div className="px-4 py-6 text-[12px] text-destructive">{error}</div>
) : (
<Tabs
value={tab}
onValueChange={(value) => setTab(value as DrawerTab)}
className="flex h-full min-h-0 flex-col gap-0"
>
<TabsList
variant="line"
className="mx-4 mt-2 justify-start gap-3 border-b border-border/60"
>
<TabsTrigger value="conversation" className="px-2">
<MessageSquare className="size-3.5" />
Conversation
</TabsTrigger>
{workItem.type === 'pr' && (
<>
<TabsTrigger value="files" className="px-2">
<FileText className="size-3.5" />
Files
{files.length > 0 && (
<span className="ml-1 text-[10px] text-muted-foreground">{files.length}</span>
)}
</TabsTrigger>
<TabsTrigger value="checks" className="px-2">
Checks
{checks.length > 0 && (
<span className="ml-1 text-[10px] text-muted-foreground">
{checks.length}
{workItem && (
<div className="flex h-full min-h-0 flex-col">
{/* Header */}
<div className="flex-none border-b border-border/60 px-4 py-3">
<div className="flex items-start gap-2">
<Icon className="mt-1 size-4 shrink-0 text-muted-foreground" />
<div className="min-w-0 flex-1">
<div className="flex items-center gap-2">
<span
className={cn(
'rounded-full border px-2 py-0.5 text-[11px] font-medium',
getStateTone(workItem)
)}
>
{getStateLabel(workItem)}
</span>
<span className="font-mono text-[12px] text-muted-foreground">
#{workItem.number}
</span>
</div>
<h2 className="mt-1 text-[15px] font-semibold leading-tight text-foreground">
{workItem.title}
</h2>
<div className="mt-1 flex flex-wrap items-center gap-x-2 gap-y-1 text-[11px] text-muted-foreground">
<span>{workItem.author ?? 'unknown'}</span>
<span>· {formatRelativeTime(workItem.updatedAt)}</span>
{workItem.branchName && (
<span className="font-mono text-[10px] text-muted-foreground/80">
· {workItem.branchName}
</span>
)}
</TabsTrigger>
</>
)}
</TabsList>
<div className="min-h-0 flex-1 overflow-y-auto scrollbar-sleek">
<TabsContent value="conversation" className="mt-0">
<ConversationTab
item={workItem}
body={body}
comments={comments}
loading={loading}
/>
</TabsContent>
{workItem.type === 'pr' && (
<TabsContent value="files" className="mt-0">
{loading && files.length === 0 ? (
<div className="flex items-center justify-center py-10">
<LoaderCircle className="size-5 animate-spin text-muted-foreground" />
</div>
) : files.length === 0 ? (
<div className="px-4 py-10 text-center text-[12px] text-muted-foreground">
No files changed.
</div>
) : (
<div>
{files.map((file) => (
<PRFileRow
key={file.path}
file={file}
repoPath={repoPath ?? ''}
prNumber={workItem.number}
headSha={details?.headSha}
baseSha={details?.baseSha}
/>
</div>
{workItem.labels.length > 0 && (
<div className="mt-2 flex flex-wrap gap-1">
{workItem.labels.map((label) => (
<span
key={label}
className="rounded-full border border-border/50 bg-background/60 px-2 py-0.5 text-[10px] text-muted-foreground"
>
{label}
</span>
))}
</div>
)}
</TabsContent>
)}
</div>
<div className="flex shrink-0 items-center gap-1">
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
className="size-7"
onClick={() => window.api.shell.openUrl(workItem.url)}
aria-label="Open on GitHub"
>
<ExternalLink className="size-4" />
</Button>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={6}>
Open on GitHub
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
className="size-7"
onClick={onClose}
aria-label="Close preview"
>
<X className="size-4" />
</Button>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={6}>
Close · Esc
</TooltipContent>
</Tooltip>
</div>
</div>
</div>
{workItem.type === 'pr' && (
<TabsContent value="checks" className="mt-0">
<ChecksTab checks={checks} loading={loading} />
</TabsContent>
{/* Tabs + body */}
<div className="min-h-0 flex-1">
{error ? (
<div className="px-4 py-6 text-[12px] text-destructive">{error}</div>
) : (
<Tabs
value={tab}
onValueChange={(value) => setTab(value as DrawerTab)}
className="flex h-full min-h-0 flex-col gap-0"
>
<TabsList
variant="line"
className="mx-4 mt-2 justify-start gap-3 border-b border-border/60"
>
<TabsTrigger value="conversation" className="px-2">
<MessageSquare className="size-3.5" />
Conversation
</TabsTrigger>
{workItem.type === 'pr' && (
<>
<TabsTrigger value="files" className="px-2">
<FileText className="size-3.5" />
Files
{files.length > 0 && (
<span className="ml-1 text-[10px] text-muted-foreground">
{files.length}
</span>
)}
</TabsTrigger>
<TabsTrigger value="checks" className="px-2">
Checks
{checks.length > 0 && (
<span className="ml-1 text-[10px] text-muted-foreground">
{checks.length}
</span>
)}
</TabsTrigger>
</>
)}
</TabsList>
<div className="min-h-0 flex-1 overflow-y-auto scrollbar-sleek">
<TabsContent value="conversation" className="mt-0">
<ConversationTab
item={workItem}
body={body}
comments={comments}
loading={loading}
/>
</TabsContent>
{workItem.type === 'pr' && (
<TabsContent value="files" className="mt-0">
{loading && files.length === 0 ? (
<div className="flex items-center justify-center py-10">
<LoaderCircle className="size-5 animate-spin text-muted-foreground" />
</div>
) : files.length === 0 ? (
<div className="px-4 py-10 text-center text-[12px] text-muted-foreground">
No files changed.
</div>
) : (
<div>
{files.map((file) => (
<PRFileRow
key={file.path}
file={file}
repoPath={repoPath ?? ''}
prNumber={workItem.number}
headSha={details?.headSha}
baseSha={details?.baseSha}
/>
))}
</div>
)}
</TabsContent>
)}
{workItem.type === 'pr' && (
<TabsContent value="checks" className="mt-0">
<ChecksTab checks={checks} loading={loading} />
</TabsContent>
)}
</div>
</Tabs>
)}
</div>
</Tabs>
)}
</div>
{/* Footer */}
<div className="flex-none border-t border-border/60 bg-background/40 px-4 py-3">
<Button
onClick={() => onUse(workItem)}
className="w-full justify-center gap-2"
aria-label={`Start workspace from ${workItem.type === 'pr' ? 'PR' : 'issue'}`}
>
{`Start workspace from ${workItem.type === 'pr' ? 'PR' : 'issue'}`}
<ArrowRight className="size-4" />
</Button>
</div>
</div>
{/* Footer */}
<div className="flex-none border-t border-border/60 bg-background/40 px-4 py-3">
<Button
onClick={() => onUse(workItem)}
className="w-full justify-center gap-2"
aria-label={`Start workspace from ${workItem.type === 'pr' ? 'PR' : 'issue'}`}
>
{`Start workspace from ${workItem.type === 'pr' ? 'PR' : 'issue'}`}
<ArrowRight className="size-4" />
</Button>
</div>
</div>
)}
</SheetContent>
</Sheet>
)
}

View file

@ -359,6 +359,14 @@ export default function NewWorkspacePage(): React.JSX.Element {
return
}
// Why: when the GitHub preview sheet is open, Radix's Dialog owns Esc —
// it closes the sheet on its own. Page-level capture would otherwise fire
// first and discard the whole draft while the user just meant to dismiss
// the preview.
if (drawerWorkItem) {
return
}
const onKeyDown = (event: KeyboardEvent): void => {
if (event.key !== 'Enter' && event.key !== 'Escape') {
return
@ -407,7 +415,7 @@ export default function NewWorkspacePage(): React.JSX.Element {
window.addEventListener('keydown', onKeyDown, { capture: true })
return () => window.removeEventListener('keydown', onKeyDown, { capture: true })
}, [activeModal, composerRef, createDisabled, handleDiscardDraft, submit])
}, [activeModal, composerRef, createDisabled, drawerWorkItem, handleDiscardDraft, submit])
return (
<div className="relative flex h-full min-h-0 flex-1 overflow-hidden bg-background dark:bg-[#1a1a1a] text-foreground">
@ -434,8 +442,9 @@ export default function NewWorkspacePage(): React.JSX.Element {
)}
<div className="relative z-10 flex min-h-0 flex-1 flex-col">
{/* Why: the Esc/discard button is left-aligned to avoid colliding with the
right-docked GitHub drawer and app sidebar, which also live on the right edge. */}
{/* Why: left-aligned so it doesn't collide with the app sidebar on the
right edge. The GitHub preview is a modal sheet that overlays the
whole surface, so this button is hidden behind it while it's open. */}
<div className="flex-none flex items-center justify-start px-5 py-3 md:px-8 md:py-4">
<Tooltip>
<TooltipTrigger asChild>

View file

@ -0,0 +1,148 @@
'use client'
import * as React from 'react'
import { XIcon } from 'lucide-react'
import { Dialog as SheetPrimitive } from 'radix-ui'
import { cva, type VariantProps } from 'class-variance-authority'
import { cn } from '@/lib/utils'
function Sheet({ ...props }: React.ComponentProps<typeof SheetPrimitive.Root>) {
return <SheetPrimitive.Root data-slot="sheet" {...props} />
}
function SheetTrigger({ ...props }: React.ComponentProps<typeof SheetPrimitive.Trigger>) {
return <SheetPrimitive.Trigger data-slot="sheet-trigger" {...props} />
}
function SheetClose({ ...props }: React.ComponentProps<typeof SheetPrimitive.Close>) {
return <SheetPrimitive.Close data-slot="sheet-close" {...props} />
}
function SheetPortal({ ...props }: React.ComponentProps<typeof SheetPrimitive.Portal>) {
return <SheetPrimitive.Portal data-slot="sheet-portal" {...props} />
}
function SheetOverlay({
className,
...props
}: React.ComponentProps<typeof SheetPrimitive.Overlay>) {
return (
<SheetPrimitive.Overlay
data-slot="sheet-overlay"
className={cn(
'fixed inset-0 z-50 bg-black/50 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:animate-in data-[state=open]:fade-in-0',
className
)}
{...props}
/>
)
}
const sheetContentVariants = cva(
'fixed z-50 flex flex-col gap-0 bg-background shadow-lg outline-none transition ease-in-out data-[state=closed]:animate-out data-[state=closed]:duration-200 data-[state=open]:animate-in data-[state=open]:duration-300',
{
variants: {
side: {
right:
'inset-y-0 right-0 h-full w-3/4 border-l border-border/60 data-[state=closed]:slide-out-to-right data-[state=open]:slide-in-from-right sm:max-w-[560px]',
left: 'inset-y-0 left-0 h-full w-3/4 border-r border-border/60 data-[state=closed]:slide-out-to-left data-[state=open]:slide-in-from-left sm:max-w-[560px]',
top: 'inset-x-0 top-0 h-auto border-b border-border/60 data-[state=closed]:slide-out-to-top data-[state=open]:slide-in-from-top',
bottom:
'inset-x-0 bottom-0 h-auto border-t border-border/60 data-[state=closed]:slide-out-to-bottom data-[state=open]:slide-in-from-bottom'
}
},
defaultVariants: {
side: 'right'
}
}
)
function SheetContent({
className,
children,
side = 'right',
showCloseButton = true,
...props
}: React.ComponentProps<typeof SheetPrimitive.Content> &
VariantProps<typeof sheetContentVariants> & {
showCloseButton?: boolean
}) {
return (
<SheetPortal>
<SheetOverlay />
<SheetPrimitive.Content
data-slot="sheet-content"
className={cn(sheetContentVariants({ side }), className)}
{...props}
>
{children}
{showCloseButton && (
<SheetPrimitive.Close
data-slot="sheet-close"
className="absolute top-4 right-4 rounded-xs opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:ring-2 focus:ring-ring focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4"
>
<XIcon />
<span className="sr-only">Close</span>
</SheetPrimitive.Close>
)}
</SheetPrimitive.Content>
</SheetPortal>
)
}
function SheetHeader({ className, ...props }: React.ComponentProps<'div'>) {
return (
<div
data-slot="sheet-header"
className={cn('flex flex-col gap-1.5 p-4', className)}
{...props}
/>
)
}
function SheetFooter({ className, ...props }: React.ComponentProps<'div'>) {
return (
<div
data-slot="sheet-footer"
className={cn('mt-auto flex flex-col gap-2 p-4', className)}
{...props}
/>
)
}
function SheetTitle({ className, ...props }: React.ComponentProps<typeof SheetPrimitive.Title>) {
return (
<SheetPrimitive.Title
data-slot="sheet-title"
className={cn('text-base font-semibold text-foreground', className)}
{...props}
/>
)
}
function SheetDescription({
className,
...props
}: React.ComponentProps<typeof SheetPrimitive.Description>) {
return (
<SheetPrimitive.Description
data-slot="sheet-description"
className={cn('text-sm text-muted-foreground', className)}
{...props}
/>
)
}
export {
Sheet,
SheetClose,
SheetContent,
SheetDescription,
SheetFooter,
SheetHeader,
SheetOverlay,
SheetPortal,
SheetTitle,
SheetTrigger
}