fix(ui): stop the add-provider modal from shifting while it loads (#13179)

ui/components/providers/workflow/forms/connect-account-form.test.tsx

@@ -1,4 +1,4 @@
-import { render, screen, waitFor } from "@testing-library/react";
+import { act, render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
@@ -90,15 +90,16 @@ describe("provider account aliases", () => {
 });
 
 describe("Registry provider source tabs", () => {
-  it("explains Registry setup without claiming a connection failure", async () => {
+  it("explains Registry setup in the Registry tab without claiming a connection failure", async () => {
     // Given
-    vi.stubEnv("UI_CLOUD_ENABLED", "true");
+    const user = userEvent.setup();
     getInstalledRegistryProviderOptions.mockResolvedValue({
       status: "onboarding",
     });
+    render(<ConnectAccountForm onSuccess={vi.fn()} />);
 
     // When
-    render(<ConnectAccountForm onSuccess={vi.fn()} />);
+    await user.click(screen.getByRole("tab", { name: "Registry" }));
 
     // Then
     expect(
@@ -110,28 +111,42 @@ describe("Registry provider source tabs", () => {
     expect(
       screen.queryByText("Registry providers could not be loaded"),
     ).not.toBeInTheDocument();
+
+    // When / Then: the built-in list never gets a banner pushed above it.
+    await user.click(screen.getByRole("tab", { name: "All providers" }));
+    expect(
+      screen.queryByText("Registry is not connected yet"),
+    ).not.toBeInTheDocument();
   });
 
-  it("offers the Registry paid plan upgrade while keeping built-in providers usable", async () => {
+  it("offers the Registry paid plan upgrade in the Registry tab while keeping built-in providers usable", async () => {
     // Given
-    vi.stubEnv("UI_CLOUD_ENABLED", "true");
     getInstalledRegistryProviderOptions.mockResolvedValue({
       status: "subscription_required",
     });
     const user = userEvent.setup();
     useCloudUpgradeStore.getState().closeCloudUpgrade();
+    render(<ConnectAccountForm onSuccess={vi.fn()} />);
 
     // When
-    render(<ConnectAccountForm onSuccess={vi.fn()} />);
+    await user.click(screen.getByRole("tab", { name: "Registry" }));
     await user.click(
       await screen.findByRole("button", { name: "Unlock Registry" }),
     );
 
     // Then
     expect(useCloudUpgradeStore.getState().activeFeature).toBe("registry");
+
+    // When
+    await user.click(screen.getByRole("tab", { name: "All providers" }));
+
+    // Then
     expect(
       screen.getByRole("option", { name: /Amazon Web Services/ }),
     ).toBeVisible();
+    expect(
+      screen.queryByRole("button", { name: "Unlock Registry" }),
+    ).not.toBeInTheDocument();
     expect(
       screen.queryByText("Registry providers could not be loaded"),
     ).not.toBeInTheDocument();
@@ -144,12 +159,64 @@ describe("Registry provider source tabs", () => {
   beforeEach(() => {
     vi.clearAllMocks();
     vi.stubEnv("UI_CLOUD_ENABLED", "true");
+    vi.stubEnv("UI_REGISTRY_ENABLED", "true");
   });
 
   afterEach(() => {
     vi.unstubAllEnvs();
   });
 
+  it("shows both source tabs before discovery settles and loads Registry providers in place", async () => {
+    // Given: Registry discovery is still in flight.
+    const user = userEvent.setup();
+    let settleDiscovery: (result: {
+      status: "ready";
+      options: { type: string; label: string }[];
+    }) => void = () => {};
+    getInstalledRegistryProviderOptions.mockReturnValue(
+      new Promise((resolve) => {
+        settleDiscovery = resolve;
+      }),
+    );
+
+    // When
+    render(<ConnectAccountForm onSuccess={vi.fn()} />);
+
+    // Then: the tabs are there from the first render.
+    expect(screen.getByRole("tab", { name: "All providers" })).toHaveAttribute(
+      "aria-selected",
+      "true",
+    );
+    expect(screen.getByRole("tab", { name: "Registry" })).toBeVisible();
+
+    // When
+    await user.click(screen.getByRole("tab", { name: "Registry" }));
+
+    // Then
+    expect(
+      screen.getByRole("status", { name: "Loading Registry providers" }),
+    ).toBeVisible();
+    expect(
+      screen.queryByText("No Registry providers available."),
+    ).not.toBeInTheDocument();
+
+    // When
+    await act(async () => {
+      settleDiscovery({
+        status: "ready",
+        options: [{ type: "acme", label: "Acme Cloud" }],
+      });
+    });
+
+    // Then
+    expect(
+      screen.getByRole("option", { name: /Acme Cloud Registry/ }),
+    ).toBeVisible();
+    expect(
+      screen.queryByRole("status", { name: "Loading Registry providers" }),
+    ).not.toBeInTheDocument();
+  });
+
   it("never mentions Registry outside Cloud, even when discovery would fail", async () => {
     // Given
     vi.stubEnv("UI_CLOUD_ENABLED", "false");
@@ -171,7 +238,22 @@ describe("Registry provider source tabs", () => {
     ).not.toBeInTheDocument();
   });
 
-  it("hides the Registry tab when discovery denies access (Local or flag off)", async () => {
+  it("hides the source tabs without asking discovery when the deployment has Registry off", () => {
+    // Given
+    vi.stubEnv("UI_REGISTRY_ENABLED", "false");
+
+    // When
+    render(<ConnectAccountForm onSuccess={vi.fn()} />);
+
+    // Then
+    expect(getInstalledRegistryProviderOptions).not.toHaveBeenCalled();
+    expect(screen.queryByRole("tablist")).not.toBeInTheDocument();
+    expect(
+      screen.getByRole("option", { name: /Amazon Web Services/ }),
+    ).toBeVisible();
+  });
+
+  it("hides the source tabs when discovery denies the user access", async () => {
     // Given
     getInstalledRegistryProviderOptions.mockResolvedValue({
       status: "access_denied",
@@ -195,44 +277,21 @@ describe("Registry provider source tabs", () => {
     ).not.toBeInTheDocument();
   });
 
-  it("shows the Registry tab once discovery confirms the deployment offers it", async () => {
-    // Given: Cloud or Private Cloud with Registry enabled and no artifacts yet.
-    getInstalledRegistryProviderOptions.mockResolvedValue({
-      status: "ready",
-      options: [],
-    });
-
-    // When
-    render(<ConnectAccountForm onSuccess={vi.fn()} />);
-
-    // Then
-    expect(await screen.findByRole("tab", { name: "Registry" })).toBeVisible();
-    expect(screen.getByRole("tab", { name: "All providers" })).toHaveAttribute(
-      "aria-selected",
-      "true",
-    );
-  });
-
-  it("keeps Registry hidden and offers a retry when access is unknown", async () => {
+  it("offers a retry in the Registry tab when access is unknown", async () => {
     // Given
     const user = userEvent.setup();
     getInstalledRegistryProviderOptions
       .mockResolvedValueOnce({ status: "unknown" })
       .mockResolvedValueOnce({ status: "ready", options: [] });
+    render(<ConnectAccountForm onSuccess={vi.fn()} />);
 
     // When
-    render(<ConnectAccountForm onSuccess={vi.fn()} />);
+    await user.click(screen.getByRole("tab", { name: "Registry" }));
 
     // Then
     expect(
       await screen.findByText("Registry providers could not be loaded"),
     ).toBeVisible();
-    expect(
-      screen.queryByRole("tab", { name: "Registry" }),
-    ).not.toBeInTheDocument();
-    expect(
-      screen.getByRole("button", { name: "Retry Registry providers" }),
-    ).toBeVisible();
 
     // When
     await user.click(
@@ -240,7 +299,9 @@ describe("Registry provider source tabs", () => {
     );
 
     // Then
-    expect(await screen.findByRole("tab", { name: "Registry" })).toBeVisible();
+    expect(
+      await screen.findByText("No Registry providers available."),
+    ).toBeVisible();
     expect(
       screen.queryByText("Registry providers could not be loaded"),
     ).not.toBeInTheDocument();
@@ -259,6 +320,7 @@ describe("Registry provider source tabs", () => {
         }),
       );
     render(<ConnectAccountForm onSuccess={vi.fn()} />);
+    await user.click(screen.getByRole("tab", { name: "Registry" }));
     const retry = await screen.findByRole("button", {
       name: "Retry Registry providers",
     });
@@ -296,6 +358,7 @@ describe("Registry provider source tabs", () => {
       )
       .mockResolvedValueOnce({ status: "error" });
     render(<ConnectAccountForm onSuccess={vi.fn()} />);
+    await user.click(screen.getByRole("tab", { name: "Registry" }));
     const retry = await screen.findByRole("button", {
       name: "Retry Registry providers",
     });

ui/components/providers/workflow/forms/connect-account-form.tsx

@@ -21,6 +21,7 @@ import { Alert, AlertDescription, AlertTitle } from "@/components/shadcn/alert";
 import { Form } from "@/components/shadcn/form";
 import { ProviderCredentialFields } from "@/lib/provider-credentials/provider-credential-fields";
 import { REGISTRY_PROVIDER_DISCOVERY } from "@/lib/registry/provider-options";
+import { isRegistryEnabled } from "@/lib/shared/env";
 import {
   createAddProviderFormSchema,
   AddProviderFormValues,
@@ -229,9 +230,10 @@ export const ConnectAccountForm = ({
     registryDiscovery?.status === REGISTRY_PROVIDER_DISCOVERY.READY
       ? registryDiscovery.options
       : [];
+  // The deployment flag is known up front, so the tabs render before discovery.
   const registryAvailable =
-    registryDiscovery?.status === REGISTRY_PROVIDER_DISCOVERY.READY ||
-    registryDiscovery?.status === REGISTRY_PROVIDER_DISCOVERY.ERROR;
+    isRegistryEnabled() &&
+    registryDiscovery?.status !== REGISTRY_PROVIDER_DISCOVERY.ACCESS_DENIED;
   const registryError =
     registryDiscovery?.status === REGISTRY_PROVIDER_DISCOVERY.ERROR ||
     registryDiscovery?.status === REGISTRY_PROVIDER_DISCOVERY.UNKNOWN;
@@ -440,34 +442,41 @@ export const ConnectAccountForm = ({
         {/* Step 1: Provider selection */}
         {prevStep === 1 && (
           <div>
-            {registryDiscovery?.status ===
-              REGISTRY_PROVIDER_DISCOVERY.SUBSCRIPTION_REQUIRED && (
-              <RegistryUpgradePrompt />
-            )}
-            {registryDiscovery?.status ===
-              REGISTRY_PROVIDER_DISCOVERY.ONBOARDING && <RegistrySetupPrompt />}
-            {registryError && (
-              <Alert variant="warning">
-                <AlertTitle>Registry providers could not be loaded</AlertTitle>
-                <AlertDescription>
-                  Built-in providers are available. Check the Registry
-                  connection and try again.
-                  {/* aria-disabled, not disabled: the pressed button keeps focus. */}
-                  <Button
-                    type="button"
-                    variant="outline"
-                    aria-disabled={isRetryingDiscovery}
-                    onClick={retryDiscovery}
-                  >
-                    {isRetryingDiscovery
-                      ? "Retrying…"
-                      : "Retry Registry providers"}
-                  </Button>
-                </AlertDescription>
-              </Alert>
-            )}
             <RadioGroupProvider
               registryAvailable={registryAvailable}
+              registryLoading={registryAvailable && registryDiscovery === null}
+              registryNotice={
+                registryDiscovery?.status ===
+                REGISTRY_PROVIDER_DISCOVERY.SUBSCRIPTION_REQUIRED ? (
+                  <RegistryUpgradePrompt />
+                ) : registryDiscovery?.status ===
+                  REGISTRY_PROVIDER_DISCOVERY.ONBOARDING ? (
+                  <RegistrySetupPrompt />
+                ) : (
+                  registryError && (
+                    <Alert variant="warning">
+                      <AlertTitle>
+                        Registry providers could not be loaded
+                      </AlertTitle>
+                      <AlertDescription>
+                        Built-in providers are available. Check the Registry
+                        connection and try again.
+                        {/* aria-disabled, not disabled: the pressed button keeps focus. */}
+                        <Button
+                          type="button"
+                          variant="outline"
+                          aria-disabled={isRetryingDiscovery}
+                          onClick={retryDiscovery}
+                        >
+                          {isRetryingDiscovery
+                            ? "Retrying…"
+                            : "Retry Registry providers"}
+                        </Button>
+                      </AlertDescription>
+                    </Alert>
+                  )
+                )
+              }
               registryOptions={registryOptions}
               control={form.control}
               isInvalid={!!form.formState.errors.providerType}

ui/components/providers/workflow/forms/use-registry-provider-discovery.test.ts

@@ -22,6 +22,7 @@ describe("useRegistryProviderDiscovery", () => {
   beforeEach(() => {
     vi.clearAllMocks();
     vi.stubEnv("UI_CLOUD_ENABLED", "true");
+    vi.stubEnv("UI_REGISTRY_ENABLED", "true");
   });
 
   it("ignores a stale discovery that settles after a newer one", async () => {

ui/components/providers/workflow/forms/use-registry-provider-discovery.ts

@@ -7,7 +7,7 @@ import {
   REGISTRY_PROVIDER_DISCOVERY,
   type RegistryProviderDiscoveryResult,
 } from "@/lib/registry/provider-options";
-import { isCloud } from "@/lib/shared/env";
+import { isRegistryEnabled } from "@/lib/shared/env";
 
 export function useRegistryProviderDiscovery() {
   // Local state needed: discovery and retries resolve asynchronously.
@@ -20,7 +20,7 @@ export function useRegistryProviderDiscovery() {
   const latestRequest = useRef(0);
 
   useEffect(() => {
-    if (!isCloud()) return;
+    if (!isRegistryEnabled()) return;
     let active = true;
     const load = async () => {
       const request = ++latestRequest.current;