mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 08:03:37 +00:00
Improve PR page error messages, accessibility, and stability
- Show actual error messages from failed operations instead of generic fallbacks - Add aria attributes for combobox/listbox patterns and proper option identifiers - Consolidate duplicate label/assignee update logic and fix event listener passive mode - Memoize GitHub source runtime to prevent stale closure in checks callbacks - Extract filled state badge tone for reuse and fix workspace attachment type - Guard textarea shortcuts against concurrent saves and add mention query test
This commit is contained in:
@@ -122,9 +122,7 @@ describe('PullRequestPage host boundaries', () => {
|
||||
expect(editSource).toContain(
|
||||
'args.patchWorkItem(args.item.id, { state: args.newState }, args.item.repoId, {'
|
||||
)
|
||||
expect(editSource).toContain(
|
||||
'args.patchWorkItem(args.item.id, { labels: newLabels }, args.item.repoId, {'
|
||||
)
|
||||
expect(editSource).toContain('args.patchWorkItem(args.item.id, { labels }, args.item.repoId, {')
|
||||
})
|
||||
|
||||
it('routes PR mention metadata through the PR repo owner host', () => {
|
||||
|
||||
@@ -183,9 +183,11 @@ export async function mergePullRequest(args: {
|
||||
}
|
||||
toast.success(translate('auto.components.PullRequestPage.c57873d721', 'Pull request merged'))
|
||||
args.onMutated()
|
||||
} catch {
|
||||
} catch (err) {
|
||||
toast.error(
|
||||
translate('auto.components.PullRequestPage.aae645d36d', 'Failed to merge pull request')
|
||||
err instanceof Error
|
||||
? err.message
|
||||
: translate('auto.components.PullRequestPage.aae645d36d', 'Failed to merge pull request')
|
||||
)
|
||||
} finally {
|
||||
args.setMergePending(false)
|
||||
@@ -256,11 +258,13 @@ export async function setPullRequestAutoMerge(args: {
|
||||
: translate('auto.components.PullRequestPage.0f5821b035', 'Auto-merge disabled')
|
||||
)
|
||||
args.onMutated()
|
||||
} catch {
|
||||
} catch (err) {
|
||||
toast.error(
|
||||
enabled
|
||||
? translate('auto.components.PullRequestPage.d31f4b508c', 'Failed to enable auto-merge')
|
||||
: translate('auto.components.PullRequestPage.973ef2fac9', 'Failed to disable auto-merge')
|
||||
err instanceof Error
|
||||
? err.message
|
||||
: enabled
|
||||
? translate('auto.components.PullRequestPage.d31f4b508c', 'Failed to enable auto-merge')
|
||||
: translate('auto.components.PullRequestPage.973ef2fac9', 'Failed to disable auto-merge')
|
||||
)
|
||||
} finally {
|
||||
args.setMergePending(false)
|
||||
|
||||
@@ -66,6 +66,8 @@ export function ChecksTab({
|
||||
const repo = useAppStore((s) =>
|
||||
targetRepoId ? (s.repos.find((candidate) => candidate.id === targetRepoId) ?? null) : null
|
||||
)
|
||||
const repos = useAppStore((s) => s.repos)
|
||||
const projects = useAppStore((s) => s.projects)
|
||||
const [fixingChecks, setFixingChecks] = useState(false)
|
||||
const [fixChecksComposerPrompt, setFixChecksComposerPrompt] = useState<string | null>(null)
|
||||
const mountedRef = useMountedRef()
|
||||
@@ -124,14 +126,23 @@ export function ChecksTab({
|
||||
projectRuntime: repo?.connectionId
|
||||
? undefined
|
||||
: getLocalRepoProjectExecutionRuntimeContext(
|
||||
useAppStore.getState(),
|
||||
// Why: repo-scoped resolution only reads repos/projects/settings, and subscribing keeps the launch platform fresh.
|
||||
{
|
||||
activeRepoId: null,
|
||||
activeWorktreeId: null,
|
||||
projects,
|
||||
repos,
|
||||
settings,
|
||||
worktreesByRepo: {}
|
||||
},
|
||||
repo?.id,
|
||||
CLIENT_PLATFORM
|
||||
)
|
||||
}),
|
||||
[repo?.connectionId, repo?.id, repo?.path]
|
||||
[projects, repo?.connectionId, repo?.id, repo?.path, repos, settings]
|
||||
)
|
||||
const runtimeHost = getGitHubSourceRuntimeHost(sourceContext)
|
||||
// Why: parses a fresh host object per call, so memoize to keep the check action callbacks stable.
|
||||
const runtimeHost = useMemo(() => getGitHubSourceRuntimeHost(sourceContext), [sourceContext])
|
||||
const canUseChecksRepoContext = canUseGitHubRepoContext(repoPath, sourceContext)
|
||||
const sorted = sortChecksBySeverity(list)
|
||||
const failedChecks = getBrokenChecks(list)
|
||||
|
||||
@@ -55,7 +55,6 @@ export function ConversationCommentCard({
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<div
|
||||
key={comment.id}
|
||||
className={cn(
|
||||
'min-w-0 overflow-hidden rounded-lg border border-border/40 bg-card shadow-xs',
|
||||
isReply && 'ml-6 max-w-[calc(100%-1.5rem)]',
|
||||
|
||||
@@ -118,12 +118,12 @@ export function ConversationDescription({
|
||||
value={resolvedBodyDraft}
|
||||
onValueChange={onDraftChange}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === 'Escape') {
|
||||
if (event.key === 'Escape' && !bodySaving) {
|
||||
event.preventDefault()
|
||||
onCancelEdit()
|
||||
return
|
||||
}
|
||||
if (isScreenSubmitShortcut(event)) {
|
||||
if (isScreenSubmitShortcut(event) && !bodySaving && bodyChanged) {
|
||||
event.preventDefault()
|
||||
onSave()
|
||||
}
|
||||
|
||||
@@ -96,37 +96,12 @@ export function toggleIssueLabel(args: {
|
||||
? [...prevLabels, args.label]
|
||||
: prevLabels.filter((l) => l !== args.label)
|
||||
|
||||
if (isAdding) {
|
||||
args.run('labels', {
|
||||
mutate: () =>
|
||||
runIssueUpdate({
|
||||
repoId: args.item.repoId,
|
||||
repoPath: args.repoPath,
|
||||
sourceContext: args.sourceContext,
|
||||
projectOrigin: args.projectOrigin,
|
||||
number: args.item.number,
|
||||
updates: { addLabels: [args.label] }
|
||||
}),
|
||||
onOptimistic: () => {
|
||||
args.onLabelsChange(newLabels)
|
||||
args.patchWorkItem(args.item.id, { labels: newLabels }, args.item.repoId, {
|
||||
sourceContext: args.sourceContext
|
||||
})
|
||||
args.patchProjectRowIfNeeded({ labels: newLabels })
|
||||
},
|
||||
onSuccess: () => {
|
||||
args.onMutated()
|
||||
},
|
||||
onRevert: () => {
|
||||
args.onLabelsChange(prevLabels)
|
||||
args.patchWorkItem(args.item.id, { labels: prevLabels }, args.item.repoId, {
|
||||
sourceContext: args.sourceContext
|
||||
})
|
||||
args.patchProjectRowIfNeeded({ labels: prevLabels })
|
||||
},
|
||||
onError: (err) => toast.error(err)
|
||||
const applyLabels = (labels: string[]): void => {
|
||||
args.onLabelsChange(labels)
|
||||
args.patchWorkItem(args.item.id, { labels }, args.item.repoId, {
|
||||
sourceContext: args.sourceContext
|
||||
})
|
||||
return
|
||||
args.patchProjectRowIfNeeded({ labels })
|
||||
}
|
||||
|
||||
args.run('labels', {
|
||||
@@ -137,22 +112,10 @@ export function toggleIssueLabel(args: {
|
||||
sourceContext: args.sourceContext,
|
||||
projectOrigin: args.projectOrigin,
|
||||
number: args.item.number,
|
||||
updates: { removeLabels: [args.label] }
|
||||
updates: isAdding ? { addLabels: [args.label] } : { removeLabels: [args.label] }
|
||||
}),
|
||||
onOptimistic: () => {
|
||||
args.onLabelsChange(newLabels)
|
||||
args.patchWorkItem(args.item.id, { labels: newLabels }, args.item.repoId, {
|
||||
sourceContext: args.sourceContext
|
||||
})
|
||||
args.patchProjectRowIfNeeded({ labels: newLabels })
|
||||
},
|
||||
onRevert: () => {
|
||||
args.onLabelsChange(prevLabels)
|
||||
args.patchWorkItem(args.item.id, { labels: prevLabels }, args.item.repoId, {
|
||||
sourceContext: args.sourceContext
|
||||
})
|
||||
args.patchProjectRowIfNeeded({ labels: prevLabels })
|
||||
},
|
||||
onOptimistic: () => applyLabels(newLabels),
|
||||
onRevert: () => applyLabels(prevLabels),
|
||||
onSuccess: () => {
|
||||
args.onMutated()
|
||||
},
|
||||
@@ -182,31 +145,10 @@ export function toggleIssueAssignee(args: {
|
||||
|
||||
// Why: scope the optimistic guard to this repo item so switching items doesn't suppress the next item's assignee sync.
|
||||
args.editedAssigneesItemKeyRef.current = args.assigneesItemKey
|
||||
if (isAssigned) {
|
||||
args.run('assignees', {
|
||||
mutate: () =>
|
||||
runIssueUpdate({
|
||||
repoId: args.item.repoId,
|
||||
repoPath: args.repoPath,
|
||||
sourceContext: args.sourceContext,
|
||||
projectOrigin: args.projectOrigin,
|
||||
number: args.item.number,
|
||||
updates: { removeAssignees: [args.login] }
|
||||
}),
|
||||
onOptimistic: () => {
|
||||
args.setLocalAssignees(newAssignees)
|
||||
args.patchProjectRowIfNeeded({ assignees: newAssignees })
|
||||
},
|
||||
onRevert: () => {
|
||||
args.setLocalAssignees(prevAssignees)
|
||||
args.patchProjectRowIfNeeded({ assignees: prevAssignees })
|
||||
},
|
||||
onSuccess: () => {
|
||||
args.onMutated()
|
||||
},
|
||||
onError: (err) => toast.error(err)
|
||||
})
|
||||
return
|
||||
|
||||
const applyAssignees = (assignees: string[]): void => {
|
||||
args.setLocalAssignees(assignees)
|
||||
args.patchProjectRowIfNeeded({ assignees })
|
||||
}
|
||||
|
||||
args.run('assignees', {
|
||||
@@ -217,19 +159,13 @@ export function toggleIssueAssignee(args: {
|
||||
sourceContext: args.sourceContext,
|
||||
projectOrigin: args.projectOrigin,
|
||||
number: args.item.number,
|
||||
updates: { addAssignees: [args.login] }
|
||||
updates: isAssigned ? { removeAssignees: [args.login] } : { addAssignees: [args.login] }
|
||||
}),
|
||||
onOptimistic: () => {
|
||||
args.setLocalAssignees(newAssignees)
|
||||
args.patchProjectRowIfNeeded({ assignees: newAssignees })
|
||||
},
|
||||
onOptimistic: () => applyAssignees(newAssignees),
|
||||
onRevert: () => applyAssignees(prevAssignees),
|
||||
onSuccess: () => {
|
||||
args.onMutated()
|
||||
},
|
||||
onRevert: () => {
|
||||
args.setLocalAssignees(prevAssignees)
|
||||
args.patchProjectRowIfNeeded({ assignees: prevAssignees })
|
||||
},
|
||||
onError: (err) => toast.error(err)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -53,24 +53,28 @@ export function PRFilesDiffToolbar({
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex shrink-0 items-center gap-2">
|
||||
<button
|
||||
<Button
|
||||
type="button"
|
||||
className="w-20 text-left text-xs text-muted-foreground transition-colors hover:text-foreground"
|
||||
variant="ghost"
|
||||
size="xs"
|
||||
className="w-20 justify-start px-0 text-muted-foreground hover:bg-transparent hover:text-foreground"
|
||||
onClick={onToggleAllCollapsed}
|
||||
>
|
||||
{allSectionsCollapsed
|
||||
? translate('auto.components.PullRequestPage.eb722a5a8c', 'Expand All')
|
||||
: translate('auto.components.PullRequestPage.dd94111c18', 'Collapse All')}
|
||||
</button>
|
||||
<button
|
||||
</Button>
|
||||
<Button
|
||||
type="button"
|
||||
className="w-24 rounded border border-border px-2 py-0.5 text-center text-xs text-muted-foreground transition-colors hover:text-foreground"
|
||||
variant="outline"
|
||||
size="xs"
|
||||
className="w-24 justify-center text-muted-foreground shadow-none hover:text-foreground"
|
||||
onClick={onToggleSideBySide}
|
||||
>
|
||||
{sideBySide
|
||||
? translate('auto.components.PullRequestPage.e5f4a24f78', 'Inline')
|
||||
: translate('auto.components.PullRequestPage.1378d79e83', 'Side by Side')}
|
||||
</button>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
|
||||
@@ -77,7 +77,7 @@ export function usePRFilesDiffViewPersistence(args: {
|
||||
writeCachedScrollPosition(args.pendingRestoreScrollTopRef.current ?? container.scrollTop)
|
||||
}
|
||||
|
||||
container.addEventListener('scroll', cacheScrollPositionOnScroll)
|
||||
container.addEventListener('scroll', cacheScrollPositionOnScroll, { passive: true })
|
||||
return () => {
|
||||
cacheScrollPositionOnTeardown()
|
||||
container.removeEventListener('scroll', cacheScrollPositionOnScroll)
|
||||
|
||||
@@ -19,4 +19,8 @@ describe('findMentionQuery', () => {
|
||||
it('allows a mention after punctuation that starts a token', () => {
|
||||
expect(findMentionQuery('(@octo', 6)).toEqual({ atIndex: 1, query: 'octo' })
|
||||
})
|
||||
|
||||
it('reads only the text before the caret', () => {
|
||||
expect(findMentionQuery('hi @octo trailing', 8)).toEqual({ atIndex: 3, query: 'octo' })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import React, { useCallback, useMemo, useState } from 'react'
|
||||
import React, { useCallback, useId, useMemo, useState } from 'react'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { filterGitHubMentionOptions } from '@/components/github/github-mention-option-filter'
|
||||
import type { MentionOption, MentionQuery } from '../page-types'
|
||||
@@ -25,6 +25,7 @@ export function MentionTextarea({
|
||||
mentionOptions: MentionOption[]
|
||||
textareaRef: React.RefObject<HTMLTextAreaElement | null>
|
||||
}): React.JSX.Element {
|
||||
const listboxId = useId()
|
||||
const [mentionQuery, setMentionQuery] = useState<MentionQuery | null>(null)
|
||||
const [activeIndex, setActiveIndex] = useState(0)
|
||||
const suggestions = useMemo(
|
||||
@@ -64,10 +65,17 @@ export function MentionTextarea({
|
||||
return (
|
||||
<div className={cn('relative min-w-0 flex-1', wrapperClassName)}>
|
||||
{showSuggestions && (
|
||||
<div className="absolute right-0 bottom-[calc(100%+6px)] left-0 z-50 max-h-64 overflow-y-auto rounded-md border border-border/70 bg-popover p-1 text-popover-foreground shadow-lg scrollbar-sleek">
|
||||
<div
|
||||
id={listboxId}
|
||||
role="listbox"
|
||||
className="absolute right-0 bottom-[calc(100%+6px)] left-0 z-50 max-h-64 overflow-y-auto rounded-md border border-border/70 bg-popover p-1 text-popover-foreground shadow-lg scrollbar-sleek"
|
||||
>
|
||||
{suggestions.map((option, index) => (
|
||||
<button
|
||||
key={option.login}
|
||||
id={`${listboxId}-${index}`}
|
||||
role="option"
|
||||
aria-selected={index === activeIndex}
|
||||
type="button"
|
||||
onMouseDown={(event) => {
|
||||
event.preventDefault()
|
||||
@@ -102,6 +110,10 @@ export function MentionTextarea({
|
||||
)}
|
||||
<textarea
|
||||
ref={textareaRef}
|
||||
role="combobox"
|
||||
aria-expanded={showSuggestions}
|
||||
aria-controls={showSuggestions ? listboxId : undefined}
|
||||
aria-activedescendant={showSuggestions ? `${listboxId}-${activeIndex}` : undefined}
|
||||
value={value}
|
||||
onChange={(event) => {
|
||||
onValueChange(event.target.value)
|
||||
|
||||
@@ -25,6 +25,7 @@ import { formatRelativeTime, getStateLabel } from '@/components/github/work-item
|
||||
import { cn } from '@/lib/utils'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import type { GitHubWorkItem } from '../../../../../shared/github/work-item-types'
|
||||
import { getSolidStateTone } from '../presentation/state-badge'
|
||||
|
||||
export function PullRequestPageHeader({
|
||||
workItem,
|
||||
@@ -34,7 +35,7 @@ export function PullRequestPageHeader({
|
||||
linkCopied,
|
||||
setLinkCopyButtonRef,
|
||||
onCopyLink,
|
||||
attachedWorkspace,
|
||||
hasAttachedWorkspace,
|
||||
attachedWorkspaceLabel,
|
||||
localState,
|
||||
Icon,
|
||||
@@ -48,7 +49,7 @@ export function PullRequestPageHeader({
|
||||
linkCopied: boolean
|
||||
setLinkCopyButtonRef: (node: HTMLButtonElement | null) => void
|
||||
onCopyLink: () => void
|
||||
attachedWorkspace: unknown
|
||||
hasAttachedWorkspace: boolean
|
||||
attachedWorkspaceLabel: string | null
|
||||
localState: GitHubWorkItem['state']
|
||||
Icon: React.ComponentType<{ className?: string }>
|
||||
@@ -58,15 +59,9 @@ export function PullRequestPageHeader({
|
||||
const ownerRepo = parseOwnerRepoFromItemUrl(workItem.url)
|
||||
const headBranch = workItem.branchName
|
||||
const baseBranch = workItem.baseRefName
|
||||
const stateBadgeTone =
|
||||
localState === 'merged'
|
||||
? 'bg-purple-600 text-white'
|
||||
: localState === 'draft'
|
||||
? 'bg-slate-500 text-white'
|
||||
: localState === 'closed'
|
||||
? 'bg-rose-600 text-white'
|
||||
: 'bg-emerald-600 text-white'
|
||||
const stateBadgeLabel = getStateLabel({ ...workItem, state: localState })
|
||||
const stateBadgeItem = { ...workItem, state: localState }
|
||||
const stateBadgeTone = getSolidStateTone(stateBadgeItem)
|
||||
const stateBadgeLabel = getStateLabel(stateBadgeItem)
|
||||
|
||||
return (
|
||||
<>
|
||||
@@ -163,7 +158,7 @@ export function PullRequestPageHeader({
|
||||
onClick={onOpenOrUsePR}
|
||||
className="w-[180px] justify-center gap-1.5 whitespace-nowrap"
|
||||
aria-label={
|
||||
attachedWorkspace
|
||||
hasAttachedWorkspace
|
||||
? translate(
|
||||
'auto.components.PullRequestPage.a459866967',
|
||||
'Resume workspace attached to PR'
|
||||
@@ -174,7 +169,7 @@ export function PullRequestPageHeader({
|
||||
)
|
||||
}
|
||||
>
|
||||
{attachedWorkspace
|
||||
{hasAttachedWorkspace
|
||||
? translate('auto.components.PullRequestPage.c9e7094a7b', 'Resume workspace')
|
||||
: translate('auto.components.PullRequestPage.71a3c0f9d2', 'Start workspace')}
|
||||
<ArrowRight className="size-4" />
|
||||
@@ -193,7 +188,7 @@ export function PullRequestPageHeader({
|
||||
</DropdownMenuTrigger>
|
||||
</ButtonGroup>
|
||||
<DropdownMenuContent align="end">
|
||||
{attachedWorkspace ? (
|
||||
{hasAttachedWorkspace ? (
|
||||
<DropdownMenuItem onSelect={onUseWorkItem}>
|
||||
<Plus className="size-4" />
|
||||
{translate('auto.components.PullRequestPage.1a2570e18e', 'Start new workspace')}
|
||||
|
||||
@@ -27,10 +27,7 @@ import type { GitHubWorkItem } from '../../../../../shared/github/work-item-type
|
||||
import type { PullRequestPageProps } from '../page-types'
|
||||
import {
|
||||
invalidateWorkItemDetailsCacheByMatch,
|
||||
invalidateWorkItemDetailsCacheForKey,
|
||||
patchCachedPRChecks,
|
||||
patchCachedPRReviewRequests,
|
||||
patchCachedWorkItemBody
|
||||
invalidateWorkItemDetailsCacheForKey
|
||||
} from '../cache/work-item-details'
|
||||
import { GHEditSection } from '../edit/section'
|
||||
import { usePullRequestDetails } from './use-details'
|
||||
@@ -198,16 +195,22 @@ export default function PullRequestPage({
|
||||
return { ...workItem, ...details.item, repoId: workItem.repoId }
|
||||
}, [details?.item, workItem])
|
||||
|
||||
const workItemRepoId = workItem?.repoId
|
||||
useEffect(() => {
|
||||
if (!workItem || details?.item.reviewRequests === undefined) {
|
||||
if (
|
||||
workItemId === undefined ||
|
||||
workItemRepoId === undefined ||
|
||||
details?.item.reviewRequests === undefined
|
||||
) {
|
||||
return
|
||||
}
|
||||
// Why: PR details can carry fresher reviewer metadata than the list row; push it back so the Tasks review chip isn't stale.
|
||||
// Why: keyed on identity, not the whole item, so replacing the same PR object doesn't repush unchanged reviewers.
|
||||
onReviewRequestsChange?.(
|
||||
{ id: workItem.id, repoId: workItem.repoId },
|
||||
{ id: workItemId, repoId: workItemRepoId },
|
||||
details.item.reviewRequests
|
||||
)
|
||||
}, [details?.item.reviewRequests, onReviewRequestsChange, workItem])
|
||||
}, [details?.item.reviewRequests, onReviewRequestsChange, workItemId, workItemRepoId])
|
||||
|
||||
const body = details?.body ?? ''
|
||||
const comments = details?.comments ?? []
|
||||
@@ -319,7 +322,7 @@ export default function PullRequestPage({
|
||||
onCopyLink={() => {
|
||||
void handleCopyWorkItemLink()
|
||||
}}
|
||||
attachedWorkspace={attachedWorkspace}
|
||||
hasAttachedWorkspace={attachedWorkspace !== null}
|
||||
attachedWorkspaceLabel={attachedWorkspaceLabel}
|
||||
localState={localState}
|
||||
Icon={Icon}
|
||||
@@ -373,9 +376,6 @@ export default function PullRequestPage({
|
||||
appendOptimisticComment={appendOptimisticComment}
|
||||
handlePRFileViewedChange={handlePRFileViewedChange}
|
||||
onReviewRequestsChange={onReviewRequestsChange}
|
||||
patchCachedPRChecks={patchCachedPRChecks}
|
||||
patchCachedWorkItemBody={patchCachedWorkItemBody}
|
||||
patchCachedPRReviewRequests={patchCachedPRReviewRequests}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -19,6 +19,11 @@ import type { PRCheckDetail } from '../../../../../shared/github/check-types'
|
||||
import type { PRComment } from '../../../../../shared/github/comment-types'
|
||||
import type { TaskSourceContext } from '../../../../../shared/task-source-context'
|
||||
import type { PullRequestPageProjectOrigin } from '../page-types'
|
||||
import {
|
||||
patchCachedPRChecks,
|
||||
patchCachedPRReviewRequests,
|
||||
patchCachedWorkItemBody
|
||||
} from '../cache/work-item-details'
|
||||
import { ConversationTab } from '../conversation/tab'
|
||||
import { ChecksTab } from '../checks/tab'
|
||||
import { PRFilesCombinedDiffViewer } from '../files/combined-diff-viewer'
|
||||
@@ -47,10 +52,7 @@ export function PullRequestPageTabs({
|
||||
invalidateCurrentDetailsCache,
|
||||
appendOptimisticComment,
|
||||
handlePRFileViewedChange,
|
||||
onReviewRequestsChange,
|
||||
patchCachedPRChecks,
|
||||
patchCachedWorkItemBody,
|
||||
patchCachedPRReviewRequests
|
||||
onReviewRequestsChange
|
||||
}: {
|
||||
tab: ItemDialogTab
|
||||
onTabChange: (value: ItemDialogTab) => void
|
||||
@@ -79,9 +81,6 @@ export function PullRequestPageTabs({
|
||||
itemKey: { id: string; repoId: string },
|
||||
reviewRequests: GitHubAssignableUser[]
|
||||
) => void
|
||||
patchCachedPRChecks: (cacheKey: string, checks: PRCheckDetail[]) => void
|
||||
patchCachedWorkItemBody: (cacheKey: string, body: string) => void
|
||||
patchCachedPRReviewRequests: (cacheKey: string, reviewRequests: GitHubAssignableUser[]) => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Tabs
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { GitHubWorkItem } from '../../../../../shared/github/work-item-types'
|
||||
import { getStateTone } from './state-badge'
|
||||
import { getSolidStateTone, getStateTone } from './state-badge'
|
||||
|
||||
function item(type: GitHubWorkItem['type'], state: GitHubWorkItem['state']): GitHubWorkItem {
|
||||
return { type, state } as GitHubWorkItem
|
||||
@@ -19,3 +19,17 @@ describe('getStateTone', () => {
|
||||
expect(getStateTone(item('issue', 'open'))).toContain('emerald')
|
||||
})
|
||||
})
|
||||
|
||||
describe('getSolidStateTone', () => {
|
||||
it('matches the outlined tone hues for every pull request state', () => {
|
||||
expect(getSolidStateTone(item('pr', 'merged'))).toContain('purple')
|
||||
expect(getSolidStateTone(item('pr', 'draft'))).toContain('slate')
|
||||
expect(getSolidStateTone(item('pr', 'closed'))).toContain('rose')
|
||||
expect(getSolidStateTone(item('pr', 'open'))).toContain('emerald')
|
||||
})
|
||||
|
||||
it('uses rose for a closed issue and emerald otherwise', () => {
|
||||
expect(getSolidStateTone(item('issue', 'closed'))).toContain('rose')
|
||||
expect(getSolidStateTone(item('issue', 'open'))).toContain('emerald')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -22,6 +22,19 @@ export function getStateTone(item: GitHubWorkItem): string {
|
||||
return 'border-emerald-500/30 bg-emerald-500/10 text-emerald-600 dark:text-emerald-300'
|
||||
}
|
||||
|
||||
/** Filled counterpart of `getStateTone` for high-emphasis surfaces like the PR page header. */
|
||||
export function getSolidStateTone(item: GitHubWorkItem): string {
|
||||
if (item.type === 'pr') {
|
||||
if (item.state === 'merged') {
|
||||
return 'bg-purple-600 text-white'
|
||||
}
|
||||
if (item.state === 'draft') {
|
||||
return 'bg-slate-500 text-white'
|
||||
}
|
||||
}
|
||||
return item.state === 'closed' ? 'bg-rose-600 text-white' : 'bg-emerald-600 text-white'
|
||||
}
|
||||
|
||||
export function WorkItemStateBadge({
|
||||
item,
|
||||
className
|
||||
|
||||
@@ -5,6 +5,7 @@ import { translate } from '@/i18n/i18n'
|
||||
import type { GitHubAssignableUser } from '../../../../../shared/github/pull-request-types'
|
||||
|
||||
export function ReviewerPickerRow({
|
||||
id,
|
||||
reviewer,
|
||||
suggested,
|
||||
active,
|
||||
@@ -14,6 +15,7 @@ export function ReviewerPickerRow({
|
||||
onHover,
|
||||
onRequest
|
||||
}: {
|
||||
id: string
|
||||
reviewer: GitHubAssignableUser
|
||||
suggested: boolean
|
||||
active: boolean
|
||||
@@ -25,6 +27,7 @@ export function ReviewerPickerRow({
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<button
|
||||
id={id}
|
||||
type="button"
|
||||
disabled={disabled}
|
||||
aria-label={
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import React from 'react'
|
||||
import React, { useId } from 'react'
|
||||
import { LoaderCircle, Pencil } from 'lucide-react'
|
||||
import { Input } from '@/components/ui/input'
|
||||
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
|
||||
@@ -47,6 +47,9 @@ export function ReviewerPicker({
|
||||
onEnter: () => void
|
||||
onRequestReviewer: (reviewer: GitHubAssignableUser) => void
|
||||
}): React.JSX.Element {
|
||||
const reviewerListId = useId()
|
||||
const reviewerRowId = (index: number): string => `${reviewerListId}-${index}`
|
||||
const activeReviewerRow = actionableReviewerRows[activeReviewerIndex] ?? null
|
||||
return (
|
||||
<Popover open={open} onOpenChange={onOpenChange}>
|
||||
<PopoverTrigger asChild>
|
||||
@@ -83,8 +86,13 @@ export function ReviewerPicker({
|
||||
'Type or choose a user'
|
||||
)}
|
||||
aria-label={translate('auto.components.PullRequestPage.a04c137bb7', 'Reviewer')}
|
||||
role="combobox"
|
||||
aria-expanded={open}
|
||||
aria-haspopup="listbox"
|
||||
aria-controls={reviewerListId}
|
||||
aria-activedescendant={
|
||||
activeReviewerRow ? reviewerRowId(activeReviewerIndex) : undefined
|
||||
}
|
||||
className="h-8 min-w-0 cursor-text rounded-md border-border/50 bg-background text-xs"
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === 'ArrowDown' && actionableReviewerRows.length > 0) {
|
||||
@@ -112,7 +120,7 @@ export function ReviewerPicker({
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
<div className="min-h-0 flex-1 overflow-y-auto scrollbar-sleek">
|
||||
<div id={reviewerListId} className="min-h-0 flex-1 overflow-y-auto scrollbar-sleek">
|
||||
{reviewerMetadataLoading ? (
|
||||
<div className="px-3 py-2 text-[13px] text-muted-foreground">
|
||||
{translate('auto.components.PullRequestPage.57750f4a8c', 'Loading...')}
|
||||
@@ -127,6 +135,7 @@ export function ReviewerPicker({
|
||||
{suggestedReviewerRows.map((reviewer, index) => (
|
||||
<ReviewerPickerRow
|
||||
key={`suggested:${reviewer.login}`}
|
||||
id={reviewerRowId(index)}
|
||||
reviewer={reviewer}
|
||||
suggested
|
||||
active={actionableReviewerRows[activeReviewerIndex]?.login === reviewer.login}
|
||||
@@ -146,6 +155,7 @@ export function ReviewerPicker({
|
||||
everyoneElseReviewerRows.map((reviewer, index) => (
|
||||
<ReviewerPickerRow
|
||||
key={`reviewer:${reviewer.login}`}
|
||||
id={reviewerRowId(suggestedReviewerRows.length + index)}
|
||||
reviewer={reviewer}
|
||||
suggested={false}
|
||||
active={actionableReviewerRows[activeReviewerIndex]?.login === reviewer.login}
|
||||
|
||||
@@ -16,6 +16,8 @@ import type { GitHubWorkItem } from '../../../../../shared/github/work-item-type
|
||||
import type { TaskSourceContext } from '../../../../../shared/task-source-context'
|
||||
import type { GlobalSettings } from '../../../../../shared/global-settings-types'
|
||||
|
||||
const MAX_REQUESTED_REVIEWERS = 15
|
||||
|
||||
type ReviewerRequestActionsArgs = {
|
||||
// Why: a ref, not the render-time `submitting` value, so two clicks in one tick can't both pass the guard.
|
||||
submittingRef: { current: boolean }
|
||||
@@ -59,7 +61,7 @@ export function createReviewerRequestActions(args: ReviewerRequestActionsArgs):
|
||||
toast.error(translate('auto.components.PullRequestPage.dace0d1a9f', 'Enter a reviewer'))
|
||||
return
|
||||
}
|
||||
if (args.localReviewRequests.length + logins.length > 15) {
|
||||
if (args.localReviewRequests.length + logins.length > MAX_REQUESTED_REVIEWERS) {
|
||||
toast.error(
|
||||
translate(
|
||||
'auto.components.PullRequestPage.8f369a6b6b',
|
||||
@@ -141,7 +143,8 @@ export function createReviewerRequestActions(args: ReviewerRequestActionsArgs):
|
||||
? translate('auto.components.PullRequestPage.03282ff3b9', 'Reviewer requested')
|
||||
: translate('auto.components.PullRequestPage.102d3d177f', 'Reviewers requested')
|
||||
)
|
||||
} catch {
|
||||
} catch (err) {
|
||||
console.error('Failed to request pull request reviewer', err)
|
||||
if (args.reviewerPanelMountedRef.current) {
|
||||
toast.error(
|
||||
translate('auto.components.PullRequestPage.2560588245', 'Failed to request reviewer')
|
||||
@@ -240,7 +243,8 @@ export function createReviewerRequestActions(args: ReviewerRequestActionsArgs):
|
||||
? translate('auto.components.PullRequestPage.2c1d93da43', 'Reviewer removed')
|
||||
: translate('auto.components.PullRequestPage.1e6d089420', 'Reviewers removed')
|
||||
)
|
||||
} catch {
|
||||
} catch (err) {
|
||||
console.error('Failed to remove pull request reviewer', err)
|
||||
if (args.reviewerPanelMountedRef.current) {
|
||||
toast.error(
|
||||
translate('auto.components.PullRequestPage.c798fa0ec7', 'Failed to remove reviewer')
|
||||
|
||||
Reference in New Issue
Block a user