fix(web): storage UI fixes and improvements

web/src/assets/styles/blocks.scss

@@ -550,6 +550,10 @@ table.proposal-result {
   input {
     text-align: end;
   }
+
+  select {
+    min-inline-size: fit-content;
+  }
 }
 
 [data-type="agama/options-picker"] {

web/src/components/core/InstallationFinished.jsx

@@ -35,13 +35,14 @@ import { Center, Icon } from "~/components/layout";
 import { EncryptionMethods } from "~/client/storage";
 import { _ } from "~/i18n";
 import { useInstallerClient } from "~/context/installer";
+import alignmentStyles from '@patternfly/react-styles/css/utilities/Alignment/alignment';
 
 const TpmHint = () => {
   const [isExpanded, setIsExpanded] = useState(false);
   const title = _("TPM sealing requires the new system to be booted directly.");
 
   return (
-    <Alert isInline variant="info" className="tpm-hint" title={<strong>{title}</strong>}>
+    <Alert isInline className={alignmentStyles.textAlignLeft} title={<strong>{title}</strong>}>
       <Stack hasGutter>
         {_("If a local media was used to run this installer, remove it before the next boot.")}
         <ExpandableSection
@@ -74,12 +75,9 @@ function InstallationFinished() {
       const iguana = await client.manager.useIguana();
       // FIXME: This logic should likely not be placed here, it's too coupled to storage internals.
       // Something to fix when this whole page is refactored in a (hopefully near) future.
-      // const { settings: { encryptionPassword, encryptionMethod } } = await client.storage.proposal.getResult();
-      // TODO: The storage client is not adapted to the HTTP API yet.
-      const encryptionPassword = null;
-      const encryptionMethod = null;
+      const { settings: { encryptionPassword, encryptionMethod } } = await client.storage.proposal.getResult();
       setUsingIguana(iguana);
-      setUsingTpm(encryptionPassword?.length && encryptionMethod === EncryptionMethods.TPM);
+      setUsingTpm(encryptionPassword?.length > 0 && encryptionMethod === EncryptionMethods.TPM);
     }
 
     // TODO: display the page in a loading mode while needed data is being fetched.
@@ -107,7 +105,7 @@ function InstallationFinished() {
                           ? _("At this point you can power off the machine.")
                           : _("At this point you can reboot the machine to log in to the new system.")}
                       </Text>
-                      {!usingTpm && <TpmHint />}
+                      {usingTpm && <TpmHint />}
                     </EmptyStateBody>
                   </EmptyState>
                   <Flex direction={{ default: "rowReverse" }}>

web/src/components/core/InstallationFinished.test.jsx

@@ -34,7 +34,7 @@ const finishInstallationFn = jest.fn();
 let encryptionPassword;
 let encryptionMethod;
 
-describe.skip("InstallationFinished", () => {
+describe("InstallationFinished", () => {
   beforeEach(() => {
     encryptionPassword = "n0tS3cr3t";
     encryptionMethod = EncryptionMethods.LUKS2;
@@ -72,7 +72,7 @@ describe.skip("InstallationFinished", () => {
     expect(finishInstallationFn).toHaveBeenCalled();
   });
 
-  describe.skip("when TPM is set as encryption method", () => {
+  describe("when TPM is set as encryption method", () => {
     beforeEach(() => {
       encryptionMethod = EncryptionMethods.TPM;
     });
@@ -90,27 +90,15 @@ describe.skip("InstallationFinished", () => {
       });
 
       it("does not show the TPM reminder", async () => {
-        const { user } = installerRender(<InstallationFinished />);
-        // Forcing the test to slow down a bit with a fake user interaction
-        // because actually the reminder will be not rendered immediately
-        // making the queryAllByText to produce a false positive if triggered
-        // too early here.
-        const congratsText = screen.getByText("Congratulations!");
-        await user.click(congratsText);
-        expect(screen.queryAllByText(/TPM/)).toHaveLength(0);
+        installerRender(<InstallationFinished />);
+        screen.queryAllByText(/TPM/);
       });
     });
   });
 
   describe("when TPM is not set as encryption method", () => {
     it("does not show the TPM reminder", async () => {
-      const { user } = installerRender(<InstallationFinished />);
-      // Forcing the test to slow down a bit with a fake user interaction
-      // because actually the reminder will be not rendered immediately
-      // making the queryAllByText to produce a false positive if triggered
-      // too early here.
-      const congratsText = screen.getByText("Congratulations!");
-      await user.click(congratsText);
+      installerRender(<InstallationFinished />);
       expect(screen.queryAllByText(/TPM/)).toHaveLength(0);
     });
   });

web/src/components/core/ProgressReport.jsx

@@ -20,11 +20,10 @@
  */
 
 import React, { useState, useEffect } from "react";
+import { Flex, Progress, Spinner, Text } from "@patternfly/react-core";
 import { useCancellablePromise } from "~/utils";
 import { useInstallerClient } from "~/context/installer";
 
-import { Grid, GridItem, Progress, Text } from "@patternfly/react-core";
-
 const ProgressReport = () => {
   const client = useInstallerClient();
   const { cancellablePromise } = useCancellablePromise();
@@ -54,34 +53,45 @@ const ProgressReport = () => {
     });
   }, [client.software]);
 
-  if (!progress.steps) return <Text>Waiting for progress status...</Text>;
+  if (!progress.steps) {
+    return (
+      <Flex
+        direction={{ default: "column" }}
+        rowGap={{ default: "rowGapXl" }}
+        alignItems={{ default: "alignItemsCenter" }}
+        justifyContent={{ default: "justifyContentCenter" }}
+      >
+        <Spinner />
+        <Text component="h1">Waiting for progress status...</Text>
+      </Flex>
+    );
+  }
 
   return (
-    <Grid hasGutter>
-      <GridItem sm={12}>
-        <Progress
-          min={0}
-          max={progress.steps}
-          value={progress.step}
-          title={progress.message}
-          label={" "}
-          aria-label={progress.message}
-        />
+    <Flex
+      direction={{ default: "column" }}
+      rowGap={{ default: "rowGapMd" }}
+    >
+      <Progress
+        min={0}
+        max={progress.steps}
+        value={progress.step}
+        title={progress.message}
+        measureLocation="none"
+      />
 
+      {
+        subProgress &&
         <Progress
-          size="sm"
           min={0}
-          max={subProgress?.steps}
-          value={subProgress?.step}
-          title={subProgress?.message}
-          label={" "}
+          max={subProgress.steps}
+          value={subProgress.step}
+          title={subProgress.message}
           measureLocation="none"
-          className={!subProgress && 'hidden'}
-          aria-label={subProgress?.message || " "}
-          aria-hidden={!subProgress}
+          size="sm"
         />
-      </GridItem>
-    </Grid>
+      }
+    </Flex>
   );
 };
 

web/src/components/core/ProgressReport.test.jsx

@@ -32,15 +32,14 @@ jest.mock("~/client");
 let callbacks;
 let onManagerProgressChange = jest.fn();
 let onSoftwareProgressChange = jest.fn();
+const getProgressFn = jest.fn();
 
 beforeEach(() => {
   createClient.mockImplementation(() => {
     return {
       manager: {
         onProgressChange: onManagerProgressChange,
-        getProgress: jest.fn().mockResolvedValue(
-          { message: "Reading repositories", current: 1, total: 10 }
-        )
+        getProgress: getProgressFn
       },
       software: {
         onProgressChange: onSoftwareProgressChange
@@ -51,10 +50,13 @@ beforeEach(() => {
 
 describe("ProgressReport", () => {
   describe("when there is not progress information available", () => {
-    it.skip("renders a waiting message", () => {
-      installerRender(<ProgressReport />);
+    beforeEach(() => {
+      getProgressFn.mockResolvedValue({});
+    });
 
-      expect(screen.findByText(/Waiting for/i)).toBeInTheDocument();
+    it("renders a waiting message", async () => {
+      installerRender(<ProgressReport />);
+      await screen.findByText(/Waiting for progress status/i);
     });
   });
 
@@ -64,13 +66,15 @@ describe("ProgressReport", () => {
       const [onSoftwareProgress, softwareCallbacks] = createCallbackMock();
       onManagerProgressChange = onManagerProgress;
       onSoftwareProgressChange = onSoftwareProgress;
+      getProgressFn.mockResolvedValue(
+        { message: "Reading repositories", current: 1, total: 10 }
+      );
       callbacks = { manager: managerCallbacks, software: softwareCallbacks };
     });
 
     it("shows the main progress bar", async () => {
       installerRender(<ProgressReport />);
 
-      await screen.findByText(/Waiting/i);
       await screen.findByText(/Reading/i);
 
       // NOTE: there can be more than one subscriptions to the
@@ -80,43 +84,30 @@ describe("ProgressReport", () => {
         cb({ message: "Partitioning", current: 1, total: 10 });
       });
 
-      await screen.findByLabelText("Partitioning");
+      await screen.findByRole("progressbar", { name: "Partitioning" });
     });
 
-    it("does not show secondary progress bar", async () => {
+    it("shows secondary progress bar when there is information from software service ", async () => {
       installerRender(<ProgressReport />);
 
-      await screen.findByText(/Waiting/i);
+      const managerCallback = callbacks.manager[callbacks.manager.length - 1];
+      const softwareCallback = callbacks.software[callbacks.software.length - 1];
 
-      const cb = callbacks.manager[callbacks.manager.length - 1];
       act(() => {
-        cb({ message: "Partitioning", current: 1, total: 10 });
+        managerCallback({ message: "Partitioning", current: 1, total: 10 });
       });
 
-      await screen.findAllByRole("progressbar", { hidden: true });
-    });
+      await screen.findByRole("progressbar", { name: "Partitioning" });
+      const bars = await screen.findAllByRole("progressbar");
+      expect(bars.length).toBe(1);
 
-    describe("when there is progress information from the software service", () => {
-      it("shows the secondary progress bar", async () => {
-        installerRender(<ProgressReport />);
-
-        await screen.findByText(/Waiting/i);
-
-        // NOTE: there can be more than one subscriptions to the
-        // manager#onChange. We're interested in the latest one here.
-        const cb0 = callbacks.manager[callbacks.manager.length - 1];
-        act(() => {
-          cb0({ message: "Installing software", current: 4, total: 10 });
-        });
-
-        const cb1 = callbacks.software[callbacks.software.length - 1];
-        act(() => {
-          cb1({ message: "Installing YaST2", current: 256, total: 500, finished: false });
-        });
-
-        const bars = screen.queryAllByRole("progressbar", { hidden: false });
-        expect(bars.length).toEqual(2);
+      act(() => {
+        managerCallback({ message: "Installing software", current: 4, total: 10 });
+        softwareCallback({ message: "Installing YaST2", current: 256, total: 500, finished: false });
       });
+
+      await screen.findByRole("progressbar", { name: "Installing software" });
+      await screen.findByRole("progressbar", { name: "Installing YaST2" });
     });
   });
 });