fix: UX when installing packages (#222)

* ensure skip install is only shown when relevant 
* ensure delete env message is modal

fixes #220

Author: karthiknadig

src/api.ts

@@ -721,6 +721,11 @@ export interface PackageInstallOptions {
      * Upgrade the packages if it is already installed.
      */
     upgrade?: boolean;
+
+    /**
+     * Show option to skip package installation
+     */
+    showSkipOption?: boolean;
 }
 
 export interface PythonProcess {

src/common/window.apis.ts

@@ -7,6 +7,8 @@ import {
     InputBox,
     InputBoxOptions,
     LogOutputChannel,
+    MessageItem,
+    MessageOptions,
     OpenDialogOptions,
     OutputChannel,
     Progress,
@@ -284,7 +286,19 @@ export async function showInputBoxWithButtons(
     }
 }
 
-export function showWarningMessage(message: string, ...items: string[]): Thenable<string | undefined> {
+export function showWarningMessage<T extends string>(message: string, ...items: T[]): Thenable<T | undefined>;
+export function showWarningMessage<T extends string>(
+    message: string,
+    options: MessageOptions,
+    ...items: T[]
+): Thenable<T | undefined>;
+export function showWarningMessage<T extends MessageItem>(message: string, ...items: T[]): Thenable<T | undefined>;
+export function showWarningMessage<T extends MessageItem>(
+    message: string,
+    options: MessageOptions,
+    ...items: T[]
+): Thenable<T | undefined>;
+export function showWarningMessage(message: string, ...items: any[]): Thenable<string | undefined> {
     return window.showWarningMessage(message, ...items);
 }
 

src/features/envCommands.ts

@@ -188,7 +188,7 @@ export async function handlePackagesCommand(
 
     try {
         if (action === Common.install) {
-            await packageManager.install(environment);
+            await packageManager.install(environment, undefined, { showSkipOption: false });
         } else if (action === Common.uninstall) {
             await packageManager.uninstall(environment);
         }

src/managers/builtin/pipManager.ts

@@ -54,7 +54,7 @@ export class PipPackageManager implements PackageManager, Disposable {
 
         if (selected.length === 0) {
             const projects = this.venv.getProjectsByEnvironment(environment);
-            selected = (await getWorkspacePackagesToInstall(this.api, projects)) ?? [];
+            selected = (await getWorkspacePackagesToInstall(this.api, options, projects)) ?? [];
         }
 
         if (selected.length === 0) {

src/managers/builtin/pipUtils.ts

@@ -4,7 +4,7 @@ import * as tomljs from '@iarna/toml';
 import { LogOutputChannel, ProgressLocation, QuickInputButtons, Uri } from 'vscode';
 import { showQuickPickWithButtons, withProgress } from '../../common/window.apis';
 import { PackageManagement, Pickers, VenvManagerStrings } from '../../common/localize';
-import { PythonEnvironmentApi, PythonProject } from '../../api';
+import { PackageInstallOptions, PythonEnvironmentApi, PythonProject } from '../../api';
 import { findFiles } from '../../common/workspace.apis';
 import { EXTENSION_ROOT_DIR } from '../../common/constants';
 import { Installable, selectFromCommonPackagesToInstall, selectFromInstallableToInstall } from '../common/pickers';
@@ -75,6 +75,7 @@ async function getCommonPackages(): Promise<Installable[]> {
 async function selectWorkspaceOrCommon(
     installable: Installable[],
     common: Installable[],
+    showSkipOption: boolean,
 ): Promise<string[] | undefined> {
     if (installable.length === 0 && common.length === 0) {
         return undefined;
@@ -95,19 +96,20 @@ async function selectWorkspaceOrCommon(
         });
     }
 
-    if (items.length > 0) {
+    if (showSkipOption && items.length > 0) {
         items.push({ label: PackageManagement.skipPackageInstallation });
-    } else {
-        return undefined;
     }
 
-    const selected = await showQuickPickWithButtons(items, {
-        placeHolder: Pickers.Packages.selectOption,
-        ignoreFocusOut: true,
-        showBackButton: true,
-        matchOnDescription: false,
-        matchOnDetail: false,
-    });
+    const selected =
+        items.length === 1
+            ? items[0]
+            : await showQuickPickWithButtons(items, {
+                  placeHolder: Pickers.Packages.selectOption,
+                  ignoreFocusOut: true,
+                  showBackButton: true,
+                  matchOnDescription: false,
+                  matchOnDetail: false,
+              });
 
     if (selected && !Array.isArray(selected)) {
         try {
@@ -122,7 +124,7 @@ async function selectWorkspaceOrCommon(
             // eslint-disable-next-line @typescript-eslint/no-explicit-any
         } catch (ex: any) {
             if (ex === QuickInputButtons.Back) {
-                return selectWorkspaceOrCommon(installable, common);
+                return selectWorkspaceOrCommon(installable, common, showSkipOption);
             }
         }
     }
@@ -131,11 +133,12 @@ async function selectWorkspaceOrCommon(
 
 export async function getWorkspacePackagesToInstall(
     api: PythonEnvironmentApi,
+    options?: PackageInstallOptions,
     project?: PythonProject[],
 ): Promise<string[] | undefined> {
     const installable = (await getProjectInstallable(api, project)) ?? [];
     const common = await getCommonPackages();
-    return selectWorkspaceOrCommon(installable, common);
+    return selectWorkspaceOrCommon(installable, common, !!options?.showSkipOption);
 }
 
 export async function getProjectInstallable(

src/managers/builtin/venvUtils.ts

@@ -400,7 +400,11 @@ export async function createPythonVenv(
         os.platform() === 'win32' ? path.join(envPath, 'Scripts', 'python.exe') : path.join(envPath, 'bin', 'python');
 
     const project = api.getPythonProject(venvRoot);
-    const packages = await getWorkspacePackagesToInstall(api, project ? [project] : undefined);
+    const packages = await getWorkspacePackagesToInstall(
+        api,
+        { showSkipOption: true },
+        project ? [project] : undefined,
+    );
 
     return await withProgress(
         {
@@ -455,10 +459,13 @@ export async function removeVenv(environment: PythonEnvironment, log: LogOutputC
 
     const confirm = await showWarningMessage(
         l10n.t('Are you sure you want to remove {0}?', envPath),
-        Common.yes,
-        Common.no,
+        {
+            modal: true,
+        },
+        { title: Common.yes },
+        { title: Common.no, isCloseAffordance: true },
     );
-    if (confirm === Common.yes) {
+    if (confirm?.title === Common.yes) {
         await withProgress(
             {
                 location: ProgressLocation.Notification,

src/managers/conda/condaPackageManager.ts

@@ -56,7 +56,7 @@ export class CondaPackageManager implements PackageManager, Disposable {
         let selected: string[] = packages ?? [];
 
         if (selected.length === 0) {
-            selected = (await getCommonCondaPackagesToInstall()) ?? [];
+            selected = (await getCommonCondaPackagesToInstall(options)) ?? [];
         }
 
         if (selected.length === 0) {

src/managers/conda/condaUtils.ts

@@ -761,7 +761,10 @@ async function getCommonPackages(): Promise<Installable[]> {
     }
 }
 
-async function selectCommonPackagesOrSkip(common: Installable[]): Promise<string[] | undefined> {
+async function selectCommonPackagesOrSkip(
+    common: Installable[],
+    showSkipOption: boolean,
+): Promise<string[] | undefined> {
     if (common.length === 0) {
         return undefined;
     }
@@ -774,19 +777,20 @@ async function selectCommonPackagesOrSkip(common: Installable[]): Promise<string
         });
     }
 
-    if (items.length > 0) {
+    if (showSkipOption && items.length > 0) {
         items.push({ label: PackageManagement.skipPackageInstallation });
-    } else {
-        return undefined;
     }
 
-    const selected = await showQuickPickWithButtons(items, {
-        placeHolder: Pickers.Packages.selectOption,
-        ignoreFocusOut: true,
-        showBackButton: true,
-        matchOnDescription: false,
-        matchOnDetail: false,
-    });
+    const selected =
+        items.length === 1
+            ? items[0]
+            : await showQuickPickWithButtons(items, {
+                  placeHolder: Pickers.Packages.selectOption,
+                  ignoreFocusOut: true,
+                  showBackButton: true,
+                  matchOnDescription: false,
+                  matchOnDetail: false,
+              });
 
     if (selected && !Array.isArray(selected)) {
         try {
@@ -799,15 +803,15 @@ async function selectCommonPackagesOrSkip(common: Installable[]): Promise<string
             // eslint-disable-next-line @typescript-eslint/no-explicit-any
         } catch (ex: any) {
             if (ex === QuickInputButtons.Back) {
-                return selectCommonPackagesOrSkip(common);
+                return selectCommonPackagesOrSkip(common, showSkipOption);
             }
         }
     }
     return undefined;
 }
 
-export async function getCommonCondaPackagesToInstall(): Promise<string[] | undefined> {
+export async function getCommonCondaPackagesToInstall(options?: PackageInstallOptions): Promise<string[] | undefined> {
     const common = await getCommonPackages();
-    const selected = await selectCommonPackagesOrSkip(common);
+    const selected = await selectCommonPackagesOrSkip(common, !!options?.showSkipOption);
     return selected;
 }