Merge pull request #387 from KKoukiou/confirm-modal-checkbox

review: switch from confirmation modal to checkbox

src/components/review/ReviewConfiguration.jsx

@@ -18,17 +18,18 @@ import cockpit from "cockpit";
 
 import React, { useContext, useEffect, useMemo, useState } from "react";
 import {
-    Button,
+    Checkbox,
     DescriptionList,
     Flex, FlexItem,
-    HelperText, HelperTextItem,
-    Modal, ModalVariant,
     Stack,
     useWizardFooter,
 } from "@patternfly/react-core";
 
+import { getDeviceChildren } from "../../helpers/storage.js";
+
 import { AnacondaWizardFooter } from "../AnacondaWizardFooter.jsx";
-import { LanguageContext, OsReleaseContext, SystemTypeContext, UsersContext } from "../Common.jsx";
+import { LanguageContext, OsReleaseContext, StorageContext, SystemTypeContext, UsersContext } from "../Common.jsx";
+import { useOriginalDevices, usePlannedActions } from "../storage/Common.jsx";
 import { useScenario } from "../storage/InstallationScenario.jsx";
 import { ReviewDescriptionListItem } from "./Common.jsx";
 import { HostnameRow } from "./Hostname.jsx";
@@ -37,6 +38,7 @@ import { StorageReview, StorageReviewNote } from "./StorageReview.jsx";
 import "./ReviewConfiguration.scss";
 
 const _ = cockpit.gettext;
+const idPrefix = "installation-review";
 
 const ReviewDescriptionList = ({ children }) => {
     return (
@@ -56,19 +58,15 @@ const ReviewDescriptionList = ({ children }) => {
     );
 };
 
-const ReviewConfiguration = ({ idPrefix, setIsFormValid }) => {
+const ReviewConfiguration = ({ setIsFormValid }) => {
     const osRelease = useContext(OsReleaseContext);
     const localizationData = useContext(LanguageContext);
     const accounts = useContext(UsersContext);
     const { label: scenarioLabel } = useScenario();
     const isBootIso = useContext(SystemTypeContext) === "BOOT_ISO";
 
-    useEffect(() => {
-        setIsFormValid(true);
-    }, [setIsFormValid]);
-
     // Display custom footer
-    const getFooter = useMemo(() => <CustomFooter />, []);
+    const getFooter = useMemo(() => <CustomFooter setIsFormValid={setIsFormValid} />, [setIsFormValid]);
     useWizardFooter(getFooter);
 
     const language = useMemo(() => {
@@ -145,82 +143,83 @@ const ReviewConfiguration = ({ idPrefix, setIsFormValid }) => {
     );
 };
 
-export const ReviewConfigurationConfirmModal = ({ idPrefix, onNext, setNextWaitsConfirmation }) => {
-    const { buttonLabel, buttonVariant, dialogTitleIconVariant, dialogWarning, dialogWarningTitle } = useScenario();
-    return (
-        <Modal
-          actions={[
-              <Button
-                id={idPrefix + "-disk-erase-confirm"}
-                key="confirm"
-                onClick={() => {
-                    setNextWaitsConfirmation(false);
-                    onNext();
-                }}
-                variant={buttonVariant}
-              >
-                  {buttonLabel}
-              </Button>,
-              <Button
-                key="cancel"
-                onClick={() => setNextWaitsConfirmation(false)}
-                variant="link">
-                  {_("Back")}
-              </Button>
-          ]}
-          isOpen
-          onClose={() => setNextWaitsConfirmation(false)}
-          title={dialogWarningTitle}
-          titleIconVariant={dialogTitleIconVariant}
-          variant={ModalVariant.small}
-        >
-            {dialogWarning}
-        </Modal>
-    );
-};
+const useConfirmationCheckboxLabel = () => {
+    const [scenarioConfirmationLabel, setScenarioConfirmationLabel] = useState(null);
+    const originalDevices = useOriginalDevices();
+    const plannedActions = usePlannedActions();
+    const { diskSelection } = useContext(StorageContext);
+    const usableDisks = diskSelection.usableDisks;
+    const selectedDisks = diskSelection.selectedDisks;
+
+    const relevantDevices = selectedDisks
+            .map(disk => getDeviceChildren({ device: disk, deviceData: originalDevices }))
+            .flat(Infinity);
+    const deviceHasAction = (actionType, device) => plannedActions.find(action => action["device-id"].v === device && action["action-type"].v === actionType);
+    const deletedDevices = relevantDevices.filter(device => deviceHasAction("destroy", device));
+    const resizedDevices = relevantDevices.filter(device => deviceHasAction("resize", device));
+
+    const allDevicesDeleted = deletedDevices.length > 0 && deletedDevices.length === relevantDevices.length;
+    const someDevicesDeleted = deletedDevices.length > 0 && deletedDevices.length < relevantDevices.length;
+    const someDevicesResized = resizedDevices.length > 0;
 
-const ReviewConfigurationFooterHelperText = () => {
-    const { screenWarning } = useScenario();
+    useEffect(() => {
+        const allDevicesDeletedText = cockpit.ngettext(
+            _("I understand that all existing data will be erased"),
+            _("I understand that all existing data will be erased from the selected disks"),
+            usableDisks.length
+        );
+
+        const someDevicesDeletedText = _("I understand that some existing data will be erased");
+        const someDevicesResizedText = _("I understand that some partitions will be modified");
+
+        if (allDevicesDeleted) {
+            setScenarioConfirmationLabel(allDevicesDeletedText);
+        } else if (someDevicesDeleted) {
+            setScenarioConfirmationLabel(someDevicesDeletedText);
+        } else if (someDevicesResized) {
+            setScenarioConfirmationLabel(someDevicesResizedText);
+        } else {
+            setScenarioConfirmationLabel("");
+        }
+    }, [allDevicesDeleted, someDevicesDeleted, someDevicesResized, usableDisks.length]);
 
-    return (
-        <HelperText id="review-warning-text">
-            <HelperTextItem
-              variant="warning"
-              hasIcon>
-                {screenWarning}
-            </HelperTextItem>
-        </HelperText>
-    );
+    return scenarioConfirmationLabel;
 };
 
-const CustomFooter = () => {
-    const [nextWaitsConfirmation, setNextWaitsConfirmation] = useState();
+const CustomFooter = ({ setIsFormValid }) => {
     const { buttonLabel } = useScenario();
-    const pageProps = new Page();
-    const footerHelperText = <ReviewConfigurationFooterHelperText />;
+    const scenarioConfirmationLabel = useConfirmationCheckboxLabel();
+    const installationIsClean = scenarioConfirmationLabel === "";
+    const [isConfirmed, setIsConfirmed] = useState(false);
+
+    const confirmationCheckbox = (
+        !installationIsClean &&
+        <Checkbox
+          id={idPrefix + "-next-confirmation-checkbox"}
+          label={scenarioConfirmationLabel}
+          isChecked={isConfirmed}
+          onChange={(_event, checked) => setIsConfirmed(checked)}
+        />
+    );
+
+    useEffect(() => {
+        setIsFormValid(isConfirmed || installationIsClean);
+    }, [setIsFormValid, isConfirmed, installationIsClean]);
 
     return (
-        <>
-            {nextWaitsConfirmation &&
-            <ReviewConfigurationConfirmModal
-              idPrefix={pageProps.id}
-              onNext={() => cockpit.location.go(["installation-progress"])}
-              setNextWaitsConfirmation={setNextWaitsConfirmation}
-            />}
-            <AnacondaWizardFooter
-              footerHelperText={footerHelperText}
-              nextButtonText={buttonLabel}
-              nextButtonVariant="warning"
-              onNext={() => nextWaitsConfirmation === undefined && setNextWaitsConfirmation(true)}
-            />
-        </>
+        <AnacondaWizardFooter
+          footerHelperText={confirmationCheckbox}
+          nextButtonText={buttonLabel}
+          nextButtonVariant={!installationIsClean ? "warning" : "primary"}
+          onNext={() => cockpit.location.go(["installation-progress"])}
+        />
     );
 };
 
 export class Page {
     constructor () {
         this.component = ReviewConfiguration;
-        this.id = "installation-review";
+        this.id = idPrefix;
         this.label = _("Review and install");
         this.title = _("Review and install");
     }

src/components/storage/InstallationScenario.jsx

@@ -213,14 +213,10 @@ export const scenarios = [{
     check: checkEraseAll,
     default: true,
     detail: helpEraseAll,
-    dialogTitleIconVariant: "warning",
-    dialogWarning: _("The selected disks will be erased, this cannot be undone. Are you sure you want to continue with the installation?"),
-    dialogWarningTitle: _("Erase data and install?"),
     id: "erase-all",
     // CLEAR_PARTITIONS_ALL = 1
     initializationMode: 1,
     label: _("Erase data and install"),
-    screenWarning: _("Erasing the data cannot be undone. Be sure to have backups."),
 }, {
     action: ReclaimSpace,
     buttonLabel: _("Install"),
@@ -229,42 +225,30 @@ export const scenarios = [{
     check: checkUseFreeSpace,
     default: false,
     detail: helpUseFreeSpace,
-    dialogTitleIconVariant: "",
-    dialogWarning: _("The installation will use the available space on your devices and will not erase any device data."),
-    dialogWarningTitle: _("Install on the free space?"),
     id: "use-free-space",
     // CLEAR_PARTITIONS_NONE = 0
     initializationMode: 0,
     label: _("Use free space for the installation"),
-    screenWarning: _("To prevent loss, make sure to backup your data."),
 }, {
     buttonLabel: _("Apply mount point assignment and install"),
     buttonVariant: "danger",
     check: checkMountPointMapping,
     default: false,
     detail: helpMountPointMapping,
-    dialogTitleIconVariant: "",
-    dialogWarning: _("The installation will use your configured partitioning layout."),
-    dialogWarningTitle: _("Install on the custom mount points?"),
     id: "mount-point-mapping",
     // CLEAR_PARTITIONS_NONE = 0
     initializationMode: 0,
     label: _("Mount point assignment"),
-    screenWarning: _("To prevent loss, make sure to backup your data."),
 }, {
     buttonLabel: _("Install"),
     buttonVariant: "danger",
     check: checkConfiguredStorage,
     default: false,
     detail: helpConfiguredStorage,
-    dialogTitleIconVariant: "",
-    dialogWarning: _("The installation will use your configured partitioning layout."),
-    dialogWarningTitle: _("Install using the configured storage?"),
     id: "use-configured-storage",
     // CLEAR_PARTITIONS_NONE = 0
     initializationMode: 0,
     label: _("Use configured storage"),
-    screenWarning: _("To prevent loss, make sure to backup your data."),
 }
 ];
 

test/check-existing-system

@@ -56,14 +56,14 @@ class TestFedoraPlansUseFreeSpace(VirtInstallMachineCase):
     disk_image = "debian-stable"
     disk_size = 20
 
-    def install(self):
+    def install(self, needs_confirmation):
         b = self.browser
         m = self.machine
 
         i = Installer(b, m)
         p = Progress(b)
 
-        i.begin_installation(button_text="Install")
+        i.begin_installation(button_text="Install", needs_confirmation=needs_confirmation)
         with b.wait_timeout(300):
             p.wait_done()
 
@@ -113,6 +113,7 @@ class TestFedoraPlansUseFreeSpace(VirtInstallMachineCase):
         b = self.browser
         m = self.machine
         i = Installer(b, m)
+        r = Review(b, m)
         s = Storage(b, m)
 
         i.open()
@@ -121,17 +122,18 @@ class TestFedoraPlansUseFreeSpace(VirtInstallMachineCase):
         i.next()
         s.check_encryption_selected(False)
         i.reach(i.steps.REVIEW)
+        r.check_checkbox_not_present()
 
-        self.install()
+        self.install(needs_confirmation=False)
         self.verifyDualBoot(root_one_size=9, root_two_size=9.9)
 
     @test_plan("https://fedoraproject.org/wiki/QA:Testcase_partitioning_guided_shrink")
     def testReclaimSpaceShrink(self):
         b = self.browser
         m = self.machine
         i = Installer(b, m)
-        s = Storage(b, m)
         r = Review(b, m)
+        s = Storage(b, m)
 
         i.open()
         i.reach(i.steps.INSTALLATION_METHOD)
@@ -143,11 +145,12 @@ class TestFedoraPlansUseFreeSpace(VirtInstallMachineCase):
         s.reclaim_modal_submit()
 
         i.reach(i.steps.REVIEW)
+        r.check_some_resized_checkbox_label()
         r.check_disk_row("vda", parent="vda1", size="5.00 GB", action="resized from 10.6 GB")
         # FIXME: uncomment when https://github.com/storaged-project/blivet/pull/1256 is released
         # r.check_resized_system("Debian", ["vda1"])
 
-        self.install()
+        self.install(needs_confirmation=True)
         self.verifyDualBoot(root_one_size=14.2, root_two_size=4.7)
 
 @nondestructive
@@ -170,6 +173,7 @@ class TestExistingSystemWindows(VirtInstallMachineCase):
         for device in ["vda1", "vda2", "vda3", "vda4"]:
             r.check_disk_row("vda", parent=device, action="delete")
 
+        r.check_all_erased_checkbox_label()
         r.check_deleted_system("Windows")
 
 

test/check-progress

@@ -38,7 +38,7 @@ class TestInstallationProgress(VirtInstallMachineCase):
         i.open()
 
         i.reach(i.steps.REVIEW)
-        i.begin_installation()
+        i.begin_installation(needs_confirmation=False)
 
         with b.wait_timeout(300):
             b.wait_in_text("h2", "Installing")

test/check-review

@@ -82,16 +82,6 @@ class TestReview(VirtInstallMachineCase):
             ignore=pixel_tests_ignore,
         )
 
-    def testNoConfirmation(self):
-        b = self.browser
-        m = self.machine
-        i = Installer(b, m)
-
-        i.open()
-
-        i.reach(i.steps.REVIEW)
-        i.begin_installation(should_fail=True, confirm_erase=False)
-
     def testUnknownLanguage(self):
         b = self.browser
         m = self.machine