Repository navigation
Redesign artifacts page as full-width table with drawer - #15233
Conversation
- Artifacts list displays as a compact data table with columns (Name, Type, Size, Updated, Expires) - Selected artifact opens in a right-side drawer instead of inline preview - Search and refresh consolidated in top toolbar - Better space utilization for browsing the artifact list
- Extract common list-table styles (container, header, row) to @/lib for consistency across artifacts and automations tables - Move row interaction utilities to @/lib/list-row-interaction for reuse - Fix drawer width to calc(100vw-80px) to avoid macOS traffic-light controls - Extract WINDOW_CONTROLS_WIDTH/HEIGHT constants so portaled surfaces avoid the Windows/Linux overlay without hardcoding pixels - Clamp artifact search query to 2KB to prevent multi-MB pastes from pinning renderer memory - Remove unused artifact list visual mock
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe artifacts page now uses a searchable, refreshable table with compact metadata and row action menus. Selecting a row opens an independent detail drawer with preview and deletion controls. Loading, authentication, error, empty, and skeleton states are extracted into dedicated components. Shared list-table styling now supports both artifacts and automations. Window-control dimensions are centralized and applied to app and portaled surfaces. Tests and localization strings were updated. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/renderer/src/components/artifacts/ArtifactListRows.tsx (1)
97-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated menu actions into a shared list.
The
DropdownMenuContentblock and theContextMenuContentblock define the same three actions (copy link, open in browser, delete artifact) with identical icons, labels, and handlers. A future change to one of these actions requires editing both blocks, and it is easy to let them drift.Extract the action definitions into a single array (label, icon,
onSelect, optionalvariant/disabled) and map it into bothDropdownMenuItemandContextMenuItemlists.♻️ Proposed refactor sketch
const rowActions = [ { key: 'copy', label: translate('auto.components.artifacts.copyLink', 'Copy link'), icon: Copy, onSelect: () => void copyArtifactLink(item.shareUrl) }, { key: 'open', label: translate('auto.components.artifacts.openInBrowser', 'Open in browser'), icon: ExternalLink, onSelect: () => openArtifactInBrowser(item.shareUrl) } ] as const // ... <DropdownMenuContent align="end" className="w-48"> {rowActions.map(({ key, label, icon: Icon, onSelect }) => ( <DropdownMenuItem key={key} onSelect={onSelect}> <Icon className="size-3.5" /> {label} </DropdownMenuItem> ))} <DropdownMenuSeparator /> <DropdownMenuItem variant="destructive" disabled={deleting} onSelect={() => deleteArtifact(item)}> <Trash2 className="size-3.5" /> {translate('auto.components.artifacts.ArtifactsPage.deleteArtifact', 'Delete artifact')} </DropdownMenuItem> </DropdownMenuContent>Apply the same
rowActions.map(...)pattern insideContextMenuContent.Also applies to: 138-160
src/renderer/src/components/artifacts/ArtifactsPageSkeleton.tsx (1)
21-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a role so the loading label is announced.
The root element is a plain
div. Screen readers do not exposearia-labelon a generic element without a role. Addrole="status"so the "Loading artifacts" label reaches assistive technology.♻️ Proposed change
<div className="flex min-h-0 flex-1 flex-col gap-4 overflow-hidden px-3 pb-4 md:px-5" + role="status" aria-busy="true" aria-label={translate( 'auto.components.artifacts.ArtifactsPageSkeleton.loading', 'Loading artifacts' )} >src/renderer/src/components/artifacts/ArtifactDetailDrawer.tsx (1)
22-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrive the reserved edge width from a platform-aware value instead of a hardcoded 80px.
The comment states the inset keeps the macOS traffic-light area uncovered. macOS paints the traffic lights on the left edge. Windows and Linux paint the window controls on the right edge, where this drawer is anchored. So the 80px left gap applies a macOS-only reservation on every platform, and Windows and Linux get an unnecessary gap.
This PR already centralizes window-control dimensions and exposes
--window-controls-width, whichArtifactDetailHeader.tsxconsumes. Reuse the same custom property here so the reserved space follows the host platform.♻️ Proposed direction
- className="h-full w-[min(96rem,calc(100vw-80px))] max-w-none translate-x-0 p-0 sm:max-w-[min(96rem,calc(100vw-80px))] data-[state=closed]:translate-x-0 data-[state=open]:translate-x-0" + className="h-full w-[min(96rem,calc(100vw-var(--window-controls-inset,80px)))] max-w-none translate-x-0 p-0 sm:max-w-[min(96rem,calc(100vw-var(--window-controls-inset,80px)))] data-[state=closed]:translate-x-0 data-[state=open]:translate-x-0"Set
--window-controls-insetnext to the existing window-control dimension constants, and resolve it to0pxwhen the platform places no controls on the uncovered edge.As per coding guidelines: "Keep platform-dependent behavior behind runtime checks for macOS, Linux, and Windows."
Source: Coding guidelines
src/renderer/src/components/artifacts/ArtifactsPage.test.tsx (1)
570-578: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the helper to match what it does.
The helper does not act on a selected artifact. It picks the first
context-menu-triggerin the document, left-clicks it to open the drawer, and then deletes through the drawer action menu. It also does not use the row context menu. In tests that render more than one row, the first row is always the target.Rename it to
deleteFirstArtifactRowFromDrawerMenu, or accept a row name and query by that name, so the call sites at Line 464, Line 501, Line 521, and Line 537 state which artifact they delete.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c599aaec-66c4-46f5-a6a7-8d47d4931927
📒 Files selected for processing (36)
src/renderer/src/App.tsxsrc/renderer/src/app-shell/AppWorkspaceShell.tsxsrc/renderer/src/app-shell/app-window-chrome.tssrc/renderer/src/components/artifacts/ArtifactActions.tsxsrc/renderer/src/components/artifacts/ArtifactCollection.test.tsxsrc/renderer/src/components/artifacts/ArtifactCollection.tsxsrc/renderer/src/components/artifacts/ArtifactDetailDrawer.tsxsrc/renderer/src/components/artifacts/ArtifactDetailHeader.tsxsrc/renderer/src/components/artifacts/ArtifactListPane.tsxsrc/renderer/src/components/artifacts/ArtifactListRows.tsxsrc/renderer/src/components/artifacts/ArtifactListSearchField.tsxsrc/renderer/src/components/artifacts/ArtifactListTableHeader.tsxsrc/renderer/src/components/artifacts/ArtifactListToolbar.tsxsrc/renderer/src/components/artifacts/ArtifactsPage.test.tsxsrc/renderer/src/components/artifacts/ArtifactsPage.tsxsrc/renderer/src/components/artifacts/ArtifactsPageSkeleton.tsxsrc/renderer/src/components/artifacts/ArtifactsPageStates.tsxsrc/renderer/src/components/artifacts/artifact-display-labels.test.tssrc/renderer/src/components/artifacts/artifact-display-labels.tssrc/renderer/src/components/artifacts/artifact-list-search.test.tssrc/renderer/src/components/artifacts/artifact-list-search.tssrc/renderer/src/components/artifacts/artifacts-table-layout.tssrc/renderer/src/components/automations/AutomationListExternalRows.tsxsrc/renderer/src/components/automations/AutomationListLocalRows.tsxsrc/renderer/src/components/automations/AutomationListTableHeader.tsxsrc/renderer/src/components/automations/AutomationsListPanel.tsxsrc/renderer/src/components/automations/AutomationsPageSkeleton.tsxsrc/renderer/src/components/automations/automations-table-layout.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/lib/list-row-interaction.test.tssrc/renderer/src/lib/list-row-interaction.tssrc/renderer/src/lib/list-table-layout.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/components/artifacts/ArtifactListPane.tsx
Included review availability: Your plan includes up to 10 reviews per rolling hour; 5 remain after this review.
|
@coderabbitai Fixing. Will be in the next commit |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
- Unify dropdown and context menu actions via artifactRowActions() to prevent them from diverging during future maintenance. - Replace hardcoded 80px with platform-aware CSS variable (--mac-traffic-lights-width) so only macOS reserves space for traffic lights; Windows and Linux controls sit on the right edge instead.
Resolve ko.json artifacts conflict: keep the shipped wording from the artifacts page redesign (stablyai#15233), drop keys it removed, translate the ones it added. # Conflicts: # src/renderer/src/i18n/locales/ko.json
* refactor(artifacts): redesign as full-width table with detail drawer - Artifacts list displays as a compact data table with columns (Name, Type, Size, Updated, Expires) - Selected artifact opens in a right-side drawer instead of inline preview - Search and refresh consolidated in top toolbar - Better space utilization for browsing the artifact list * refactor(artifacts,automations): extract shared list-table layout - Extract common list-table styles (container, header, row) to @/lib for consistency across artifacts and automations tables - Move row interaction utilities to @/lib/list-row-interaction for reuse - Fix drawer width to calc(100vw-80px) to avoid macOS traffic-light controls - Extract WINDOW_CONTROLS_WIDTH/HEIGHT constants so portaled surfaces avoid the Windows/Linux overlay without hardcoding pixels - Clamp artifact search query to 2KB to prevent multi-MB pastes from pinning renderer memory - Remove unused artifact list visual mock * Extract shared artifact row actions and use CSS var for traffic lights - Unify dropdown and context menu actions via artifactRowActions() to prevent them from diverging during future maintenance. - Replace hardcoded 80px with platform-aware CSS variable (--mac-traffic-lights-width) so only macOS reserves space for traffic lights; Windows and Linux controls sit on the right edge instead.
* refactor(artifacts): redesign as full-width table with detail drawer - Artifacts list displays as a compact data table with columns (Name, Type, Size, Updated, Expires) - Selected artifact opens in a right-side drawer instead of inline preview - Search and refresh consolidated in top toolbar - Better space utilization for browsing the artifact list * refactor(artifacts,automations): extract shared list-table layout - Extract common list-table styles (container, header, row) to @/lib for consistency across artifacts and automations tables - Move row interaction utilities to @/lib/list-row-interaction for reuse - Fix drawer width to calc(100vw-80px) to avoid macOS traffic-light controls - Extract WINDOW_CONTROLS_WIDTH/HEIGHT constants so portaled surfaces avoid the Windows/Linux overlay without hardcoding pixels - Clamp artifact search query to 2KB to prevent multi-MB pastes from pinning renderer memory - Remove unused artifact list visual mock * Extract shared artifact row actions and use CSS var for traffic lights - Unify dropdown and context menu actions via artifactRowActions() to prevent them from diverging during future maintenance. - Replace hardcoded 80px with platform-aware CSS variable (--mac-traffic-lights-width) so only macOS reserves space for traffic lights; Windows and Linux controls sit on the right edge instead.
* refactor(artifacts): redesign as full-width table with detail drawer - Artifacts list displays as a compact data table with columns (Name, Type, Size, Updated, Expires) - Selected artifact opens in a right-side drawer instead of inline preview - Search and refresh consolidated in top toolbar - Better space utilization for browsing the artifact list * refactor(artifacts,automations): extract shared list-table layout - Extract common list-table styles (container, header, row) to @/lib for consistency across artifacts and automations tables - Move row interaction utilities to @/lib/list-row-interaction for reuse - Fix drawer width to calc(100vw-80px) to avoid macOS traffic-light controls - Extract WINDOW_CONTROLS_WIDTH/HEIGHT constants so portaled surfaces avoid the Windows/Linux overlay without hardcoding pixels - Clamp artifact search query to 2KB to prevent multi-MB pastes from pinning renderer memory - Remove unused artifact list visual mock * Extract shared artifact row actions and use CSS var for traffic lights - Unify dropdown and context menu actions via artifactRowActions() to prevent them from diverging during future maintenance. - Replace hardcoded 80px with platform-aware CSS variable (--mac-traffic-lights-width) so only macOS reserves space for traffic lights; Windows and Linux controls sit on the right edge instead.
Problem
Artifacts page had a split sidebar layout that didn't scale visually and wasn't composable with Orca's table patterns. Window-control positioning only worked inline, breaking portaled surfaces (sheets, dialogs). List-table styling wasn't shared with automations, causing maintenance drift.
Solution
Redesigned artifacts as a full-width table with a right-side detail drawer for previews. Extracted shared list-table chrome to
@/lib/list-table-layoutso both artifacts and automations use consistent styling. Exported window-controls dimensions as constants so portaled surfaces can inset correctly without hardcoding pixels.What Changed
@/lib/list-table-layout(shared container, header, row styles),@/lib/list-row-interaction(moved from automations)Why
Separating preview from list enables better space use and matches how automations work. Shared layout prevents visual drift. Window-control constants work both inline and for portaled surfaces.
Linked Issue
Fixes #
Visual Proof
N/A — Layout refactor tested via component tests; visual consistency verified through test snapshots. Automations table renders identically after extraction.
Testing
AI Disclosure
N/A — internal contributor
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Ensure no issues in: Security, Cross-platform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)