fix(desktop): drop red failed-host indicator from v2 ports dropdown#3787
fix(desktop): drop red failed-host indicator from v2 ports dropdown#3787
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe sidebar port list component removes consumption of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 removes the red
Confidence Score: 4/5Safe to merge once the all-hosts-fail silent-disappearance edge case is confirmed intentional by the team. The only finding is P2: a behavioral edge case where the Ports section fully disappears when all hosts are unreachable (previously it stayed visible with an error indicator). All imports are correctly cleaned up and the happy-path rendering is simplified correctly. If the silent-disappearance is an accepted product decision, this is a clean merge. DashboardSidebarPortsList.tsx — early-return guard change on line 13
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarPortsList/DashboardSidebarPortsList.tsx | Removes the red LuCircleAlert error indicator, its tooltip, and associated portLoadErrors/failedHostCount logic. One edge case: the early-return guard now hides the entire section when all hosts fail (totalPortCount===0 + errors), where previously it remained visible with an error state. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DashboardSidebarPortsList renders] --> B{totalPortCount === 0?}
B -- Yes --> C[return null — section hidden]
B -- No --> D[Render Ports header + badge]
D --> E[Show port-count badge ml-auto]
E --> F{isCollapsed?}
F -- No --> G[Render DashboardSidebarPortGroup list]
F -- Yes --> H[Collapse — no list rendered]
subgraph Before this PR
B2{totalPortCount === 0 AND failedHostCount === 0?}
B2 -- Yes --> C2[return null]
B2 -- No, errors exist --> ERR[Render section + red LuCircleAlert tooltip]
end
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarPortsList/DashboardSidebarPortsList.tsx
Line: 13-15
Comment:
**Silent disappearance when all hosts fail**
The old guard was `totalPortCount === 0 && failedHostCount === 0`, which kept the Ports section visible (with the alert icon) when all hosts errored but returned 0 ports. The new guard `totalPortCount === 0` now returns `null` for that same scenario — the entire section silently disappears instead of staying visible. The test plan covers "at least one reachable host" but not the all-hosts-fail case; users in that state get no indication that Ports exist but couldn't load.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(desktop): drop red failed-host indic..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (totalPortCount === 0) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The old guard was totalPortCount === 0 && failedHostCount === 0, which kept the Ports section visible (with the alert icon) when all hosts errored but returned 0 ports. The new guard totalPortCount === 0 now returns null for that same scenario — the entire section silently disappears instead of staying visible. The test plan covers "at least one reachable host" but not the all-hosts-fail case; users in that state get no indication that Ports exist but couldn't load.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarPortsList/DashboardSidebarPortsList.tsx
Line: 13-15
Comment:
**Silent disappearance when all hosts fail**
The old guard was `totalPortCount === 0 && failedHostCount === 0`, which kept the Ports section visible (with the alert icon) when all hosts errored but returned 0 ports. The new guard `totalPortCount === 0` now returns `null` for that same scenario — the entire section silently disappears instead of staying visible. The test plan covers "at least one reachable host" but not the all-hosts-fail case; users in that state get no indication that Ports exist but couldn't load.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarPortsList/DashboardSidebarPortsList.tsx (1)
10-11: Consider pruning the unusedportLoadErrorsfrom the hook's return type.This component is the only consumer of
useDashboardSidebarPortsData(), yet it only destructurestotalPortCountandworkspacePortGroups. The hook still computes and returnsportLoadErrors(lines 172–191 inuseDashboardSidebarPortsData.ts), but it is never used anywhere in the codebase. Removing it from the hook's return type and the computation will eliminate dead code.🤖 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/DashboardSidebarPortsList/DashboardSidebarPortsList.tsx` around lines 10 - 11, The hook useDashboardSidebarPortsData currently computes and returns portLoadErrors but the consumer (destructuring totalPortCount and workspacePortGroups) never uses it; remove the dead portLoadErrors work by deleting its computation (the logic generating portLoadErrors) and remove it from the hook's return value so the hook only returns totalPortCount and workspacePortGroups; update the hook implementation and any types to drop portLoadErrors and ensure no other code references portLoadErrors.
🤖 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/DashboardSidebarPortsList/DashboardSidebarPortsList.tsx`:
- Around line 10-11: The hook useDashboardSidebarPortsData currently computes
and returns portLoadErrors but the consumer (destructuring totalPortCount and
workspacePortGroups) never uses it; remove the dead portLoadErrors work by
deleting its computation (the logic generating portLoadErrors) and remove it
from the hook's return value so the hook only returns totalPortCount and
workspacePortGroups; update the hook implementation and any types to drop
portLoadErrors and ensure no other code references portLoadErrors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f265a622-003c-4344-8fa7-928aaa3aca45
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarPortsList/DashboardSidebarPortsList.tsx
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
LuCircleAlertindicator (and its tooltip) from the v2 dashboard sidebar Ports headerfailedHostCount/portLoadErrorsbranching inDashboardSidebarPortsListTest plan
bun run typecheckandbun run lintpassSummary by cubic
Removed the red failed-host
LuCircleAlertindicator and its tooltip from the v2 dashboard sidebar Ports header. Added a subtleLuCircleHelpbutton with a tooltip that opens the Ports docs in a new tab, kept the port total right-aligned, and removed unused error-handling code and imports.Written for commit 207ccce. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
New Features / UX