gator icon indicating copy to clipboard operation
gator copied to clipboard

Move confirmation dialogs from packageActions to CondaPkgPanel

Open RRosio opened this issue 1 month ago • 0 comments

Problem

Confirmation dialogs are currently shown inside packageActions.ts functions (updateAllPackages() and applyPackageChanges()), which causes several issues:

  1. State modified before confirmation: Component state (isApplyingChanges, filters) is set before the user sees the dialog
  2. Unnecessary cleanup on cancel: _updatePackages() runs in finally blocks even when users cancel
  3. Poor separation of concerns: UI dialogs should be in UI components, not action modules

Currently

// In CondaPkgPanel.tsx
async handleUpdateAll(): Promise<void> {
  this.setState({ isApplyingChanges: true }); //  X State changed before confirmation
  await updateAllPackages(this._model);       // Shows dialog internally
  this._updatePackages();                      // X Runs even if cancelled
}

Proposed Solution

Move dialogs to component level:

  1. In CondaPkgPanel.tsx: Show confirmation dialog first
   async handleUpdateAll(): Promise<void> {
     const confirmation = await showDialog({
       title: 'Update all',
       body: 'Please confirm...'
     });
     
     if (!confirmation.button.accept) {
       return; // Exit early, no state changes
     }
     
     try {
       this.setState({ isApplyingChanges: true }); // Only after confirmation
       await updateAllPackages(this._model);
     } finally {
       this.setState({ isApplyingChanges: false });
       this._updatePackages(); // Only runs if confirmed
     }
   }
  1. In packageActions.ts: Remove dialogs, make functions pure actions
   export async function updateAllPackages(
     pkgModel: Conda.IPackageManager,
     environment?: string
   ): Promise<void> {
     // Remove showDialog() - caller handles confirmation
     // Just do the work
     await pkgModel.update(['--all'], theEnvironment);
   }

This provides the following fixes:

  • No state changes until user confirms
  • No unnecessary _updatePackages() calls on cancel
  • Better separation: UI in components, logic in actions

RRosio avatar Nov 15 '25 02:11 RRosio