Fix Manage Lane Tabs#380
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthrough
ChangesManageLaneDialog Tab Stability and Effects
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
🚥 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 |
|
@copilot review but do not make fixes |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
5eaec57 to
16e7d8e
Compare
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
PR Review
Scope: 2 file(s), +148 / −23
Verdict: Minor issues
This PR fixes Manage Lane dialog tab selection jumping back when the underlying LaneSummary object is refreshed (for example after an appearance color update) by keying effects off singleLaneId / singleLaneType instead of the full lane object. The core fix is sound and covered by a focused regression test.
🎨 UI/UX
[Low] Batch manage now defaults to Delete
File: apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx:L285-L286
Issue: On open, activeTab is set to tabDefs[0], which is always Delete because that tab is listed first. The removed preferredOrder logic previously selected the first non-delete tab (appearance → stack → archive → delete), so multi-lane Manage N Open Lanes… flows (batch mode: only Delete + Archive tabs) used to land on Archive, not Delete.
Fix: When resetting on open, keep the stable singleLaneId / singleLaneType dependencies, but restore a non-destructive-first pick (e.g. the old preferredOrder.find(...)) instead of tabDefs[0], or only use tabDefs[0] for single-lane opens if Delete-first is intentional there.
Notes
- Good fix:
getDeleteRiskandonDeleteEventsubscriptions no longer re-run on every lane snapshot refresh—only when id/type changes—matchingade-perf-lanesguidance for cheap appearance updates. - Single-lane default tab is now Delete (asserted in the new test); that reverses the prior “non-destructive tab first” comment—fine if deliberate for manage/deeplink flows.
- Delete/archive still require confirmation; this finding is about default tab focus, not missing guards.
Sent by Cursor Automation: BUGBOT in Versic
|
@copilot review but do not make fixes |


Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Tests
Refactor
Greptile Summary
This PR refactors
ManageLaneDialogtab state management to fix spurious tab resets when the lane object refreshes (e.g., after a property likecolorchanges). It does so by extractingsingleLane?.idandsingleLane?.laneTypeinto primitive variables used as effect dependencies, and by memoizingdefaultTabas a string rather than depending on thetabDefsarray reference.tabDefs(array reference) withdefaultTab(string primitive) in the reset effect's dependency array prevents unnecessary resets whensingleLane's identity doesn't change but its object reference does.defaultTabmemo: Centralizes the non-destructive tab preference logic (["appearance", "stack", "archive", "delete"]), eliminating duplication between the main reset effect and the fallback guard.Confidence Score: 5/5
Safe to merge — the refactor correctly uses primitive dependency values to eliminate spurious tab resets, and the new test suite validates all key tab-selection scenarios.
The change is narrowly scoped to dependency-array hygiene and memoization: swapping the tabDefs array reference for the defaultTab string primitive is the right fix for the refresh-triggered reset bug. The defaultTab memo faithfully reproduces the original preferred-order logic, and the fallback guard was updated consistently. All three affected behaviors are now covered by tests.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["Dialog opens or lane changes"] --> B{open?} B -- No --> C["Clear deleteRisk and deleteProgress"] B -- Yes --> D["setActiveTab(defaultTab)"] subgraph Memos E["tabDefs memo - depends on singleLaneType"] --> F["Filtered list: delete, appearance, stack, archive"] G["defaultTab memo - depends on tabDefs"] --> H["preferredOrder: appearance, stack, archive, delete"] H --> I["Return first match found in tabDefs"] end D -.-> G J["Fallback guard effect"] --> K{activeTab in tabDefs?} K -- Yes --> L["no-op"] K -- No --> M["setActiveTab(defaultTab)"]Reviews (2): Last reviewed commit: "ship: iteration 1 - address tab default ..." | Re-trigger Greptile