Improving VS Code account loading states for Connection Dialog and Add Firewall Dialog (#21861)

* Fixing spinner alignment

* removing hardcode

* adding caching to Entra account load logic

* cleanup

* fixed up add firewall dialog

* Loc

* PR cleanup

* Adding sign-in button in banner for old Entra account system

* Updating test

Author: Benjin

extensions/mssql/l10n/bundle.l10n.json

@@ -62,6 +62,7 @@
     "Warning": "Warning",
     "Sign In": "Sign In",
     "Loading": "Loading",
+    "Loading...": "Loading...",
     "General": "General",
     "Previous": "Previous",
     "OK": "OK",
@@ -209,6 +210,7 @@
         "comment": ["{0} is the IP address of the client"]
     },
     "Add my subnet IP range": "Add my subnet IP range",
+    "IP address range": "IP address range",
     "From/Label for the start IP address in the firewall rule IP range": {
         "message": "From",
         "comment": ["Label for the start IP address in the firewall rule IP range"]
@@ -885,7 +887,6 @@
     "Custom GraphQL Type": "Custom GraphQL Type",
     "Optional - Override default GraphQL type name": "Optional - Override default GraphQL type name",
     "Source Table": "Source Table",
-    "Loading...": "Loading...",
     "Initializing DAB configuration...": "Initializing DAB configuration...",
     "No entities found": "No entities found",
     "Toggle all entities in {0}/{0} is the schema name": {
@@ -2286,6 +2287,10 @@
         "message": "No SQL resource is configured for the current cloud '{0}'. Please update your Azure account settings.",
         "comment": ["{0} is the display name of the current cloud"]
     },
+    "Azure account '{0}' was not found. Sign in with the correct account or select a different one./{0} is the display name or ID of the Azure account that was not found": {
+        "message": "Azure account '{0}' was not found. Sign in with the correct account or select a different one.",
+        "comment": ["{0} is the display name or ID of the Azure account that was not found"]
+    },
     "Error signing into Azure: {0}/{0} is the error message": {
         "message": "Error signing into Azure: {0}",
         "comment": ["{0} is the error message"]

extensions/mssql/src/connectionconfig/connectionDialogWebviewController.ts

@@ -1074,6 +1074,15 @@ export class Azure {
         });
     }
 
+    public static accountNotFound(accountDisplayName: string): string {
+        return l10n.t({
+            message:
+                "Azure account '{0}' was not found. Sign in with the correct account or select a different one.",
+            args: [accountDisplayName],
+            comment: ["{0} is the display name or ID of the Azure account that was not found"],
+        });
+    }
+
     public static errorSigningIntoAzure(errorMessage: string): string {
         return l10n.t({
             message: "Error signing into Azure: {0}",

extensions/mssql/src/constants/locConstants.ts

@@ -47,6 +47,7 @@ export class AddFirewallRuleWebviewController extends WebviewPanelController<
                 message: initializationProps.errorMessage,
                 clientIp: "",
                 isSignedIn: false,
+                loadingAccounts: true,
                 accounts: [],
                 tenants: {},
                 addFirewallRuleStatus: ApiStatus.NotStarted,
@@ -90,14 +91,8 @@ export class AddFirewallRuleWebviewController extends WebviewPanelController<
      * Initialize the controller
      */
     private async initializeDialog(errorMessage: string): Promise<void> {
-        // Check if user is signed into Azure, and populate the dialog if they are
-        this.state.isSignedIn = await VsCodeAzureHelper.isSignedIn();
+        const accountLoadingPromise = this.loadAzureAccounts(); // Load accounts and extract client IP in parallel
 
-        if (this.state.isSignedIn) {
-            await populateAzureAccountInfo(this.state, false /* forceSignInPrompt */);
-        }
-
-        // Extract the client IP address from the error message
         const handleFirewallErrorResult = await this.firewallService.handleFirewallRule(
             errorFirewallRule,
             errorMessage,
@@ -118,6 +113,21 @@ export class AddFirewallRuleWebviewController extends WebviewPanelController<
         }
 
         this.state.clientIp = handleFirewallErrorResult.ipAddress;
+
+        await accountLoadingPromise;
+    }
+
+    private async loadAzureAccounts(): Promise<void> {
+        try {
+            this.state.isSignedIn = await VsCodeAzureHelper.isSignedIn();
+
+            if (this.state.isSignedIn) {
+                await populateAzureAccountInfo(this.state, false /* forceSignInPrompt */);
+            }
+        } finally {
+            this.state.loadingAccounts = false;
+            this.updateState();
+        }
     }
 
     /**
@@ -165,7 +175,14 @@ export class AddFirewallRuleWebviewController extends WebviewPanelController<
         });
 
         this.registerReducer("signIntoAzure", async (state) => {
-            await populateAzureAccountInfo(state, true /* forceSignInPrompt */);
+            state.loadingAccounts = true;
+            this.updateState(state);
+
+            try {
+                await populateAzureAccountInfo(state, true /* forceSignInPrompt */);
+            } finally {
+                state.loadingAccounts = false;
+            }
 
             return state;
         });
@@ -181,11 +198,7 @@ export async function populateAzureAccountInfo(
     try {
         auth = await VsCodeAzureHelper.signIn(forceSignInPrompt);
     } catch (error) {
-        this.logger.error(`Error signing into Azure: ${getErrorMessage(error)}`);
-        this.vscodeWrapper.showErrorMessage(
-            Loc.Azure.errorSigningIntoAzure(getErrorMessage(error)),
-        );
-
+        console.error(`Error signing into Azure: ${getErrorMessage(error)}`);
         return;
     }
 

extensions/mssql/src/controllers/addFirewallRuleWebviewController.ts

@@ -762,7 +762,7 @@ export class ObjectExplorerService {
                 );
                 if (choice === LocalizedConstants.ObjectExplorer.FailedOEConnectionErrorSignIn) {
                     try {
-                        await VsCodeAzureHelper.signIn(true); // User chose to sign in to the missing account; try again.
+                        await VsCodeAzureHelper.signIn(true /* forceSignInPrompt */); // User chose to sign in to the missing account; try again.
                         return await prepareConnectionProfile();
                     } catch (retryError) {
                         this._logger.error(

extensions/mssql/src/objectExplorer/objectExplorerService.ts

@@ -8,10 +8,17 @@ export class Deferred<T> {
     promise: Promise<T>;
     resolve: (value?: T | PromiseLike<T>) => void;
     reject: (reason?: any) => void;
+    isCompleted = false;
     constructor() {
         this.promise = new Promise<T>((resolve, reject) => {
-            this.resolve = resolve;
-            this.reject = reject;
+            this.resolve = (value?: T | PromiseLike<T>) => {
+                this.isCompleted = true;
+                resolve(value);
+            };
+            this.reject = (reason?: any) => {
+                this.isCompleted = true;
+                reject(reason);
+            };
         });
     }
 

extensions/mssql/src/protocol.ts

@@ -15,6 +15,8 @@ export interface AddFirewallRuleState {
     message: string;
     clientIp: string;
     isSignedIn: boolean;
+    /** Whether accounts and tenants are currently being loaded */
+    loadingAccounts: boolean;
     accounts: IMssqlAzureAccount[];
     /** Maps from account ID to list of tenants */
     tenants: Record<string, IMssqlAzureTenant[]>;

extensions/mssql/src/sharedInterfaces/addFirewallRule.ts

@@ -88,6 +88,11 @@ export interface FormItemSpec<
      * Whether the form item is an advanced option
      */
     isAdvancedOption?: boolean;
+
+    /**
+     * Whether the form item is currently loading its data (e.g. dropdown options)
+     */
+    loading?: boolean;
 }
 
 export interface FormItemValidationState {

extensions/mssql/src/sharedInterfaces/form.ts

@@ -12,8 +12,10 @@ import {
     FieldProps,
     InfoLabel,
     Input,
+    Label,
     LabelProps,
     Option,
+    Spinner,
     Text,
     Textarea,
     makeStyles,
@@ -73,6 +75,11 @@ export const useFormStyles = makeStyles({
         display: "flex",
         marginLeft: "auto",
     },
+    labelDecoration: {
+        display: "inline-flex",
+        alignItems: "center",
+        columnGap: "0px",
+    },
 });
 
 export const FormInput = <
@@ -218,29 +225,29 @@ export const FormField = <
                 }
                 required={component.required}
                 // @ts-ignore there's a bug in the typings somewhere, so ignoring this line to avoid angering type-checker
-                label={
+                label={{
                     // The html here shouldn't need to be sanitized, and should be safe
                     // because it's only ever set by forms internal to the extension
-                    component.tooltip ? (
-                        {
-                            children: (_: unknown, slotProps: LabelProps) => (
-                                <InfoLabel {...slotProps} info={component.tooltip}>
-                                    <span
-                                        dangerouslySetInnerHTML={{
-                                            __html: component.label,
-                                        }}
-                                    />
-                                </InfoLabel>
-                            ),
-                        }
-                    ) : (
-                        <span
-                            dangerouslySetInnerHTML={{
-                                __html: component.label,
-                            }}
-                        />
-                    )
-                }
+                    children: (_: unknown, slotProps: LabelProps) => {
+                        const labelContent = (
+                            <span
+                                dangerouslySetInnerHTML={{
+                                    __html: component.label,
+                                }}
+                            />
+                        );
+                        const LabelComponent = component.tooltip ? InfoLabel : Label;
+                        const tooltipProps = component.tooltip ? { info: component.tooltip } : {};
+                        return (
+                            <span className={formStyles.labelDecoration}>
+                                <LabelComponent {...slotProps} {...tooltipProps}>
+                                    {labelContent}
+                                </LabelComponent>
+                                {component.loading && <Spinner size="extra-tiny" />}
+                            </span>
+                        );
+                    },
+                }}
                 {...props}
                 style={{ color: tokens.colorNeutralForeground1 }}>
                 {generateFormComponent<TForm, TState, TFormItemSpec, TContext>(

extensions/mssql/src/webviews/common/forms/form.component.tsx

@@ -67,6 +67,7 @@ export class LocConstants {
             warning: l10n.t("Warning"),
             signIn: l10n.t("Sign In"),
             loading: l10n.t("Loading"),
+            loadingWithEllipsis: l10n.t("Loading..."),
             general: l10n.t("General"),
             previous: l10n.t("Previous"),
             ok: l10n.t("OK"),
@@ -300,6 +301,7 @@ export class LocConstants {
                     comment: ["{0} is the IP address of the client"],
                 }),
             addMySubnetRange: l10n.t("Add my subnet IP range"),
+            ipAddressRange: l10n.t("IP address range"),
             fromLabel: l10n.t({
                 message: "From",
                 comment: ["Label for the start IP address in the firewall rule IP range"],