fix(web): mitigate flickering at progress and loaders

web/package/agama-web-ui.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Wed Apr  9 12:38:21 UTC 2025 - David Diaz <[email protected]>
+
+- Fix flickering issue during loading and progress transitions
+  (gh#agama-project/agama#2240, gh#agama-project/agama#2255).
+
 -------------------------------------------------------------------
 Wed Apr  9 05:36:35 UTC 2025 - José Iván López González <[email protected]>
 

web/src/App.tsx

@@ -62,10 +62,10 @@ function App() {
       return <Navigate to={ROOT.installationFinished} />;
     }
 
-    if (!products || !connected) return <Loading useLayout />;
+    if (!products || !connected) return <Loading listenQuestions />;
 
     if (phase === InstallationPhase.Startup && isBusy) {
-      return <Loading useLayout />;
+      return <Loading listenQuestions />;
     }
 
     if (selectedProduct === undefined && location.pathname !== PRODUCT.root) {

web/src/components/core/ProgressReport.tsx

@@ -22,24 +22,19 @@
 
 import React, { useEffect, useState } from "react";
 import {
-  Bullseye,
-  Card,
-  CardBody,
   Content,
   Flex,
-  Grid,
-  GridItem,
   ProgressStep,
   ProgressStepper,
   ProgressStepProps,
   Spinner,
   Stack,
   Truncate,
 } from "@patternfly/react-core";
-
-import { _ } from "~/i18n";
 import { useProgress, useProgressChanges, useResetProgress } from "~/queries/progress";
 import { Progress as ProgressType } from "~/types/progress";
+import sizingStyles from "@patternfly/react-styles/css/utilities/Sizing/sizing";
+import { _ } from "~/i18n";
 
 type StepProps = {
   id: string;
@@ -90,7 +85,10 @@ const Progress = ({ steps, step, firstStep, detail }) => {
   };
 
   return (
-    <ProgressStepper isCenterAligned className="progress-report">
+    <ProgressStepper
+      isCenterAligned
+      className={[sizingStyles.w_100, sizingStyles.h_33OnMd].join(" ")}
+    >
       {firstStep && (
         <ProgressStep key="initial" variant="success">
           {firstStep}
@@ -132,27 +130,17 @@ function ProgressReport({ title, firstStep }: { title: string; firstStep?: React
   const detail = findDetail([softwareProgress, storageProgress]);
 
   return (
-    <Bullseye>
-      <Grid hasGutter>
-        <GridItem sm={10} smOffset={1}>
-          <Card isPlain>
-            <CardBody>
-              <Flex
-                direction={{ default: "column" }}
-                rowGap={{ default: "rowGap2xl" }}
-                alignItems={{ default: "alignItemsCenter" }}
-              >
-                <Spinner size="xl" />
-                <Content component="h1" id="progress-title" style={{ textAlign: "center" }}>
-                  {title}
-                </Content>
-                <Progress steps={steps} step={progress} detail={detail} firstStep={firstStep} />
-              </Flex>
-            </CardBody>
-          </Card>
-        </GridItem>
-      </Grid>
-    </Bullseye>
+    <Flex
+      direction={{ default: "column" }}
+      rowGap={{ default: "rowGapMd" }}
+      alignItems={{ default: "alignItemsCenter" }}
+      justifyContent={{ default: "justifyContentCenter" }}
+      className={sizingStyles.h_100OnMd}
+    >
+      <Spinner size="xl" />
+      <Content component="h1">{title}</Content>
+      <Progress steps={steps} step={progress} detail={detail} firstStep={firstStep} />
+    </Flex>
   );
 }
 

web/src/components/layout/Loading.test.tsx

@@ -45,34 +45,41 @@ jest.mock("~/components/layout/Layout", () => {
 });
 
 describe("Loading", () => {
-  it("renders given message", async () => {
-    plainRender(<Loading text="Doing something" />);
-    await screen.findByText("Doing something");
+  it("renders provided text", async () => {
+    plainRender(<Loading text="Loading something" />);
+    await screen.findByText("Loading something");
   });
 
-  describe("when not using a custom message", () => {
-    it("renders the default loading environment message", async () => {
-      plainRender(<Loading />);
-      await screen.findByText(/Loading installation environment/i);
-    });
+  it("uses provided aria-label", async () => {
+    plainRender(<Loading aria-label="Loading something" />);
+    const icon = await screen.findByLabelText("Loading something");
+    expect(icon).toHaveRole("progressbar");
   });
 
-  describe("when not using the useLayout prop or its value is false", () => {
-    it("does not wrap the content within a PlainLayout", () => {
-      const { rerender } = plainRender(<Loading text="Making a test" />);
-      expect(screen.queryByText("PlainLayout Mock")).toBeNull();
-      rerender(<Loading text="Making a test" useLayout={false} />);
-      expect(screen.queryByText("PlainLayout Mock")).toBeNull();
-    });
+  it("uses 'Loading' as default aria-label when neither text nor aria-label is provided", async () => {
+    plainRender(<Loading />);
+    const icon = await screen.findByLabelText("Loading");
+    expect(icon).toHaveRole("progressbar");
   });
 
-  describe("when using the useLayout prop", () => {
-    it("wraps the content within a PlainLayout with neither, header nor sidebar", () => {
-      installerRender(<Loading text="Making a test" useLayout />);
-      expect(screen.queryByText("Header Mock")).toBeNull();
-      expect(screen.queryByText("Sidebar Mock")).toBeNull();
-      screen.getByText("PlainLayout Mock");
-      screen.getByText("Making a test");
-    });
+  it("hides the spinner icon from a11y tree when text is given", () => {
+    const { container } = plainRender(<Loading text="Loading something" />);
+    const icon = container.querySelector("svg");
+    expect(icon).toHaveAttribute("aria-hidden");
+  });
+
+  it("wraps itself within a PlainLayout without header and sideabar when listenQuestions is enabled", () => {
+    installerRender(<Loading text="Making a test" listenQuestions />);
+    expect(screen.queryByText("Header Mock")).toBeNull();
+    expect(screen.queryByText("Sidebar Mock")).toBeNull();
+    screen.getByText("PlainLayout Mock");
+    screen.getByText("Making a test");
+  });
+
+  it("does not wrap itself within a PlainLayout when listenQuestions is not used or set to false", () => {
+    const { rerender } = plainRender(<Loading text="Making a test" />);
+    expect(screen.queryByText("PlainLayout Mock")).toBeNull();
+    rerender(<Loading text="Making a test" listenQuestions={false} />);
+    expect(screen.queryByText("PlainLayout Mock")).toBeNull();
   });
 });

web/src/components/layout/Loading.tsx

@@ -20,27 +20,124 @@
  * find current contact information at www.suse.com.
  */
 
-import React from "react";
-import { Bullseye, EmptyState, Spinner } from "@patternfly/react-core";
+import React, { Fragment } from "react";
+import { Flex, EmptyState, Spinner, SpinnerProps } from "@patternfly/react-core";
 import { PlainLayout } from "~/components/layout";
 import { LayoutProps } from "~/components/layout/Layout";
+import sizingStyles from "@patternfly/react-styles/css/utilities/Sizing/sizing";
+import spacingStyles from "@patternfly/react-styles/css/utilities/Spacing/spacing";
+import { isEmpty } from "~/utils";
 import { _ } from "~/i18n";
 
-const LoadingIcon = () => <Spinner size="xl" />;
+/**
+ * Renders a plain layout without either, header nor mountSidebar
+ */
 const Layout = (props: LayoutProps) => (
   <PlainLayout mountHeader={false} mountSidebar={false} {...props} />
 );
 
+type LoadingProps = {
+  /** Text to be rendered alongside the spinner */
+  text?: string;
+  /** Accessible text, required when rendering only the spinner */
+  "aria-label"?: string;
+  /**
+   * Whether the loading screen should listen for and render any questions
+   *
+   * The Questions component is mounted within the application layout
+   * (src/components/Layout.tsx). However, certain branches in src/App.tsx force
+   * to render the Loading component before the layout is mounted.
+   *
+   * This behavior requires a mechanism to enable the loading to listen
+   * for and render backend questions before the frontend has all the
+   * data necessary to fully mount the layout.
+   *
+   * This is why this prop exists. While this could be improved and ideally
+   * the Loading component shouldn’t need to wrap itself with the layout, be
+   * cautious when tempted to remove this behavior without a solid alternative.
+   * Doing so could silently reintroduced the regression fixed in
+   * https://github.com/agama-project/agama/pull/1825
+   *
+   * FIXME: Find and implement a solid alternative
+   */
+  listenQuestions?: boolean;
+  /** Loader style, full screen or inline */
+  variant?: "full-screen" | "inline";
+  /** Size for the spinner icon */
+  spinnerSize?: SpinnerProps["size"];
+};
+
+/**
+ * Renders a loader centered in the screen
+ */
+const FullScreenLoading = ({
+  text,
+  "aria-label": ariaLabel,
+  spinnerSize,
+}: Omit<LoadingProps, "listenQuestions" | "variant">) => {
+  return (
+    <Flex
+      className={sizingStyles.h_100vh}
+      alignContent={{ default: "alignContentCenter" }}
+      justifyContent={{ default: "justifyContentCenter" }}
+    >
+      {isEmpty(text) ? (
+        <Spinner
+          size={spinnerSize}
+          aria-label={ariaLabel}
+          aria-hidden={isEmpty(ariaLabel) || undefined}
+        />
+      ) : (
+        <EmptyState
+          variant="xl"
+          titleText={text}
+          headingLevel="h1"
+          icon={() => <Spinner size={spinnerSize} aria-hidden />}
+        />
+      )}
+    </Flex>
+  );
+};
+
+/**
+ * Renders an inline loader
+ */
+const InlineLoading = ({
+  text,
+  "aria-label": ariaLabel,
+  spinnerSize,
+}: Omit<LoadingProps, "listenQuestions" | "variant">) => {
+  return (
+    <Flex gap={{ default: "gapMd" }} className={spacingStyles.pMd}>
+      <Spinner
+        size={spinnerSize}
+        aria-label={ariaLabel}
+        aria-hidden={(!isEmpty(text) && isEmpty(ariaLabel)) || undefined}
+      />
+      {text}
+    </Flex>
+  );
+};
+
 function Loading({
-  text = _("Loading installation environment, please wait."),
-  useLayout = false,
-}) {
-  const Wrapper = useLayout ? Layout : React.Fragment;
+  text,
+  "aria-label": ariaLabel,
+  listenQuestions = false,
+  variant = "full-screen",
+  spinnerSize,
+}: LoadingProps) {
+  const Wrapper = listenQuestions ? Layout : Fragment;
+  const LoadingComponent = variant === "full-screen" ? FullScreenLoading : InlineLoading;
+  const defaultSpinnerSize = variant === "full-screen" ? "xl" : "sm";
+  const defaultAriaLabel = _("Loading");
+
   return (
     <Wrapper>
-      <Bullseye>
-        <EmptyState variant="xl" titleText={text} headingLevel="h1" icon={LoadingIcon} />
-      </Bullseye>
+      <LoadingComponent
+        text={text}
+        aria-label={ariaLabel || (isEmpty(text) ? defaultAriaLabel : undefined)}
+        spinnerSize={spinnerSize || defaultSpinnerSize}
+      />
     </Wrapper>
   );
 }