From 3eab34253c89a722570081d70b6bb075c77c460a Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 9 Aug 2026 12:11:33 -0700 Subject: [PATCH] Polish Artifacts management UI (#13356) * refactor(artifacts): polish artifact management UI * fix(artifacts): address UI polish review --------- Co-authored-by: Jinwoo-H --- .../runtime-home-service.test.ts | 1 + src/main/codex-accounts/service.test.ts | 1 + .../components/artifacts/ArtifactActions.tsx | 17 +- .../artifacts/ArtifactCollection.test.tsx | 104 +++++-- .../artifacts/ArtifactCollection.tsx | 101 ++----- .../artifacts/ArtifactDetailHeader.tsx | 55 ++++ .../components/artifacts/ArtifactListPane.tsx | 209 +++++++++++++ .../components/artifacts/ArtifactPreview.tsx | 11 +- .../artifacts/ArtifactsPage.test.tsx | 71 ++++- .../components/artifacts/ArtifactsPage.tsx | 277 ++++++++++++------ .../artifacts/artifact-display-labels.ts | 52 ++++ .../artifacts/artifact-link-actions.ts | 15 + .../components/confirmation-dialog-context.ts | 2 + .../components/confirmation-dialog.test.tsx | 104 +++++++ .../src/components/confirmation-dialog.tsx | 52 +++- .../confirmation-skip-preference.test.ts | 69 +++++ .../confirmation-skip-preference.ts | 54 ++++ .../GeneralWorkspaceSettingsSection.tsx | 31 ++ src/renderer/src/i18n/locales/en.json | 41 ++- src/shared/constants.ts | 1 + src/shared/types.ts | 2 + 21 files changed, 1036 insertions(+), 234 deletions(-) create mode 100644 src/renderer/src/components/artifacts/ArtifactDetailHeader.tsx create mode 100644 src/renderer/src/components/artifacts/ArtifactListPane.tsx create mode 100644 src/renderer/src/components/artifacts/artifact-display-labels.ts create mode 100644 src/renderer/src/components/artifacts/artifact-link-actions.ts create mode 100644 src/renderer/src/components/confirmation-dialog.test.tsx create mode 100644 src/renderer/src/components/confirmation-skip-preference.test.ts create mode 100644 src/renderer/src/components/confirmation-skip-preference.ts diff --git a/src/main/codex-accounts/runtime-home-service.test.ts b/src/main/codex-accounts/runtime-home-service.test.ts index e6688f390..22628ccce 100644 --- a/src/main/codex-accounts/runtime-home-service.test.ts +++ b/src/main/codex-accounts/runtime-home-service.test.ts @@ -140,6 +140,7 @@ function createSettings(overrides: TestSettingsOverrides = {}): GlobalSettings { skipDeleteWorktreeConfirm: false, skipCloseTerminalWithRunningProcessConfirm: false, skipDeleteAutomationConfirm: false, + skipDeleteArtifactConfirm: false, skipCodexRateLimitResetConfirm: false, defaultTaskViewPreset: 'all', defaultTaskSource: 'github', diff --git a/src/main/codex-accounts/service.test.ts b/src/main/codex-accounts/service.test.ts index 7c55304c5..91737c007 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -132,6 +132,7 @@ function createSettings(overrides: Partial = {}): GlobalSettings skipDeleteWorktreeConfirm: false, skipCloseTerminalWithRunningProcessConfirm: false, skipDeleteAutomationConfirm: false, + skipDeleteArtifactConfirm: false, skipCodexRateLimitResetConfirm: false, defaultTaskViewPreset: 'all', defaultTaskSource: 'github', diff --git a/src/renderer/src/components/artifacts/ArtifactActions.tsx b/src/renderer/src/components/artifacts/ArtifactActions.tsx index 8171dc6b9..8caea7f0b 100644 --- a/src/renderer/src/components/artifacts/ArtifactActions.tsx +++ b/src/renderer/src/components/artifacts/ArtifactActions.tsx @@ -1,9 +1,9 @@ import { Copy, ExternalLink, Loader2, Trash2 } from 'lucide-react' -import { toast } from 'sonner' import type { ArtifactListItem } from '../../../../shared/artifacts' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { translate } from '@/i18n/i18n' +import { copyArtifactLink, openArtifactInBrowser } from './artifact-link-actions' type ArtifactActionsProps = { deleting: boolean @@ -16,21 +16,12 @@ export function ArtifactActions({ item, onDelete }: ArtifactActionsProps): React.JSX.Element { - const copyLink = async (): Promise => { - try { - await window.api.ui.writeClipboardText(item.shareUrl) - toast.success(translate('auto.components.artifacts.copySuccess', 'Artifact link copied')) - } catch { - toast.error(translate('auto.components.artifacts.copyFailed', 'Could not copy artifact link')) - } - } - return (
- @@ -40,7 +31,7 @@ export function ArtifactActions({ variant="ghost" size="icon-sm" className="text-muted-foreground hover:text-foreground" - onClick={() => void window.api.shell.openUrl(item.shareUrl)} + onClick={() => openArtifactInBrowser(item.shareUrl)} aria-label={translate('auto.components.artifacts.openInBrowser', 'Open in browser')} > diff --git a/src/renderer/src/components/artifacts/ArtifactCollection.test.tsx b/src/renderer/src/components/artifacts/ArtifactCollection.test.tsx index d51bedaa1..3a5b16e7d 100644 --- a/src/renderer/src/components/artifacts/ArtifactCollection.test.tsx +++ b/src/renderer/src/components/artifacts/ArtifactCollection.test.tsx @@ -14,9 +14,14 @@ vi.mock('./ArtifactActions', () => ({ ArtifactActions: () =>
Artifact actions
})) +import { TooltipProvider } from '@/components/ui/tooltip' import { ArtifactCollection } from './ArtifactCollection' +const DAY_MS = 24 * 60 * 60 * 1000 + +// Why: relative to now — the labels under test are relative times, so fixed dates would rot. function artifact(slug: string, title: string): ArtifactListItem { + const createdAt = new Date(Date.now() - DAY_MS).toISOString() return { artifact: { version: 1, @@ -25,9 +30,9 @@ function artifact(slug: string, title: string): ArtifactListItem { originalFileName: `${slug}.html`, sourceContentType: 'text/html', renderedContentType: 'text/html', - createdAt: '2026-08-07T12:00:00.000Z', - updatedAt: '2026-08-07T12:00:00.000Z', - expiresAt: '2026-09-07T12:00:00.000Z', + createdAt, + updatedAt: createdAt, + expiresAt: new Date(Date.now() + 30 * DAY_MS).toISOString(), byteSize: 1200, deletedAt: null }, @@ -38,29 +43,90 @@ function artifact(slug: string, title: string): ArtifactListItem { describe('ArtifactCollection', () => { afterEach(cleanup) + function renderCollection( + items: ArtifactListItem[], + selectArtifact = vi.fn() + ): { container: HTMLElement; selectArtifact: ReturnType } { + const { container } = render( + + + + ) + return { container, selectArtifact } + } + it('keeps the artifact list beside a contained preview', async () => { const items = [artifact('first', 'First artifact'), artifact('second', 'Second artifact')] - const selectArtifact = vi.fn() - const { container } = render( - - ) + const { container, selectArtifact } = renderCollection(items) const collection = container.firstElementChild - expect(collection).toHaveClass('grid-cols-[16rem_minmax(0,1fr)]') - expect(collection?.children[0]?.tagName).toBe('ASIDE') + // Why: full-bleed split — no card frame around the panes. + expect(collection).toHaveClass('lg:grid-cols-[minmax(240px,300px)_minmax(0,1fr)]') + expect(collection).not.toHaveClass('rounded-md') expect(collection?.children[1]?.tagName).toBe('SECTION') expect(screen.getByText('Preview https://share.onorca.dev/a/first')).toBeInTheDocument() - await userEvent.click(screen.getByRole('button', { name: /Second artifact/ })) + await userEvent.click(screen.getByRole('option', { name: /Second artifact/ })) expect(selectArtifact).toHaveBeenCalledWith('second') }) + + it('exposes the list as a single-tab-stop listbox', () => { + const items = [artifact('first', 'First artifact'), artifact('second', 'Second artifact')] + renderCollection(items) + + expect(screen.getByRole('listbox', { name: 'Shared artifacts' })).toBeInTheDocument() + const [first, second] = screen.getAllByRole('option') + expect(first).toHaveAttribute('aria-selected', 'true') + expect(first).toHaveAttribute('aria-current', 'page') + expect(first).toHaveAttribute('tabindex', '0') + expect(second).toHaveAttribute('aria-selected', 'false') + expect(second).toHaveAttribute('tabindex', '-1') + }) + + it('moves focus with arrows and commits selection on Enter', async () => { + const items = [artifact('first', 'First artifact'), artifact('second', 'Second artifact')] + const { selectArtifact } = renderCollection(items) + const [first, second] = screen.getAllByRole('option') + + first.focus() + await userEvent.keyboard('{ArrowDown}') + expect(second).toHaveFocus() + // Why: arrows must not commit — each selection reloads the preview webview. + expect(selectArtifact).not.toHaveBeenCalled() + + await userEvent.keyboard('{Enter}') + expect(selectArtifact).toHaveBeenCalledWith('second') + }) + + it('filters the list by name and keeps the preview mounted', async () => { + const items = [artifact('first', 'First artifact'), artifact('second', 'Second artifact')] + renderCollection(items) + + await userEvent.type(screen.getByPlaceholderText('Search artifacts'), 'second') + expect(screen.getAllByRole('option')).toHaveLength(1) + expect(screen.getByRole('option', { name: /Second artifact/ })).toBeInTheDocument() + expect(screen.getByText('Preview https://share.onorca.dev/a/first')).toBeInTheDocument() + + await userEvent.clear(screen.getByPlaceholderText('Search artifacts')) + await userEvent.type(screen.getByPlaceholderText('Search artifacts'), 'nothing') + expect(screen.queryAllByRole('option')).toHaveLength(0) + expect(screen.getByText('No matches')).toBeInTheDocument() + }) + + it('shows the share url and expiry instead of repeating the row metadata', () => { + const items = [artifact('first', 'First artifact')] + renderCollection(items) + + expect(screen.getByText('https://share.onorca.dev/a/first')).toBeInTheDocument() + expect(screen.getByText(/Link expires/)).toBeInTheDocument() + }) }) diff --git a/src/renderer/src/components/artifacts/ArtifactCollection.tsx b/src/renderer/src/components/artifacts/ArtifactCollection.tsx index ca6a270a6..a601a964f 100644 --- a/src/renderer/src/components/artifacts/ArtifactCollection.tsx +++ b/src/renderer/src/components/artifacts/ArtifactCollection.tsx @@ -1,31 +1,8 @@ -import { Files, Loader2 } from 'lucide-react' import type { ArtifactListItem } from '../../../../shared/artifacts' -import { Button } from '@/components/ui/button' -import { translate } from '@/i18n/i18n' -import { cn } from '@/lib/utils' -import { ArtifactActions } from './ArtifactActions' +import { ArtifactDetailHeader } from './ArtifactDetailHeader' +import { ArtifactListPane } from './ArtifactListPane' import { ArtifactPreview } from './ArtifactPreview' -function formatArtifactDate(value: string): string { - return new Intl.DateTimeFormat(undefined, { dateStyle: 'medium', timeStyle: 'short' }).format( - new Date(value) - ) -} - -function formatByteSize(value: number): string { - if (value < 1024) { - return `${value} B` - } - if (value < 1024 * 1024) { - return `${(value / 1024).toFixed(1)} KB` - } - return `${(value / (1024 * 1024)).toFixed(1)} MB` -} - -function artifactName(item: ArtifactListItem): string { - return item.artifact.title || item.artifact.originalFileName || item.artifact.slug -} - export function ArtifactCollection({ artifacts, deletingId, @@ -46,63 +23,25 @@ export function ArtifactCollection({ loadMore: () => void }): React.JSX.Element { return ( -
- + // Why: match Automations while stacking the list on narrow layouts. +
+
-
-
-

{artifactName(selectedArtifact)}

-

- {formatArtifactDate(selectedArtifact.artifact.updatedAt)} ·{' '} - {formatByteSize(selectedArtifact.artifact.byteSize)} -

-
- -
+
diff --git a/src/renderer/src/components/artifacts/ArtifactDetailHeader.tsx b/src/renderer/src/components/artifacts/ArtifactDetailHeader.tsx new file mode 100644 index 000000000..806f2d98b --- /dev/null +++ b/src/renderer/src/components/artifacts/ArtifactDetailHeader.tsx @@ -0,0 +1,55 @@ +import { Globe } from 'lucide-react' +import type { ArtifactListItem } from '../../../../shared/artifacts' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { translate } from '@/i18n/i18n' +import { ArtifactActions } from './ArtifactActions' +import { + artifactName, + formatArtifactExpiry, + formatArtifactUpdatedAt, + formatByteSize +} from './artifact-display-labels' + +export function ArtifactDetailHeader({ + deleting, + item, + onDelete +}: { + deleting: boolean + item: ArtifactListItem + onDelete: (target: ArtifactListItem) => void +}): React.JSX.Element { + return ( +
+ {/* Why: a floor rather than min-w-0 — otherwise the title truncates to nothing before the actions wrap. */} +
+

{artifactName(item)}

+
+ + + + + + {translate( + 'auto.components.artifacts.ArtifactDetailHeader.publicLink', + 'Anyone with this link can view it' + )} + + + + {translate( + 'auto.components.artifacts.ArtifactDetailHeader.publicLink', + 'Anyone with this link can view it' + )} + +

{item.shareUrl}

+
+

+ {formatArtifactUpdatedAt(item.artifact.updatedAt)} ·{' '} + {formatByteSize(item.artifact.byteSize)} · {formatArtifactExpiry(item.artifact.expiresAt)} +

+
+ +
+ ) +} diff --git a/src/renderer/src/components/artifacts/ArtifactListPane.tsx b/src/renderer/src/components/artifacts/ArtifactListPane.tsx new file mode 100644 index 000000000..debfcfd07 --- /dev/null +++ b/src/renderer/src/components/artifacts/ArtifactListPane.tsx @@ -0,0 +1,209 @@ +import { useMemo, useRef, useState } from 'react' +import { Copy, ExternalLink, Loader2, Search, Trash2 } from 'lucide-react' +import type { ArtifactListItem } from '../../../../shared/artifacts' +import { Button } from '@/components/ui/button' +import { + ContextMenu, + ContextMenuContent, + ContextMenuItem, + ContextMenuSeparator, + ContextMenuTrigger +} from '@/components/ui/context-menu' +import { Input } from '@/components/ui/input' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { translate } from '@/i18n/i18n' +import { cn } from '@/lib/utils' +import { + artifactName, + artifactTypeIcon, + formatArtifactDate, + formatArtifactExpiry, + formatArtifactUpdatedAt, + formatByteSize +} from './artifact-display-labels' +import { copyArtifactLink, openArtifactInBrowser } from './artifact-link-actions' + +const OPTION_SELECTOR = '[role="option"]' + +function moveOptionFocus(listbox: HTMLElement | null, from: HTMLElement, step: number): void { + const options = [...(listbox?.querySelectorAll(OPTION_SELECTOR) ?? [])] + const next = options[options.indexOf(from) + step] + next?.focus() +} + +function focusEdgeOption(listbox: HTMLElement | null, edge: 'first' | 'last'): void { + const options = [...(listbox?.querySelectorAll(OPTION_SELECTOR) ?? [])] + const target = edge === 'first' ? options.at(0) : options.at(-1) + target?.focus() +} + +export function ArtifactListPane({ + artifacts, + className, + deletingId, + selectedArtifact, + selectArtifact, + deleteArtifact, + hasMore, + loadingMore, + loadMore +}: { + artifacts: readonly ArtifactListItem[] + className?: string + deletingId: string | null + selectedArtifact: ArtifactListItem + selectArtifact: (slug: string) => void + deleteArtifact: (item: ArtifactListItem) => void + hasMore: boolean + loadingMore: boolean + loadMore: () => void +}): React.JSX.Element { + const listboxRef = useRef(null) + const [query, setQuery] = useState('') + const normalizedQuery = query.trim().toLowerCase() + const matches = useMemo( + () => + normalizedQuery + ? artifacts.filter((item) => artifactName(item).toLowerCase().includes(normalizedQuery)) + : artifacts, + [artifacts, normalizedQuery] + ) + + // Why: arrows move focus only — committing selection would reload the preview webview on every keypress. + const onOptionKeyDown = (event: React.KeyboardEvent, slug: string): void => { + const option = event.currentTarget + if (event.key === 'ArrowDown') { + event.preventDefault() + moveOptionFocus(listboxRef.current, option, 1) + } else if (event.key === 'ArrowUp') { + event.preventDefault() + moveOptionFocus(listboxRef.current, option, -1) + } else if (event.key === 'Home') { + event.preventDefault() + focusEdgeOption(listboxRef.current, 'first') + } else if (event.key === 'End') { + event.preventDefault() + focusEdgeOption(listboxRef.current, 'last') + } else if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault() + selectArtifact(slug) + } + } + + return ( +
+
+ + setQuery(event.target.value)} + placeholder={translate( + 'auto.components.artifacts.ArtifactListPane.search', + 'Search artifacts' + )} + className="h-8 pl-8 text-sm" + /> +
+
+
+ {matches.map((item) => { + const selected = item.artifact.slug === selectedArtifact.artifact.slug + const name = artifactName(item) + const TypeIcon = artifactTypeIcon(item) + return ( + + +
selectArtifact(item.artifact.slug)} + onKeyDown={(event) => onOptionKeyDown(event, item.artifact.slug)} + className={cn( + 'flex w-full cursor-pointer items-center gap-3 border-b border-border/50 px-3 py-3 text-left transition-colors last:border-b-0 hover:bg-accent/50 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring', + selected && 'bg-accent' + )} + > + + + + + {name} + + +

{name}

+

+ {formatArtifactDate(item.artifact.updatedAt)} +

+

+ {formatArtifactExpiry(item.artifact.expiresAt)} +

+ + + + {formatArtifactUpdatedAt(item.artifact.updatedAt)} ·{' '} + {formatByteSize(item.artifact.byteSize)} + + +
+ + + void copyArtifactLink(item.shareUrl)}> + + {translate('auto.components.artifacts.copyLink', 'Copy link')} + + openArtifactInBrowser(item.shareUrl)}> + + {translate('auto.components.artifacts.openInBrowser', 'Open in browser')} + + + deleteArtifact(item)} + > + + {translate( + 'auto.components.artifacts.ArtifactsPage.deleteArtifact', + 'Delete artifact' + )} + + + + ) + })} +
+ {matches.length === 0 ? ( +

+ {translate('auto.components.artifacts.ArtifactListPane.noMatches', 'No matches')} +

+ ) : null} + {hasMore ? ( +
+ +
+ ) : null} +
+
+ ) +} diff --git a/src/renderer/src/components/artifacts/ArtifactPreview.tsx b/src/renderer/src/components/artifacts/ArtifactPreview.tsx index d081a3734..69b8d26ab 100644 --- a/src/renderer/src/components/artifacts/ArtifactPreview.tsx +++ b/src/renderer/src/components/artifacts/ArtifactPreview.tsx @@ -44,7 +44,7 @@ function attachArtifactWebview({ webview.style.width = '100%' webview.style.height = '100%' webview.style.border = 'none' - webview.style.background = '#ffffff' + // Why: forcing white flashes beneath dark artifact pages during navigation. webview.addEventListener('did-start-loading', onLoadStarted) webview.addEventListener('did-stop-loading', onLoadStopped) webview.addEventListener('did-fail-load', onLoadFailed) @@ -138,14 +138,17 @@ export function ArtifactPreview({ shareUrl }: { shareUrl: string }): React.JSX.E }, [shareUrl]) return ( -
+
{state === 'loading' ? ( -
+
) : null} {state === 'unavailable' ? ( -
+

{translate('auto.components.artifacts.previewUnavailable', 'Preview unavailable')} diff --git a/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx b/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx index 32309c922..37d2462f9 100644 --- a/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx +++ b/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx @@ -18,6 +18,10 @@ const mocks = vi.hoisted(() => ({ confirm: vi.fn(), refreshAuth: vi.fn(), rpc: vi.fn(), + settings: { skipDeleteArtifactConfirm: false } as Record, + updateSettings: vi.fn(), + openSettingsPage: vi.fn(), + openSettingsTarget: vi.fn(), resolvePartition: vi.fn(), writeClipboardText: vi.fn(), openUrl: vi.fn(), @@ -56,7 +60,11 @@ function storeState(): Record { connectCurrentOrcaProfile: mocks.connect, orcaProfileAuthStatus: mocks.authStatus, orcaProfileConnecting: false, - refreshCurrentOrcaProfileAuth: mocks.refreshAuth + refreshCurrentOrcaProfileAuth: mocks.refreshAuth, + settings: mocks.settings, + updateSettings: mocks.updateSettings, + openSettingsPage: mocks.openSettingsPage, + openSettingsTarget: mocks.openSettingsTarget } } @@ -76,6 +84,10 @@ describe('ArtifactsPage', () => { mocks.confirm.mockReset() mocks.refreshAuth.mockReset() mocks.rpc.mockReset() + mocks.settings = { skipDeleteArtifactConfirm: false } + mocks.updateSettings.mockReset().mockResolvedValue(undefined) + mocks.openSettingsPage.mockReset() + mocks.openSettingsTarget.mockReset() mocks.resolvePartition.mockReset().mockResolvedValue('persist:orca-default') mocks.writeClipboardText.mockReset().mockResolvedValue(undefined) mocks.openUrl.mockReset().mockResolvedValue(undefined) @@ -118,7 +130,8 @@ describe('ArtifactsPage', () => { it('renders the selected artifact in-app with copy link as the primary action', async () => { render() - expect(await screen.findAllByText('Quarterly report')).toHaveLength(2) + expect(await screen.findByRole('option', { name: /Quarterly report/ })).toBeInTheDocument() + expect(screen.getByRole('heading', { level: 2, name: 'Quarterly report' })).toBeInTheDocument() const closeButton = screen.getByRole('button', { name: 'Close artifacts' }) expect(closeButton).toHaveClass('size-7', 'rounded-full') expect(closeButton.closest('header')).toHaveClass('px-5', 'pb-3', 'pt-1.5', 'md:px-8') @@ -218,8 +231,8 @@ describe('ArtifactsPage', () => { value: { artifacts: [artifactListItem('Second page', 'second-page')] } }) - expect(await screen.findByText('Second page')).toBeInTheDocument() - expect(screen.getAllByText('First page')).toHaveLength(2) + expect(await screen.findByRole('option', { name: /Second page/ })).toBeInTheDocument() + expect(screen.getByRole('option', { name: /First page/ })).toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Load more' })).not.toBeInTheDocument() }) @@ -239,7 +252,7 @@ describe('ArtifactsPage', () => { expect(screen.queryByText('No shared artifacts')).not.toBeInTheDocument() fireEvent.click(screen.getByRole('button', { name: 'Load more' })) - expect(await screen.findAllByText('Older artifact')).toHaveLength(2) + expect(await screen.findByRole('option', { name: /Older artifact/ })).toBeInTheDocument() }) it('keeps loaded artifacts when loading another page fails', async () => { @@ -254,11 +267,11 @@ describe('ArtifactsPage', () => { .mockRejectedValueOnce(new Error('network down')) render() - await screen.findAllByText('Still visible') + await screen.findByRole('option', { name: /Still visible/ }) fireEvent.click(screen.getByRole('button', { name: 'Load more' })) expect(await screen.findByText('Could not load more artifacts.')).toBeInTheDocument() - expect(screen.getAllByText('Still visible')).toHaveLength(2) + expect(screen.getByRole('option', { name: /Still visible/ })).toBeInTheDocument() expect(screen.getByRole('button', { name: 'Load more' })).toBeEnabled() }) @@ -283,7 +296,7 @@ describe('ArtifactsPage', () => { state: 'connected' } view.rerender() - expect(await screen.findAllByText('Account B')).toHaveLength(2) + expect(await screen.findByRole('option', { name: /Account B/ })).toBeInTheDocument() resolveRefresh() await waitFor(() => @@ -323,7 +336,7 @@ describe('ArtifactsPage', () => { state: 'connected' } view.rerender() - expect(await screen.findAllByText('Account B')).toHaveLength(2) + expect(await screen.findByRole('option', { name: /Account B/ })).toBeInTheDocument() resolveRefresh() await waitFor(() => @@ -407,7 +420,7 @@ describe('ArtifactsPage', () => { view.rerender() resolveDelete({ status: 'ok', value: undefined }) - expect(await screen.findAllByText('Shared slug B')).toHaveLength(2) + expect(await screen.findByRole('option', { name: /Shared slug B/ })).toBeInTheDocument() }) it('does not resurrect a deletion from an older refresh', async () => { @@ -438,6 +451,44 @@ describe('ArtifactsPage', () => { await waitFor(() => expect(screen.queryByText('Delete me')).not.toBeInTheDocument()) }) + it('skips the delete confirmation once the preference is saved', async () => { + mocks.settings = { skipDeleteArtifactConfirm: true } + mocks.rpc.mockResolvedValue({ + status: 'ok', + value: { artifacts: [artifactListItem('Skip me', 'skip-me')] } + }) + render() + await screen.findByRole('option', { name: /Skip me/ }) + + mocks.rpc.mockResolvedValueOnce({ status: 'ok', value: undefined }) + fireEvent.click(screen.getByRole('button', { name: 'Delete artifact' })) + + await waitFor(() => expect(screen.queryByRole('option', { name: /Skip me/ })).toBeNull()) + expect(mocks.confirm).not.toHaveBeenCalled() + }) + + it('persists the skip preference only when the confirmation is accepted', async () => { + mocks.confirm.mockResolvedValue(true) + mocks.rpc.mockResolvedValue({ + status: 'ok', + value: { artifacts: [artifactListItem('Ask me', 'ask-me')] } + }) + render() + await screen.findByRole('option', { name: /Ask me/ }) + + mocks.rpc.mockResolvedValueOnce({ status: 'ok', value: undefined }) + fireEvent.click(screen.getByRole('button', { name: 'Delete artifact' })) + await waitFor(() => expect(mocks.confirm).toHaveBeenCalledOnce()) + + // Why: the dialog owns the checkbox; the page only supplies what to persist when it is checked. + const options = mocks.confirm.mock.calls[0]?.[0] as { + dontAskAgain?: { onConfirmed: () => void } + } + expect(mocks.updateSettings).not.toHaveBeenCalled() + options.dontAskAgain?.onConfirmed() + expect(mocks.updateSettings).toHaveBeenCalledWith({ skipDeleteArtifactConfirm: true }) + }) + it('treats an organization switch as an account identity change', () => { const status = { activeProfileId: 'profile-a', diff --git a/src/renderer/src/components/artifacts/ArtifactsPage.tsx b/src/renderer/src/components/artifacts/ArtifactsPage.tsx index c88a68afb..7ca866476 100644 --- a/src/renderer/src/components/artifacts/ArtifactsPage.tsx +++ b/src/renderer/src/components/artifacts/ArtifactsPage.tsx @@ -1,9 +1,10 @@ import { useEffect, useState } from 'react' -import { Files, Loader2, RefreshCw, X } from 'lucide-react' +import { ArrowRight, Files, Loader2, RefreshCw, X } from 'lucide-react' import type { ArtifactCloudOperation, ArtifactListItem } from '../../../../shared/artifacts' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { useConfirmationDialog } from '@/components/confirmation-dialog-context' +import { persistConfirmationSkipPreference } from '@/components/confirmation-skip-preference' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { useAppStore } from '@/store' import { translate } from '@/i18n/i18n' @@ -18,10 +19,19 @@ export default function ArtifactsPage(): React.JSX.Element { const connecting = useAppStore((state) => state.orcaProfileConnecting) const connect = useAppStore((state) => state.connectCurrentOrcaProfile) const refreshAuth = useAppStore((state) => state.refreshCurrentOrcaProfileAuth) + const openSettingsPage = useAppStore((state) => state.openSettingsPage) + const openSettingsTarget = useAppStore((state) => state.openSettingsTarget) + const settings = useAppStore((state) => state.settings) + const updateSettings = useAppStore((state) => state.updateSettings) const confirm = useConfirmationDialog() const [deleting, setDeleting] = useState<{ identity: string; slug: string } | null>(null) const [selectedSlug, setSelectedSlug] = useState(null) const signedIn = authStatus?.state === 'connected' + const needsReconnect = authStatus?.state === 'reconnect-required' + const openAccountSettings = (): void => { + openSettingsTarget({ pane: 'orca-account', repoId: null }) + openSettingsPage() + } const { accountIdentity, artifacts, @@ -52,6 +62,18 @@ export default function ArtifactsPage(): React.JSX.Element { if (event.key !== 'Escape' || event.defaultPrevented) { return } + // Why: Esc clears field focus before closing the page, matching Automations. + const target = event.target + if ( + target instanceof HTMLInputElement || + target instanceof HTMLTextAreaElement || + target instanceof HTMLSelectElement || + (target instanceof HTMLElement && target.isContentEditable) + ) { + event.preventDefault() + target.blur() + return + } event.preventDefault() closePage() } @@ -67,17 +89,32 @@ export default function ArtifactsPage(): React.JSX.Element { const requestedAccountIsCurrent = (): boolean => artifactAccountIdentity(useAppStore.getState().orcaProfileAuthStatus) === requestedIdentity const name = item.artifact.title || item.artifact.originalFileName || item.artifact.slug - const accepted = await confirm({ - title: translate('auto.components.artifacts.ArtifactsPage.deleteTitle', 'Delete artifact?'), - description: translate( - 'auto.components.artifacts.ArtifactsPage.deleteDescription', - '“{{name}}” will no longer be available at its public link.', - { name } - ), - confirmLabel: translate('auto.components.artifacts.ArtifactsPage.delete', 'Delete'), - confirmVariant: 'destructive' - }) - if (!accepted || !requestedAccountIsCurrent()) { + if (!settings?.skipDeleteArtifactConfirm) { + const accepted = await confirm({ + title: translate('auto.components.artifacts.ArtifactsPage.deleteTitle', 'Delete artifact?'), + description: translate( + 'auto.components.artifacts.ArtifactsPage.deleteDescription', + '“{{name}}” will no longer be available at its public link.', + { name } + ), + confirmLabel: translate('auto.components.artifacts.ArtifactsPage.delete', 'Delete'), + confirmVariant: 'destructive', + dontAskAgain: { + onConfirmed: () => + persistConfirmationSkipPreference({ + updates: { skipDeleteArtifactConfirm: true }, + settingsSectionId: 'general-skip-delete-artifact-confirm', + updateSettings, + openSettingsPage, + openSettingsTarget + }) + } + }) + if (!accepted) { + return + } + } + if (!requestedAccountIsCurrent()) { return } setDeleting({ identity: requestedIdentity, slug: item.artifact.slug }) @@ -139,9 +176,26 @@ export default function ArtifactsPage(): React.JSX.Element {

-

- {translate('auto.components.artifacts.ArtifactsPage.title', 'Artifacts')} -

+
+

+ {translate('auto.components.artifacts.ArtifactsPage.title', 'Artifacts')} +

+ {signedIn && artifacts.length > 0 ? ( +

+ {nextCursor + ? translate( + 'auto.components.artifacts.ArtifactsPage.loadedCountMore', + '{{count}} loaded · more available', + { count: artifacts.length } + ) + : translate( + 'auto.components.artifacts.ArtifactsPage.loadedCount', + '{{count}} shared', + { count: artifacts.length } + )} +

+ ) : null} +
{signedIn ? ( @@ -164,94 +218,135 @@ export default function ArtifactsPage(): React.JSX.Element { ) : null} -
-
- {!signedIn ? ( -
- -
-

- {translate( - 'auto.components.artifacts.ArtifactsPage.signInHeading', - 'Sign in to Orca' - )} -

-

- {translate( - 'auto.components.artifacts.ArtifactsPage.signInCopy', - 'Sign in to view and manage artifacts shared through your account.' - )} -

-
- -
- ) : loading && artifacts.length === 0 ? ( -
- -
- ) : artifacts.length === 0 ? ( -
- + {/* Why: pane edges match the full-bleed Automations layout. */} +
+ {error ? ( +
+

{error}

+ +
+ ) : null} + {!signedIn ? ( +
+ +

- {nextCursor + {needsReconnect ? translate( - 'auto.components.artifacts.ArtifactsPage.moreAvailable', - 'More artifacts are available' + 'auto.components.artifacts.ArtifactsPage.reconnectHeading', + 'Sign in to Orca again' ) : translate( - 'auto.components.artifacts.ArtifactsPage.empty', - 'No shared artifacts' + 'auto.components.artifacts.ArtifactsPage.signInHeading', + 'Sign in to share artifacts' )}

-

- {nextCursor +

+ {needsReconnect ? translate( - 'auto.components.artifacts.ArtifactsPage.moreAvailableCopy', - 'Load the next page to continue.' + 'auto.components.artifacts.ArtifactsPage.reconnectCopy', + 'Sign in again to view and manage the artifacts shared through your account.' ) : translate( - 'auto.components.artifacts.ArtifactsPage.emptyCopy', - 'Ask your agent to share an HTML or Markdown file, and it will appear here.' + 'auto.components.artifacts.ArtifactsPage.signInCopy', + 'Use your Orca account to upload artifacts and manage their public links.' )}

- {nextCursor ? ( - - ) : null}
- ) : ( - selectedArtifact && ( - void deleteArtifact(target)} - hasMore={Boolean(nextCursor)} - loadingMore={loadingMore} - loadMore={() => void loadMoreArtifacts()} - /> - ) - )} - {error ?

{error}

: null} -
+ {authStatus?.configured === true ? ( + + ) : ( +
+

+ {translate( + 'auto.components.artifacts.ArtifactsPage.unconfiguredCopy', + 'Orca account sign-in is not configured on this machine yet.' + )} +

+ +
+ )} +
+ ) : loading && artifacts.length === 0 ? ( +
+ +
+ ) : artifacts.length === 0 ? ( +
+ +

+ {nextCursor + ? translate( + 'auto.components.artifacts.ArtifactsPage.moreAvailable', + 'More artifacts are available' + ) + : translate('auto.components.artifacts.ArtifactsPage.empty', 'No shared artifacts')} +

+

+ {nextCursor + ? translate( + 'auto.components.artifacts.ArtifactsPage.moreAvailableCopy', + 'Load the next page to continue.' + ) + : translate( + 'auto.components.artifacts.ArtifactsPage.emptyCopy', + 'Ask your agent to share an HTML or Markdown file, and it will appear here.' + )} +

+ {nextCursor ? ( + + ) : null} +
+ ) : ( + selectedArtifact && ( + void deleteArtifact(target)} + hasMore={Boolean(nextCursor)} + loadingMore={loadingMore} + loadMore={() => void loadMoreArtifacts()} + /> + ) + )}
) diff --git a/src/renderer/src/components/artifacts/artifact-display-labels.ts b/src/renderer/src/components/artifacts/artifact-display-labels.ts new file mode 100644 index 000000000..5bd0e510c --- /dev/null +++ b/src/renderer/src/components/artifacts/artifact-display-labels.ts @@ -0,0 +1,52 @@ +import { FileCode2, FileText, type LucideIcon } from 'lucide-react' +import type { ArtifactListItem } from '../../../../shared/artifacts' +import { getIntlLocale, translate } from '@/i18n/i18n' +import { formatUiRelativeTime, formatUiRelativeTimeFromDate } from '@/i18n/relative-time-format' + +export function artifactName(item: ArtifactListItem): string { + return item.artifact.title || item.artifact.originalFileName || item.artifact.slug +} + +export function formatArtifactDate(value: string): string { + return new Intl.DateTimeFormat(getIntlLocale(), { + dateStyle: 'medium', + timeStyle: 'short' + }).format(new Date(value)) +} + +export function formatByteSize(value: number): string { + if (value < 1024) { + return `${value} B` + } + if (value < 1024 * 1024) { + return `${(value / 1024).toFixed(1)} KB` + } + return `${(value / (1024 * 1024)).toFixed(1)} MB` +} + +export function formatArtifactUpdatedAt(value: string): string { + return translate('auto.components.artifacts.updatedAt', 'Updated {{when}}', { + when: formatUiRelativeTimeFromDate( + value, + translate('auto.components.artifacts.updatedRecently', 'recently') + ) + }) +} + +/** Phrased from the stored timestamp alone — never a claim about server-side state. */ +export function formatArtifactExpiry(value: string): string { + const expiresAt = new Date(value) + if (Number.isNaN(expiresAt.getTime())) { + return translate('auto.components.artifacts.expiryUnknown', 'Expiry unknown') + } + const remainingMs = expiresAt.getTime() - Date.now() + return remainingMs <= 0 + ? translate('auto.components.artifacts.expired', 'Link expired') + : translate('auto.components.artifacts.expires', 'Link expires {{when}}', { + when: formatUiRelativeTime(remainingMs) + }) +} + +export function artifactTypeIcon(item: ArtifactListItem): LucideIcon { + return item.artifact.sourceContentType === 'text/markdown' ? FileText : FileCode2 +} diff --git a/src/renderer/src/components/artifacts/artifact-link-actions.ts b/src/renderer/src/components/artifacts/artifact-link-actions.ts new file mode 100644 index 000000000..31685b2b7 --- /dev/null +++ b/src/renderer/src/components/artifacts/artifact-link-actions.ts @@ -0,0 +1,15 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' + +export async function copyArtifactLink(shareUrl: string): Promise { + try { + await window.api.ui.writeClipboardText(shareUrl) + toast.success(translate('auto.components.artifacts.copySuccess', 'Artifact link copied')) + } catch { + toast.error(translate('auto.components.artifacts.copyFailed', 'Could not copy artifact link')) + } +} + +export function openArtifactInBrowser(shareUrl: string): void { + void window.api.shell.openUrl(shareUrl) +} diff --git a/src/renderer/src/components/confirmation-dialog-context.ts b/src/renderer/src/components/confirmation-dialog-context.ts index 94a0ebc99..8dbaf3649 100644 --- a/src/renderer/src/components/confirmation-dialog-context.ts +++ b/src/renderer/src/components/confirmation-dialog-context.ts @@ -8,6 +8,8 @@ export type ConfirmationDialogOptions = { confirmLabel?: string cancelLabel?: string confirmVariant?: 'default' | 'destructive' + /** Renders a "Don't ask again" checkbox. `onConfirmed` runs only when the user confirms with it checked. */ + dontAskAgain?: { label?: string; onConfirmed: () => void } } export type ConfirmationDialogContextValue = ( diff --git a/src/renderer/src/components/confirmation-dialog.test.tsx b/src/renderer/src/components/confirmation-dialog.test.tsx new file mode 100644 index 000000000..25738ee4c --- /dev/null +++ b/src/renderer/src/components/confirmation-dialog.test.tsx @@ -0,0 +1,104 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' +import { cleanup, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { afterEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: Record) => unknown) => + selector({ setContextualToursBlockingSurfaceVisible: vi.fn() }) +})) + +import { ConfirmationDialogProvider } from './confirmation-dialog' +import { + useConfirmationDialog, + type ConfirmationDialogOptions +} from './confirmation-dialog-context' + +function Harness({ + options, + onSettled +}: { + options: ConfirmationDialogOptions + onSettled: (confirmed: boolean) => void +}): React.JSX.Element { + const confirm = useConfirmationDialog() + return ( + + ) +} + +function renderDialog(options: ConfirmationDialogOptions): { onSettled: ReturnType } { + const onSettled = vi.fn() + render( + + + + ) + return { onSettled } +} + +describe('ConfirmationDialogProvider', () => { + afterEach(cleanup) + + it('omits the checkbox unless the caller opts in', async () => { + renderDialog({ title: 'Delete artifact?' }) + + await userEvent.click(screen.getByRole('button', { name: 'ask' })) + expect(await screen.findByText('Delete artifact?')).toBeInTheDocument() + expect(screen.queryByRole('checkbox')).not.toBeInTheDocument() + }) + + it('runs the skip callback when confirmed with the box checked', async () => { + const onConfirmed = vi.fn() + const { onSettled } = renderDialog({ + title: 'Delete artifact?', + confirmLabel: 'Delete', + dontAskAgain: { onConfirmed } + }) + + await userEvent.click(screen.getByRole('button', { name: 'ask' })) + await userEvent.click(await screen.findByRole('checkbox', { name: "Don't ask again" })) + await userEvent.click(screen.getByRole('button', { name: 'Delete' })) + + expect(onConfirmed).toHaveBeenCalledOnce() + await waitFor(() => expect(onSettled).toHaveBeenCalledWith(true)) + }) + + it('never saves the preference when the user backs out', async () => { + const onConfirmed = vi.fn() + const { onSettled } = renderDialog({ + title: 'Delete artifact?', + dontAskAgain: { onConfirmed } + }) + + await userEvent.click(screen.getByRole('button', { name: 'ask' })) + await userEvent.click(await screen.findByRole('checkbox', { name: "Don't ask again" })) + await userEvent.click(screen.getByRole('button', { name: 'Cancel' })) + + expect(onConfirmed).not.toHaveBeenCalled() + await waitFor(() => expect(onSettled).toHaveBeenCalledWith(false)) + }) + + it('does not carry a checked box into the next prompt', async () => { + const onConfirmed = vi.fn() + renderDialog({ + title: 'Delete artifact?', + confirmLabel: 'Delete', + dontAskAgain: { onConfirmed } + }) + + await userEvent.click(screen.getByRole('button', { name: 'ask' })) + await userEvent.click(await screen.findByRole('checkbox', { name: "Don't ask again" })) + await userEvent.click(screen.getByRole('button', { name: 'Cancel' })) + + await userEvent.click(screen.getByRole('button', { name: 'ask' })) + expect(await screen.findByRole('checkbox', { name: "Don't ask again" })).toHaveAttribute( + 'data-state', + 'unchecked' + ) + }) +}) diff --git a/src/renderer/src/components/confirmation-dialog.tsx b/src/renderer/src/components/confirmation-dialog.tsx index 3011078c3..5cf7ef6e4 100644 --- a/src/renderer/src/components/confirmation-dialog.tsx +++ b/src/renderer/src/components/confirmation-dialog.tsx @@ -1,6 +1,8 @@ import React, { useCallback, useEffect, useRef, useState } from 'react' import { Button } from '@/components/ui/button' +import { Checkbox } from '@/components/ui/checkbox' +import { Label } from '@/components/ui/label' import { Dialog, DialogContent, @@ -30,6 +32,7 @@ export function ConfirmationDialogProvider({ }): React.JSX.Element { const nextIdRef = useRef(0) const [queue, setQueue] = useState([]) + const [dontAskAgain, setDontAskAgain] = useState(false) const activeRequest = queue[0] ?? null const activeRequestRef = useRef(activeRequest) const setContextualToursBlockingSurfaceVisible = useAppStore( @@ -62,19 +65,28 @@ export function ConfirmationDialogProvider({ }) }, []) - const settleActiveRequest = useCallback((confirmed: boolean) => { - const request = activeRequestRef.current - if (!request) { - return - } - request.resolve(confirmed) - setQueue((currentQueue) => { - if (currentQueue[0]?.id === request.id) { - return currentQueue.slice(1) + const settleActiveRequest = useCallback( + (confirmed: boolean) => { + const request = activeRequestRef.current + if (!request) { + return } - return currentQueue.filter((queuedRequest) => queuedRequest.id !== request.id) - }) - }, []) + // Why: cancelling must not persist a preference the user backed out of. + if (confirmed && dontAskAgain) { + request.options.dontAskAgain?.onConfirmed() + } + // Why: queued prompts must not inherit this request's preference. + setDontAskAgain(false) + request.resolve(confirmed) + setQueue((currentQueue) => { + if (currentQueue[0]?.id === request.id) { + return currentQueue.slice(1) + } + return currentQueue.filter((queuedRequest) => queuedRequest.id !== request.id) + }) + }, + [dontAskAgain] + ) return ( @@ -90,6 +102,22 @@ export function ConfirmationDialogProvider({ {displayedRequest.options.description} ) : null} + {displayedRequest?.options.dontAskAgain ? ( +
+ setDontAskAgain(checked === true)} + /> + +
+ ) : null}
+
+ + + updateSettings({ + skipDeleteArtifactConfirm: !settings.skipDeleteArtifactConfirm + }) + } + /> + +
+