Conversation
…able Replace stacked card rows with a dense full-width table: Sidebar / Name / Host / Branch / Created columns that align across projects, sortable column headers, proper truncation on long workspace and host names, and hover-revealed row actions.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe V2 workspaces UI was refactored: header compacted into a single bordered row; workspaces data unified into a single array with client-side sorting and project grouping; rows moved to a grid-based list; a separate device-badge component was removed and its logic inlined. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Header as V2WorkspacesHeader
participant List as V2WorkspacesList
participant Row as V2WorkspaceRow
User->>Header: Type search / toggle device filter
Header->>List: Provide search/filter params
User->>Header: Click sortable column
Header->>List: Update sortField/sortDirection
List->>List: Sort workspaces (hostTypeRank, compareWorkspaces) & group by project
List->>Row: Render each workspace in grid row
Row->>Row: Determine host icon & offline state
Row-->>User: Display workspace row with metadata and actions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR replaces the Key changes:
One P1 issue found: the sidebar column's sort comparator and its default direction are mismatched, causing non-sidebar items to appear at the top when the Sidebar column is first clicked. Confidence Score: 4/5Safe to merge after fixing the sidebar sort-direction inversion; all other changes are clean UI improvements. The overall redesign is well-structured — shared grid constant, responsive breakpoints, accessible keyboard navigation, and proper ARIA attributes are all handled correctly. One concrete P1 logic bug exists: the sidebar comparator formula (b - a) and its DEFAULT_DIRECTION_BY_FIELD 'desc' are mismatched, so the first click on the Sidebar column header surfaces non-sidebar items at the top. This is a noticeable UX inversion for a primary sorting affordance but does not affect data integrity or break navigation. The P2 offline tooltip regression is minor. Everything else looks solid. V2WorkspacesList.tsx — sidebar sort direction logic around line 83 needs a one-line fix.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx | Core list rewrite — adds client-side sorting and responsive column header. Contains a P1 sort-direction inversion for the Sidebar column that causes non-sidebar items to appear first on first click. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx | Row redesigned from card (Item) to table row (li + div[role=button]); icon-only sidebar buttons with Tooltip; offline status regressed from Tooltip to native title attribute. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesHeader/V2WorkspacesHeader.tsx | Header condensed to single line; removes subtitle description; search bar narrowed to fixed w-72; device filter toggle preserved; clean, no issues. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/constants.ts | New shared grid-template constant used by both column header and workspace rows to ensure column alignment. Clean approach. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/page.tsx | Updated to consume all from the hook instead of pinned/others; trivial and correct. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/components/V2WorkspaceDeviceBadge/V2WorkspaceDeviceBadge.tsx | Deleted — host badge functionality moved inline into V2WorkspaceRow. Offline tooltip was a Tooltip component; now only a title attribute (see row comment). |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[page.tsx] -->|all workspaces| B[V2WorkspacesList]
A -->|counts| C[V2WorkspacesHeader]
B --> D{totalCount = 0?}
D -->|yes| E[Empty state with sticky header]
D -->|no| F[ScrollArea]
F --> G[Sticky SortableHeader row]
G -->|onSort callback| H[handleSort updates sortField and sortDirection]
H --> I[useMemo projectGroups - filter then groupByProject then sort]
I --> F
F --> J[project group header band]
J --> K[ul of V2WorkspaceRow]
K --> L1[Sidebar dot indicator]
K --> L2[Name truncated with title]
K --> L3[Host icon and name - hidden below md]
K --> L4[Branch mono - hidden below lg]
K --> L5[Created time - hidden below xl]
K --> L6[Add or Remove sidebar icon button with Tooltip]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx
Line: 83-84
Comment:
**Sidebar sort direction is inverted on first click**
The comparator `Number(b.isInSidebar) - Number(a.isInSidebar)` is already "descending" by construction — it returns a negative value when `a` is in the sidebar, meaning sidebar items sort first when `direction === "asc"`. However, the default direction for this field is `"desc"`, which negates the result and therefore puts **non-sidebar items first** the first time a user clicks the Sidebar column header.
Step through the math with `a.isInSidebar = true`, `b.isInSidebar = false`:
- `cmp = Number(false) - Number(true) = 0 - 1 = -1`
- `direction === "desc"` → `return -cmp = 1` → `b` sorts before `a` → non-sidebar first ✗
Either flip the default direction to `"asc"` for the sidebar field:
```suggestion
const DEFAULT_DIRECTION_BY_FIELD: Record<SortField, SortDirection> = {
sidebar: "asc",
name: "asc",
host: "asc",
branch: "asc",
created: "desc",
};
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx
Line: 131-146
Comment:
**Offline tooltip regressed to native browser title**
The old `V2WorkspaceDeviceBadge` showed a styled `<Tooltip>` ("Host is offline") for offline remote hosts. The new code drops to a plain `title` attribute on the host `<span>`. The `title` text is only exposed on mouse hover (not keyboard focus) and renders as an unstyled OS tooltip, which doesn't match the rest of the UI — especially now that the sidebar-action buttons use the `<Tooltip>` component right next to it. Consider wrapping the offline host cell in a `<Tooltip>` similar to the button tooltips when `treatAsOffline` is true.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(desktop): redesign v2-workspaces li..." | Re-trigger Greptile
| case "sidebar": | ||
| cmp = Number(b.isInSidebar) - Number(a.isInSidebar); |
There was a problem hiding this comment.
Sidebar sort direction is inverted on first click
The comparator Number(b.isInSidebar) - Number(a.isInSidebar) is already "descending" by construction — it returns a negative value when a is in the sidebar, meaning sidebar items sort first when direction === "asc". However, the default direction for this field is "desc", which negates the result and therefore puts non-sidebar items first the first time a user clicks the Sidebar column header.
Step through the math with a.isInSidebar = true, b.isInSidebar = false:
cmp = Number(false) - Number(true) = 0 - 1 = -1direction === "desc"→return -cmp = 1→bsorts beforea→ non-sidebar first ✗
Either flip the default direction to "asc" for the sidebar field:
| case "sidebar": | |
| cmp = Number(b.isInSidebar) - Number(a.isInSidebar); | |
| const DEFAULT_DIRECTION_BY_FIELD: Record<SortField, SortDirection> = { | |
| sidebar: "asc", | |
| name: "asc", | |
| host: "asc", | |
| branch: "asc", | |
| created: "desc", | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx
Line: 83-84
Comment:
**Sidebar sort direction is inverted on first click**
The comparator `Number(b.isInSidebar) - Number(a.isInSidebar)` is already "descending" by construction — it returns a negative value when `a` is in the sidebar, meaning sidebar items sort first when `direction === "asc"`. However, the default direction for this field is `"desc"`, which negates the result and therefore puts **non-sidebar items first** the first time a user clicks the Sidebar column header.
Step through the math with `a.isInSidebar = true`, `b.isInSidebar = false`:
- `cmp = Number(false) - Number(true) = 0 - 1 = -1`
- `direction === "desc"` → `return -cmp = 1` → `b` sorts before `a` → non-sidebar first ✗
Either flip the default direction to `"asc"` for the sidebar field:
```suggestion
const DEFAULT_DIRECTION_BY_FIELD: Record<SortField, SortDirection> = {
sidebar: "asc",
name: "asc",
host: "asc",
branch: "asc",
created: "desc",
};
```
How can I resolve this? If you propose a fix, please make it concise.| <span | ||
| className={cn( | ||
| "hidden min-w-0 items-center gap-1.5 text-xs text-muted-foreground md:flex", | ||
| treatAsOffline && "text-muted-foreground/60", | ||
| )} | ||
| title={`${workspace.hostName}${treatAsOffline ? " (offline)" : ""}`} | ||
| > | ||
| <HostIcon className="size-3 shrink-0" /> | ||
| <span className="min-w-0 truncate">{workspace.hostName}</span> | ||
| {treatAsOffline ? ( | ||
| <span | ||
| aria-hidden | ||
| className="inline-block size-1.5 shrink-0 rounded-full bg-muted-foreground/40" | ||
| /> | ||
| ) : null} | ||
| </span> |
There was a problem hiding this comment.
Offline tooltip regressed to native browser title
The old V2WorkspaceDeviceBadge showed a styled <Tooltip> ("Host is offline") for offline remote hosts. The new code drops to a plain title attribute on the host <span>. The title text is only exposed on mouse hover (not keyboard focus) and renders as an unstyled OS tooltip, which doesn't match the rest of the UI — especially now that the sidebar-action buttons use the <Tooltip> component right next to it. Consider wrapping the offline host cell in a <Tooltip> similar to the button tooltips when treatAsOffline is true.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx
Line: 131-146
Comment:
**Offline tooltip regressed to native browser title**
The old `V2WorkspaceDeviceBadge` showed a styled `<Tooltip>` ("Host is offline") for offline remote hosts. The new code drops to a plain `title` attribute on the host `<span>`. The `title` text is only exposed on mouse hover (not keyboard focus) and renders as an unstyled OS tooltip, which doesn't match the rest of the UI — especially now that the sidebar-action buttons use the `<Tooltip>` component right next to it. Consider wrapping the offline host cell in a `<Tooltip>` similar to the button tooltips when `treatAsOffline` is true.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx (1)
82-90:⚠️ Potential issue | 🟠 MajorPrevent nested button keyboard events from opening the row.
Keyboard events (Enter/Space) from focused Add/Remove buttons bubble to the parent row's
onKeyDownhandler, causing unwanted navigation. The nested buttons' click handlers usestopPropagation(), but this only prevents click event bubbling, not keyboard event bubbling. Ignore child-originated keyboard events before callinghandleOpen.🐛 Proposed fix
const handleRowKeyDown = useCallback( (event: React.KeyboardEvent<HTMLDivElement>) => { + if (event.target !== event.currentTarget) { + return; + } if (event.key === "Enter" || event.key === " ") { event.preventDefault(); handleOpen(); } }, [handleOpen], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx` around lines 82 - 90, The row's onKeyDown handler (handleRowKeyDown) should ignore keyboard events that originate from nested interactive elements so Enter/Space on child Add/Remove buttons don't trigger handleOpen; modify handleRowKeyDown to early-return when the event target is not the row itself (e.g., if (event.target !== event.currentTarget) return;) or when (event.target as HTMLElement).closest('button') is truthy, then only proceed to call handleOpen for genuine row-level key presses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx`:
- Around line 150-205: Extract the SortableHeader component into its own
co-located component folder: create SortableHeader/SortableHeader.tsx containing
the SortableHeader function and its SortableHeaderProps interface, export it as
the default export, and add an index.ts barrel exporting default; then remove
the SortableHeader declaration from the V2WorkspacesList file and replace it
with an import of SortableHeader (e.g. import SortableHeader from
'./SortableHeader'), keeping all used symbols (LuChevronsUpDown, LuChevronUp,
LuChevronDown, cn, SortField, SortDirection) and prop usage identical so
behavior is unchanged.
- Around line 83-85: The sidebar comparator is inverted: replace the current
line cmp = Number(b.isInSidebar) - Number(a.isInSidebar); with cmp =
Number(a.isInSidebar) - Number(b.isInSidebar); (or an equivalent boolean
comparator that yields a.isInSidebar before b.isInSidebar) in the case "sidebar"
block inside V2WorkspacesList (and apply the same change to the other sidebar
comparator occurrence later in the file) so that descending sort places
workspaces that are in the sidebar first.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx`:
- Around line 82-90: The row's onKeyDown handler (handleRowKeyDown) should
ignore keyboard events that originate from nested interactive elements so
Enter/Space on child Add/Remove buttons don't trigger handleOpen; modify
handleRowKeyDown to early-return when the event target is not the row itself
(e.g., if (event.target !== event.currentTarget) return;) or when (event.target
as HTMLElement).closest('button') is truthy, then only proceed to call
handleOpen for genuine row-level key presses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2085635-b061-453c-88d3-777d34a5cd08
📒 Files selected for processing (7)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesHeader/V2WorkspacesHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/components/V2WorkspaceDeviceBadge/V2WorkspaceDeviceBadge.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/components/V2WorkspaceDeviceBadge/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/constants.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/page.tsx
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/components/V2WorkspaceDeviceBadge/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/components/V2WorkspaceDeviceBadge/V2WorkspaceDeviceBadge.tsx
| interface SortableHeaderProps { | ||
| field: SortField; | ||
| label: string; | ||
| align?: "start" | "center"; | ||
| className?: string; | ||
| sortField: SortField; | ||
| sortDirection: SortDirection; | ||
| onSort: (field: SortField) => void; | ||
| srOnlyLabel?: boolean; | ||
| } | ||
|
|
||
| export function V2WorkspacesList({ pinned, others }: V2WorkspacesListProps) { | ||
| function SortableHeader({ | ||
| field, | ||
| label, | ||
| align = "start", | ||
| className, | ||
| sortField, | ||
| sortDirection, | ||
| onSort, | ||
| srOnlyLabel = false, | ||
| }: SortableHeaderProps) { | ||
| const isActive = sortField === field; | ||
| const Icon = !isActive | ||
| ? LuChevronsUpDown | ||
| : sortDirection === "asc" | ||
| ? LuChevronUp | ||
| : LuChevronDown; | ||
| const sortLabel = isActive | ||
| ? sortDirection === "asc" | ||
| ? "ascending" | ||
| : "descending" | ||
| : "not sorted"; | ||
|
|
||
| return ( | ||
| <button | ||
| type="button" | ||
| onClick={() => onSort(field)} | ||
| aria-label={`Sort by ${label}, currently ${sortLabel}`} | ||
| className={cn( | ||
| "group flex min-w-0 items-center gap-1 rounded outline-none transition-colors", | ||
| "hover:text-foreground focus-visible:ring-2 focus-visible:ring-ring/40", | ||
| align === "center" && "justify-center", | ||
| isActive && "text-foreground", | ||
| className, | ||
| )} | ||
| > | ||
| <span className={cn("truncate", srOnlyLabel && "sr-only")}>{label}</span> | ||
| <Icon | ||
| className={cn( | ||
| "size-3 shrink-0 transition-opacity", | ||
| isActive ? "opacity-100" : "opacity-0 group-hover:opacity-60", | ||
| )} | ||
| /> | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move SortableHeader into its own component file.
This file now defines both SortableHeader and V2WorkspacesList; please split the header into a co-located SortableHeader/SortableHeader.tsx with an index.ts barrel. As per coding guidelines, "**/*.{tsx,ts}: Do not create multi-component files; use one component per file." Also: "**/*.tsx: Use one folder per component with structure: ComponentName/ComponentName.tsx + index.ts for barrel export."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx`
around lines 150 - 205, Extract the SortableHeader component into its own
co-located component folder: create SortableHeader/SortableHeader.tsx containing
the SortableHeader function and its SortableHeaderProps interface, export it as
the default export, and add an index.ts barrel exporting default; then remove
the SortableHeader declaration from the V2WorkspacesList file and replace it
with an import of SortableHeader (e.g. import SortableHeader from
'./SortableHeader'), keeping all used symbols (LuChevronsUpDown, LuChevronUp,
LuChevronDown, cn, SortField, SortDirection) and prop usage identical so
behavior is unchanged.
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx:84">
P2: Sidebar sort direction is inverted because the comparator is pre-reversed before the shared asc/desc inversion.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx:136">
P2: The offline host indicator regressed from a styled `<Tooltip>` (as used by the adjacent sidebar buttons) to a plain `title` attribute. Native `title` tooltips don't appear on keyboard focus and look inconsistent with the rest of the row. Wrap the offline host cell in a `<Tooltip>` when `treatAsOffline` is true, matching the pattern already used for the action buttons.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Sidebar column sort direction was inverted (default "desc" put non-sidebar items first); use a normal boolean comparator so descending means "in sidebar first". - Row onKeyDown was firing when Enter/Space bubbled from focused action buttons; ignore keyboard events originating from descendants. - Offline host cell regressed to a native `title` tooltip; wrap it in the shared `<Tooltip>` so the indicator is keyboard-accessible and styled consistently with the action buttons. - Extract SortableHeader into its own folder per repo conventions (one component per file) and move shared sort types into a types module.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
Sidebar · Name · Host · Branch · Created) align across all projects, each column header is a sort toggle, and rows truncate properly on long names/branches/hosts with title tooltips.Add to sidebar/Remove from sidebar) became icon-only buttons revealed on hover/focus, reclaiming horizontal space.Test plan
/v2-workspacesroute and verify projects render with aligned columns.Tabonto a row, pressEnterorSpaceto open; focus ring is visible.Summary by cubic
Redesigned the v2-workspaces view as a dense, Linear-style sortable table for faster scanning and consistent alignment, replacing stacked rows and the sidebar/others split. Improves accessibility, responsiveness, sticky column header, and row actions while keeping project grouping.
New Features
Bug Fixes
Written for commit 2e27732. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI Improvements