Conversation
…e-on-the-sidebar-icons-for-local-should-be-super-simple-icon-for-remote-sh
📝 WalkthroughWalkthroughThe PR extends the dashboard sidebar workspace data model to include a Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 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 adds host-type-aware and offline-state-aware visual feedback to workspace icons and tooltips in the dashboard sidebar. Local workspaces now display a minimal dot, remote/cloud workspaces display a cloud icon, and remote workspaces whose associated host is offline display a dimmed Key changes:
Confidence Score: 5/5Safe to merge — the changes are additive, well-scoped, and the offline detection logic is correct. All data-flow paths are consistent: hostIsOnline is gated to null for non-remote-device types at the data layer and the strict === false check correctly differentiates confirmed offline from status unknown (null). The icon/tooltip logic covers all three host types without gaps. No existing behavior is broken. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceIcon/DashboardSidebarWorkspaceIcon.tsx | Replaces type-based icon logic with a renderHostIcon() helper; local-device gets a small dot, offline remote-device gets LuCloudOff at 60% opacity, everything else gets LuCloud. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts | Adds hostIsOnline field to the live query and gates it so only remote-device workspaces carry a non-null value. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsx | Passes hostIsOnline to DashboardSidebarWorkspaceIcon and replaces generic tooltip copy with type-specific and offline-aware strings. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/types.ts | Adds hostIsOnline: boolean |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx | Threads hostIsOnline prop down to DashboardSidebarCollapsedWorkspaceButton. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarCollapsedWorkspaceButton/DashboardSidebarCollapsedWorkspaceButton.tsx | Accepts and forwards hostIsOnline to DashboardSidebarWorkspaceIcon for the collapsed sidebar icon. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["useDashboardSidebarData\n(live query: hosts?.isOnline)"] --> B{hostType?}
B -->|"cloud\n(hostMachineId == null)"| C["hostIsOnline: null"]
B -->|"local-device\n(machineId matches)"| D["hostIsOnline: null"]
B -->|"remote-device\n(different machineId)"| E["hostIsOnline: boolean | null\n(from hosts.isOnline)"]
C & D & E --> F["DashboardSidebarWorkspace\n{ hostType, hostIsOnline }"]
F --> G["DashboardSidebarWorkspaceItem"]
G --> H["DashboardSidebarCollapsedWorkspaceButton"]
G --> I["DashboardSidebarExpandedWorkspaceRow"]
H --> J["DashboardSidebarWorkspaceIcon"]
I --> J
J --> K{Render icon}
K -->|"local-device"| L["● dot span"]
K -->|"remote-device && hostIsOnline === false"| M["LuCloudOff opacity-60"]
K -->|"remote-device (online/null) or cloud"| N["LuCloud"]
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin' in..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsx (1)
134-153: Optional: flatten the nested ternary in the tooltip copy.The double-nested ternary for both
<p>blocks is correct but a bit hard to scan. Consider extracting a small helper (e.g.getHostTooltipCopy(hostType, hostIsOnline)returning{ title, subtitle }) to improve readability and keep the two strings in sync as host types evolve.🤖 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/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsx` around lines 134 - 153, Refactor the nested ternaries inside DashboardSidebarExpandedWorkspaceRow (the two <p> blocks within TooltipContent) by creating a helper function getHostTooltipCopy(hostType, hostIsOnline) that returns an object like { title, subtitle }; replace the inline ternaries with title and subtitle lookups so both tooltip strings are produced in one place, keeping hostType and hostIsOnline logic centralized and easier to maintain as host types change.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceIcon/DashboardSidebarWorkspaceIcon.tsx (1)
33-63: Icon branching looks good; one minor accessibility nit.Logic correctly handles local-dot / remote-online-cloud / remote-offline-cloud-off / cloud → cloud. Since
renderHostIconis invoked only inside render and doesn't need a stable identity, inlining it (or wrapping withuseMemo) is a matter of taste — current form is fine.Minor a11y suggestion: the icons are purely decorative (tooltip in
DashboardSidebarExpandedWorkspaceRowprovides the label, and the collapsed button conveys semantics viaaria-label), so consider addingaria-hidden="true"to theLuCloud/LuCloudOffSVGs and the local-device dot<span>to avoid any stray accessible-name contribution from SVG<title>defaults in some react-icons setups.🤖 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/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceIcon/DashboardSidebarWorkspaceIcon.tsx` around lines 33 - 63, The decorative host icons returned by renderHostIcon (the local-device dot <span> and the LuCloud / LuCloudOff SVG components) can contribute stray accessible names; mark them explicitly decorative by adding aria-hidden="true" to the <span> used for the local device dot and to the LuCloud and LuCloudOff components inside renderHostIcon so screen readers ignore these visuals (the accessible label is provided elsewhere via DashboardSidebarExpandedWorkspaceRow tooltip / collapsed button aria-label).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsx`:
- Around line 134-153: Refactor the nested ternaries inside
DashboardSidebarExpandedWorkspaceRow (the two <p> blocks within TooltipContent)
by creating a helper function getHostTooltipCopy(hostType, hostIsOnline) that
returns an object like { title, subtitle }; replace the inline ternaries with
title and subtitle lookups so both tooltip strings are produced in one place,
keeping hostType and hostIsOnline logic centralized and easier to maintain as
host types change.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceIcon/DashboardSidebarWorkspaceIcon.tsx`:
- Around line 33-63: The decorative host icons returned by renderHostIcon (the
local-device dot <span> and the LuCloud / LuCloudOff SVG components) can
contribute stray accessible names; mark them explicitly decorative by adding
aria-hidden="true" to the <span> used for the local device dot and to the
LuCloud and LuCloudOff components inside renderHostIcon so screen readers ignore
these visuals (the accessible label is provided elsewhere via
DashboardSidebarExpandedWorkspaceRow tooltip / collapsed button aria-label).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 202507bf-3d67-4b56-8472-1390d3f9ca88
📒 Files selected for processing (6)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarCollapsedWorkspaceButton/DashboardSidebarCollapsedWorkspaceButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceIcon/DashboardSidebarWorkspaceIcon.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/types.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Kitenite
deleted the
please-help-me-handle-state-on-the-sidebar-icons-for-local-should-be-super-simple-icon-for-remote-sh
branch
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
v2Hosts.isOnline === false) renderCloudOffwith reduced opacityTest plan
Summary by CodeRabbit
Summary by cubic
Show the workspace’s host type and online state in the sidebar. Local shows a small dot; remote/cloud show a cloud; remote-device shows a dimmed cloud-off when offline, with clear tooltips.
hostIsOnlineto sidebar workspace data and props.LuCloudOffwhen a remote device is offline.Written for commit 49215a5. Summary will update on new commits.