Skip to content

Fix Manage Lane Tabs#380

Merged
arul28 merged 2 commits into
mainfrom
ade/fix-manage-lane-tabs-dfec9940
May 27, 2026
Merged

Fix Manage Lane Tabs#380
arul28 merged 2 commits into
mainfrom
ade/fix-manage-lane-tabs-dfec9940

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented May 27, 2026

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

ADE   Open in ADE  ·  ade/fix-manage-lane-tabs-dfec9940 branch  ·  PR #380

Summary by CodeRabbit

  • Tests

    • Added comprehensive test suite for ManageLaneDialog tab behavior and state persistence.
  • Refactor

    • Improved ManageLaneDialog tab selection stability and optimized delete risk checks for lanes.

Review Change Stack

Greptile Summary

This PR refactors ManageLaneDialog tab state management to fix spurious tab resets when the lane object refreshes (e.g., after a property like color changes). It does so by extracting singleLane?.id and singleLane?.laneType into primitive variables used as effect dependencies, and by memoizing defaultTab as a string rather than depending on the tabDefs array reference.

  • Tab reset fix: Replacing tabDefs (array reference) with defaultTab (string primitive) in the reset effect's dependency array prevents unnecessary resets when singleLane's identity doesn't change but its object reference does.
  • defaultTab memo: Centralizes the non-destructive tab preference logic (["appearance", "stack", "archive", "delete"]), eliminating duplication between the main reset effect and the fallback guard.
  • Tests: A new test suite verifies all three behaviors — single-lane default, batch-mode default, and stability across lane refreshes.

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

Filename Overview
apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx Tab state refactored to use primitive dependency values and a memoized defaultTab; logic is correct and the fallback guard also updated to use defaultTab.
apps/desktop/src/renderer/components/lanes/ManageLaneDialog.test.tsx New test suite covers single-lane default tab, batch-mode default tab, and stability across lane object refreshes; all scenarios correctly modelled.

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)"]
Loading

Reviews (2): Last reviewed commit: "ship: iteration 1 - address tab default ..." | Re-trigger Greptile

@vercel
Copy link
Copy Markdown

vercel Bot commented May 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 27, 2026 8:27pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

ManageLaneDialog refactors to derive stable lane identity values (singleLaneId, singleLaneType) used to drive tab visibility, initialize active-tab state on open/selection changes, and gate delete-related effects. New test suite validates initial tab selection and tab persistence across rerenders.

Changes

ManageLaneDialog Tab Stability and Effects

Layer / File(s) Summary
Tab visibility foundation
apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx
Component derives singleLaneId and singleLaneType once and uses them to rebuild tab definitions via memoized array with show conditions (delete, appearance, stack, archive), filtered to visible tabs. Memo dependency reduced to singleLaneType.
Active tab initialization
apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx
New effect resets transient dialog state on open or selection context changes, setting activeTab to the first available tab id (or defaulting to "archive"), replacing previous preferred-order logic.
Delete risk and progress effects
apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx
Pre-flight delete risk effect gates on open, singleLaneId, singleLaneType, clears risk for primary lanes, and fetches for single-lane case. Live delete progress effect subscribes only when singleLaneId is set and filters events by that id.
Tab behavior test suite
apps/desktop/src/renderer/components/lanes/ManageLaneDialog.test.tsx
New Vitest/JSDOM test suite with mock setup for window.ade lane functions. Two test cases verify: (1) dialog opens with first available tab labeled "Delete" selected, (2) changing to "Archive" tab persists across rerender when managedLane refreshes (e.g., color update) without resetting selection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

desktop

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix Manage Lane Tabs' directly corresponds to the main change: fixing tab behavior in the ManageLaneDialog component, specifically addressing tab reset logic and state preservation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/fix-manage-lane-tabs-dfec9940

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 27, 2026

@copilot review but do not make fixes

@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 27, 2026

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.

@arul28 arul28 force-pushed the ade/fix-manage-lane-tabs-dfec9940 branch from 5eaec57 to 16e7d8e Compare May 27, 2026 19:56
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 27, 2026

@copilot review but do not make fixes

Comment thread apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx Outdated
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (appearancestackarchivedelete), 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: getDeleteRisk and onDeleteEvent subscriptions no longer re-run on every lane snapshot refresh—only when id/type changes—matching ade-perf-lanes guidance 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.
Open in Web View Automation 

Sent by Cursor Automation: BUGBOT in Versic

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 27, 2026

@copilot review but do not make fixes

@arul28 arul28 merged commit 1c3cedb into main May 27, 2026
28 of 29 checks passed
@arul28 arul28 deleted the ade/fix-manage-lane-tabs-dfec9940 branch May 27, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant