fix(desktop): restore recently-viewed dropdown + tighten topbar chrome#4370
Conversation
…r chrome Recently-viewed dropdown was permanently disabled in v2: the path matcher only knew about /workspace/$id (v1) and the slug capture greedily consumed search params and nested subroutes, so /tasks/SUP-123?tab=… and /v2-workspace/$id never resolved to a known entity. Add a v2-workspace resource type, an automation resource type, strip search/hash before matching, anchor to the first path segment, and dedupe by canonical path. Filter cross-version workspace entries so clicking a v1 entry while in v2 mode no longer drops you on the cross-version mismatch screen. Re-show the dropdown trigger in both topbar and v2 sidebar header (was hardcoded to false in #4314 because it was always rendering as the disabled icon). Also: collapse the resource monitor trigger to an icon-only ghost button and float it to the far right of its row in both surfaces.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds v2-workspace and automation to recently-viewed parsing and dropdown; HistoryDropdown is always rendered. ResourceConsumption gains optional className and uses shared Button. DashboardSidebarHeader and TopBar layouts adjusted to reflect the new component positioning. ChangesHistory Dropdown Expansion and Navigation Refactoring
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 restores the recently-viewed history dropdown in both v1 and v2 modes by fixing path matching (adding
Confidence Score: 3/5Safe to merge for layout fixes; the history logic has one missing guard that can send v1-mode users to broken automation routes. The automation filter in HistoryDropdown lacks the isV2CloudEnabled check that workspace types use. A user who has visited automation pages in v2, then switches to v1, could see those entries in the dropdown and navigate to /automations/... — a route that only exists in v2 — likely landing on an error state. The rest of the changes look correct. HistoryDropdown.tsx — the automation entry filter at line 298 needs an isV2CloudEnabled guard consistent with the workspace type guards above it.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/components/HistoryDropdown/HistoryDropdown.tsx | Adds V2WorkspaceRow, AutomationRow components and live-query data for v2 workspaces and automations; automation filter is missing the isV2CloudEnabled guard that workspace types have, risking broken navigation in v1 mode. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/components/HistoryDropdown/hooks/useRecentlyViewed/useRecentlyViewed.ts | Adds v2-workspace, task, and automation path patterns with proper query/hash stripping and canonical-path deduplication; logic is clean and correctly ordered (newest-first). |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/NavigationControls.tsx | Removes the now-unnecessary showHistoryDropdown prop and always renders HistoryDropdown; straightforward cleanup. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx | Moves ResourceConsumption to the right-side cluster behind the !sidebarHostsChrome guard; placement logic is consistent with sidebarHostsChrome semantics. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx | Collapses trigger to icon-only ghost Button with absolute dot positioning; tooltip simplified to always-visible with inline memory reading. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx | Re-enables NavigationControls, adds ml-auto to ResourceConsumption and pr-2 to the drag row; minimal layout changes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[persistentHistory.getEntries] --> B[pathnameOf: strip ?/#]
B --> C{match route prefix}
C -->|/v2-workspace/:id| D[type: v2-workspace]
C -->|/workspace/:id| E[type: workspace]
C -->|/tasks/:id| F[type: task]
C -->|/automations/:id| G[type: automation]
C -->|no match| H[null - skip]
D & E & F & G --> I[dedup by canonical path]
I --> J[useRecentlyViewed returns entries]
J --> K{filteredEntries}
K -->|workspace| L{isV2CloudEnabled?}
L -->|yes| M[hide]
L -->|no| N[show if in workspaceData]
K -->|v2-workspace| O{isV2CloudEnabled?}
O -->|no| P[hide]
O -->|yes| Q[show if in v2WorkspaceData]
K -->|automation| R[show if in automationData - no v2 guard]
K -->|task| S[show if in taskData]
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/components/NavigationControls/components/HistoryDropdown/HistoryDropdown.tsx:298-300
**Automation entries not guarded by `isV2CloudEnabled`**
v1-mode and v2-mode workspace entries both have explicit `isV2CloudEnabled` guards that prevent cross-version navigation, but `automation` entries have no such guard. If a user has automation visit history from a prior v2 session and then switches to v1 mode, any matching automation record still present in the live DB will pass the `automationData.some(...)` filter and appear in the dropdown. Clicking one navigates to `/automations/${id}`, a route that presumably does not exist in v1 mode, likely landing the user on an error or blank screen.
Reviews (1): Last reviewed commit: "fix(desktop): restore recently-viewed dr..." | Re-trigger Greptile
| if (entry.type === "automation") { | ||
| return (automationData ?? []).some((a) => a.id === entry.entityId); | ||
| } |
There was a problem hiding this comment.
Automation entries not guarded by
isV2CloudEnabled
v1-mode and v2-mode workspace entries both have explicit isV2CloudEnabled guards that prevent cross-version navigation, but automation entries have no such guard. If a user has automation visit history from a prior v2 session and then switches to v1 mode, any matching automation record still present in the live DB will pass the automationData.some(...) filter and appear in the dropdown. Clicking one navigates to /automations/${id}, a route that presumably does not exist in v1 mode, likely landing the user on an error or blank screen.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/components/HistoryDropdown/HistoryDropdown.tsx
Line: 298-300
Comment:
**Automation entries not guarded by `isV2CloudEnabled`**
v1-mode and v2-mode workspace entries both have explicit `isV2CloudEnabled` guards that prevent cross-version navigation, but `automation` entries have no such guard. If a user has automation visit history from a prior v2 session and then switches to v1 mode, any matching automation record still present in the live DB will pass the `automationData.some(...)` filter and appear in the dropdown. Clicking one navigates to `/automations/${id}`, a route that presumably does not exist in v1 mode, likely landing the user on an error or blank screen.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/components/HistoryDropdown/HistoryDropdown.tsx`:
- Around line 298-300: The current branch in HistoryDropdown that returns
automation entries (when entry.type === "automation") should also ensure the app
is in V2 mode; change the condition to only allow automation history when a
V2-mode boolean (e.g., isV2Mode or isV2) is true and the automation still exists
(replace the existing early return with a check like: entry.type ===
"automation" && isV2Mode && (automationData ?? []).some(a => a.id ===
entry.entityId)); locate this logic in the HistoryDropdown component where
entry.type and automationData are used and gate it on the V2-mode flag available
on the component (prop, state, or context).
🪄 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: 90add3d8-2741-4dc4-9322-deb7178ff7f5
📒 Files selected for processing (6)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/NavigationControls.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/components/HistoryDropdown/HistoryDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/components/HistoryDropdown/hooks/useRecentlyViewed/useRecentlyViewed.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
/workspace/$id(v1) and the slug capture greedily ate search params and nested subroutes — so/v2-workspace/$idnever matched and/tasks/SUP-123?tab=…never resolved to a slug. Now matches/v2-workspace/$idand/automations/$id, strips?…/#…, anchors to the first path segment, and dedupes by the canonical path.CrossVersionMismatchState("Pick a workspace").showHistoryDropdown={false}because the regex was broken and it was always rendering as the disabled icon — fixing the underlying matcher means we can drop the prop).ml-auto+pr-2in the v2 sidebar header).Test plan
/tasks/SUP-XYZ?tab=…and/tasks/SUP-XYZ/issue/123— confirm the dropdown collapses both into a single/tasks/SUP-XYZentry.Summary by cubic
Restores the Recently Viewed dropdown in v2 by fixing route matching and adding support for v2 workspaces and automations. Also tightens topbar chrome by making the resource monitor an icon-only button aligned to the right.
Bug Fixes
/tasks/KEY?tab=xto/tasks/KEY).v2-workspaceandautomationresource types with correct labels/icons; include v2 project/branch and automation agent metadata.showHistoryDropdownprop.UI Improvements
ghostbutton with a status dot and clearer tooltip; floated to the far right in the topbar and v2 sidebar header.Written for commit 301bd5a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Style