Improve deletion confirmation modal

fixes #1100
cockpit-project · Oct 19, 2023 · d6a1364 · d6a1364
1 parent c78f68c
commit d6a1364
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 23 deletions.
diff --git a/src/ImageDeleteModal.jsx b/src/ImageDeleteModal.jsx
@@ -1,6 +1,7 @@
 import React, { useState } from 'react';
 import { Button } from "@patternfly/react-core/dist/esm/components/Button";
 import { Checkbox } from "@patternfly/react-core/dist/esm/components/Checkbox";
+import { List, ListItem } from '@patternfly/react-core/dist/esm/components/List';
 import { Modal } from "@patternfly/react-core/dist/esm/components/Modal";
 import { Stack, StackItem } from "@patternfly/react-core/dist/esm/layouts/Stack";
 import { useDialogs } from "dialogs.jsx";
@@ -39,13 +40,6 @@ export const ImageDeleteModal = ({ imageWillDelete, onAddNotification }) => {
         }));
     };
 
-    const pickAll = () => {
-        setTags(prevState => Object.keys(prevState).reduce((acc, item, i) => {
-            acc[item] = true;
-            return acc;
-        }, {}));
-    };
-
     const handleRemoveImage = (tags, all) => {
         const handleForceRemoveImage = () => {
             Dialogs.close();
@@ -81,11 +75,20 @@ export const ImageDeleteModal = ({ imageWillDelete, onAddNotification }) => {
         }
     };
 
+    const imageName = repoTags[0]?.split(":")[0].split("/").at(-1) ?? "";
+
+    let isAllSelected = null;
+    if (checkedTags.length === repoTags.length)
+        isAllSelected = true;
+    else if (checkedTags.length === 0)
+        isAllSelected = false;
+
     return (
         <Modal isOpen
                  position="top" variant="medium"
+                 titleIconVariant="warning"
                  onClose={Dialogs.close}
-                 title={cockpit.format(_("Delete $0"), repoTags ? repoTags[0] : "")}
+                 title={cockpit.format(_("Delete $0 image?"), imageName)}
                  footer={<>
                      <Button id="btn-img-delete" variant="danger" isDisabled={checkedTags.length === 0}
                              onClick={() => handleRemoveImage(checkedTags, checkedTags.length === repoTags.length)}>
@@ -97,19 +100,21 @@ export const ImageDeleteModal = ({ imageWillDelete, onAddNotification }) => {
             <Stack hasGutter>
                 { repoTags.length > 1 && <StackItem>{_("Multiple tags exist for this image. Select the tagged images to delete.")}</StackItem> }
                 <StackItem isFilled>
-                    { repoTags.map(x => {
-                        return (
-                            <Checkbox isChecked={checkedTags.indexOf(x) > -1}
-                                        id={"delete-" + x}
-                                        aria-label={x}
-                                        key={x}
-                                        label={x}
-                                        onChange={(_event, checked) => onValueChanged(x, checked)} />
-                        );
-                    })}
+                    {repoTags.length > 1 && <Checkbox isChecked={isAllSelected} id='delete-all' label={_("All")} aria-label='All'
+                        onChange={(_event, checked) => repoTags.forEach(item => onValueChanged(item, checked))}
+                        body={
+                            repoTags.map(x => (
+                                <Checkbox isChecked={checkedTags.indexOf(x) > -1}
+                                            id={"delete-" + x}
+                                            aria-label={x}
+                                            key={x}
+                                            label={x}
+                                            onChange={(_event, checked) => onValueChanged(x, checked)} />
+                            ))
+                        } />}
+                    {repoTags.length === 1 && <List><ListItem>{repoTags[0]}</ListItem></List>}
                 </StackItem>
             </Stack>
-            { repoTags.length > 2 && <Button isDisabled={repoTags.length === checkedTags.length} variant="link" onClick={pickAll}>{_("select all")}</Button> }
         </Modal>
     );
 };
diff --git a/src/podman.scss b/src/podman.scss
@@ -10,6 +10,10 @@
     @extend .ct-card;
 }
 
+.pf-v5-c-modal-box__title-text {
+    white-space: break-spaces;
+}
+
 #containers-images, #containers-containers {
     // Decrease padding for the image/container toggle button list
     .pf-v5-c-table.pf-m-compact .pf-v5-c-table__toggle {

diff --git a/test/check-application b/test/check-application
@@ -792,7 +792,7 @@ class TestApplication(testlib.MachineCase):
 
         b.click(busybox_sel + " .pf-v5-c-dropdown__toggle")
         b.click(busybox_sel + " button.btn-delete")
-        b.click("button:contains('select all')")
+        b.click("#delete-all")
         self.assertTrue(b.get_checked(f".pf-v5-c-check__input[aria-label='{IMG_BUSYBOX_LATEST}']"))
         self.assertTrue(b.get_checked(f".pf-v5-c-check__input[aria-label='{IMG_BUSYBOX}:2']"))
         self.assertTrue(b.get_checked(f".pf-v5-c-check__input[aria-label='{IMG_BUSYBOX}:4']"))
@@ -1010,7 +1010,7 @@ class TestApplication(testlib.MachineCase):
             # Untag busybox image which duplicates the image we are about to download
             self.execute(True, f"podman rmi -f {IMG_BUSYBOX} localhost:5000/my-busybox localhost:6000/my-busybox")
 
-        class DownloadImageDialog:
+        class DownloadImageDialog(testlib.MachineCase):
             def __init__(self, imageName, imageTag=None, user="system"):
                 self.imageName = imageName
                 self.imageTag = imageTag
@@ -1091,9 +1091,16 @@ class TestApplication(testlib.MachineCase):
                 b.click(sel + " .pf-v5-c-dropdown__toggle")
                 b.click(sel + ' button.btn-delete')
 
-                b.set_checked(f".pf-v5-c-check__input[aria-label='localhost:5000/{self.imageName}{imageTagSuffix}']",
-                              True)
                 if another:
+                    b.click("#delete-all")
+                    sel = f".pf-v5-c-check__input[aria-label='localhost:5000/{self.imageName}{imageTagSuffix}']"
+                    self.assertTrue(b.get_checked(sel))
+                    self.assertTrue(b.get_checked(f".pf-v5-c-check__input[aria-label='{another}']"))
+                    b.click("#delete-all")
+                    b.wait_visible("#btn-img-delete:disabled")
+
+                    b.set_checked(
+                        f".pf-v5-c-check__input[aria-label='localhost:5000/{self.imageName}{imageTagSuffix}']", True)
                     b.set_checked(f".pf-v5-c-check__input[aria-label='{another}']", True)
 
                 # Confirm deletion in the delete dialog