feat(desktop): default v2 workspaces to all devices, sort by host then created#3928
feat(desktop): default v2 workspaces to all devices, sort by host then created#3928saddlepaddle merged 1 commit intomainfrom
Conversation
…n created - Default device filter is now "All devices" (was "This device") - Default sort is host asc (local device first), createdAt desc as tiebreak - Show CgLaptop icon next to main workspace names, matching sidebar
📝 WalkthroughWalkthroughThe changes modify the V2 workspaces feature to alter default sorting behavior from created (descending) to host (ascending), introduce a main workspace visual indicator icon in workspace rows, and update the default device filter from THIS_DEVICE to ALL across the filtering store. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Greptile SummaryThis PR updates v2 workspaces to default the device filter to All devices (previously "This device"), changes the default sort to Confidence Score: 5/5Safe to merge — all logic changes are correct and well-scoped, with only a minor keyboard-accessibility gap in the new tooltip. All findings are P2 style/accessibility suggestions. The sort tiebreak ( V2WorkspaceRow.tsx — minor keyboard-accessibility improvement on the
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx | Changed default sort from created/desc to host/asc and updated the hasActiveFilters baseline to DEVICE_FILTER_ALL. Logic is correct — compareWorkspaces with host/asc produces local-device first, then alphabetical hostname, then newest-first tiebreak. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx | Added CgLaptop icon with tooltip for main workspaces. Icon is not keyboard-focusable (no tabIndex), so tooltip is hover-only; aria-label partially mitigates accessibility gap. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/stores/v2WorkspacesFilterStore/v2WorkspacesFilterStore.ts | Initial and reset deviceFilter changed from DEVICE_FILTER_THIS_DEVICE to DEVICE_FILTER_ALL. Consistent and correct change. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[V2WorkspacesList mounts] --> B[sortField='host', sortDirection='asc']
A --> C[deviceFilter='all' from store]
B & C --> D[groupByProject]
D --> E[compareWorkspaces per project]
E --> F{Sort field?}
F -->|host| G[hostTypeRank: local=0, remote=1]
G --> H{Tie?}
H -->|no| I[Apply direction]
H -->|yes| J[localeCompare hostName]
J --> K{Tie?}
K -->|no| I
K -->|yes| L[tiebreak: b.createdAt - a.createdAt]
L --> I
I --> M[Sorted workspaces per project]
M --> N[Projects sorted by latestCreatedAt desc]
N --> O[Render rows]
O --> P{isMainWorkspace?}
P -->|yes| Q[CgLaptop icon + tooltip]
P -->|no| R[Name only]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx:214-224
**Tooltip not keyboard-accessible**
`CgLaptop` is a bare SVG with no `tabIndex`, so keyboard users can never focus it to trigger the tooltip. The `aria-label="Main workspace"` on the SVG does expose the information to screen readers, but interactive tooltip hover-on-focus is lost. Adding a `tabIndex={0}` to the element (or wrapping it in a focusable `<span>`) would allow keyboard users to access the tooltip as well.
```suggestion
<CgLaptop
tabIndex={0}
className="size-3.5 shrink-0 text-muted-foreground"
aria-label="Main workspace"
/>
```
Reviews (1): Last reviewed commit: "feat(desktop): default v2 workspaces to ..." | Re-trigger Greptile
|
|
||
| <span className="flex min-w-0 items-center gap-2"> | ||
| {isMainWorkspace ? ( | ||
| <Tooltip delayDuration={300}> | ||
| <TooltipTrigger asChild> | ||
| <CgLaptop | ||
| className="size-3.5 shrink-0 text-muted-foreground" | ||
| aria-label="Main workspace" | ||
| /> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="top">Main workspace</TooltipContent> |
There was a problem hiding this comment.
Tooltip not keyboard-accessible
CgLaptop is a bare SVG with no tabIndex, so keyboard users can never focus it to trigger the tooltip. The aria-label="Main workspace" on the SVG does expose the information to screen readers, but interactive tooltip hover-on-focus is lost. Adding a tabIndex={0} to the element (or wrapping it in a focusable <span>) would allow keyboard users to access the tooltip as well.
| <span className="flex min-w-0 items-center gap-2"> | |
| {isMainWorkspace ? ( | |
| <Tooltip delayDuration={300}> | |
| <TooltipTrigger asChild> | |
| <CgLaptop | |
| className="size-3.5 shrink-0 text-muted-foreground" | |
| aria-label="Main workspace" | |
| /> | |
| </TooltipTrigger> | |
| <TooltipContent side="top">Main workspace</TooltipContent> | |
| <CgLaptop | |
| tabIndex={0} | |
| className="size-3.5 shrink-0 text-muted-foreground" | |
| aria-label="Main workspace" | |
| /> |
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: 214-224
Comment:
**Tooltip not keyboard-accessible**
`CgLaptop` is a bare SVG with no `tabIndex`, so keyboard users can never focus it to trigger the tooltip. The `aria-label="Main workspace"` on the SVG does expose the information to screen readers, but interactive tooltip hover-on-focus is lost. Adding a `tabIndex={0}` to the element (or wrapping it in a focusable `<span>`) would allow keyboard users to access the tooltip as well.
```suggestion
<CgLaptop
tabIndex={0}
className="size-3.5 shrink-0 text-muted-foreground"
aria-label="Main workspace"
/>
```
How can I resolve this? If you propose a fix, please make it concise.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 166-169: The empty-state message in V2WorkspacesList is still
instructing users only to clear the device filter even though hasActiveFilters
(derived from searchQuery, deviceFilter vs DEVICE_FILTER_ALL, and projectFilter
vs PROJECT_FILTER_ALL) now includes search and project filters; update the
empty-state copy to reference clearing filters more generally (e.g., “clear
filters or search terms”) or dynamically mention which filter is active so users
know to clear the device, project, or search query as appropriate; adjust the
text displayed where the empty-state is rendered in V2WorkspacesList to use the
broader copy or to inspect deviceFilter, projectFilter, and searchQuery to
tailor the suggestion.
🪄 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: 9d57b406-4b57-4daf-a56c-47c1f80a4607
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/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/stores/v2WorkspacesFilterStore/v2WorkspacesFilterStore.ts
| const hasActiveFilters = | ||
| searchQuery.trim() !== "" || | ||
| deviceFilter !== DEVICE_FILTER_THIS_DEVICE || | ||
| deviceFilter !== DEVICE_FILTER_ALL || | ||
| projectFilter !== PROJECT_FILTER_ALL; |
There was a problem hiding this comment.
Broaden the empty-state guidance.
hasActiveFilters now covers search, device, and project filters, but the empty-state copy still tells users to clear only the device filter. When the project filter is what excludes results, this points them at the wrong control.
Suggested copy tweak
- "Try a different search term or clear the device filter."
+ "Try a different search term or clear the filters."🤖 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 166 - 169, The empty-state message in V2WorkspacesList is still
instructing users only to clear the device filter even though hasActiveFilters
(derived from searchQuery, deviceFilter vs DEVICE_FILTER_ALL, and projectFilter
vs PROJECT_FILTER_ALL) now includes search and project filters; update the
empty-state copy to reference clearing filters more generally (e.g., “clear
filters or search terms”) or dynamically mention which filter is active so users
know to clear the device, project, or search query as appropriate; adjust the
text displayed where the empty-state is rendered in V2WorkspacesList to use the
broader copy or to inspect deviceFilter, projectFilter, and searchQuery to
tailor the suggestion.
Summary
createdAt descas the natural tiebreak fromcompareWorkspacesCgLaptopicon used in the sidebar next to their name, with a "Main workspace" tooltipTest plan
Summary by CodeRabbit
New Features
Updates