Drop redundant back/forward buttons

The path breadcrumb has back/forward buttons to its left which are essentially a redundant reimplementation of the browser back/forward buttons. Let's drop that. Dropping these will also clean up a not inconsiderable amount of the amount of state we're passing around between components.
allisonkarlitskaya · Jan 31, 2024 · 21b9a3f · 21b9a3f
1 parent b0cae7f
commit 21b9a3f
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 105 deletions.
diff --git a/src/apis/spawnHelpers.jsx b/src/apis/spawnHelpers.jsx
@@ -30,7 +30,7 @@ const renameCommand = ({ selected, path, name }) => {
         : ["mv", path.join("/") + "/" + selected.name, path.join("/") + "/" + name];
 };
 
-export const spawnDeleteItem = ({ path, selected, itemPath, Dialogs, setSelected, setHistory, setHistoryIndex }) => {
+export const spawnDeleteItem = ({ path, selected, itemPath, Dialogs, setSelected }) => {
     cockpit.spawn([
         "rm",
         "-r",
@@ -44,8 +44,6 @@ export const spawnDeleteItem = ({ path, selected, itemPath, Dialogs, setSelected
                     const newPath = path.slice(0, -1).join("/");
 
                     cockpit.location.go("/", { path: encodeURIComponent(newPath) });
-                    setHistory(h => h.slice(0, -1));
-                    setHistoryIndex(i => i - 1);
                 }
             })
             .finally(Dialogs.close)
@@ -73,7 +71,7 @@ export const spawnForceDelete = ({ selected, path, itemPath, Dialogs, setDeleteF
             });
 };
 
-export const spawnRenameItem = ({ selected, name, path, Dialogs, setErrorMessage, setHistory, setHistoryIndex }) => {
+export const spawnRenameItem = ({ selected, name, path, Dialogs, setErrorMessage }) => {
     const newPath = selected.items_cnt
         ? path.slice(0, -1).join("/") + "/" + name
         : path.join("/") + "/" + name;
@@ -82,8 +80,6 @@ export const spawnRenameItem = ({ selected, name, path, Dialogs, setErrorMessage
             .then(() => {
                 if (selected.items_cnt) {
                     cockpit.location.go("/", { path: encodeURIComponent(newPath) });
-                    setHistory(h => h.slice(0, -1));
-                    setHistoryIndex(i => i - 1);
                 }
                 Dialogs.close();
             }, err => setErrorMessage(err.message));

diff --git a/src/app.jsx b/src/app.jsx
@@ -60,8 +60,6 @@ export const Application = () => {
     const [selected, setSelected] = useState([]);
     const [selectedContext, setSelectedContext] = useState(null);
     const [showHidden, setShowHidden] = useState(localStorage.getItem("cockpit-navigator.showHiddenFiles") === "true");
-    const [history, setHistory] = useState([]);
-    const [historyIndex, setHistoryIndex] = useState(0);
     const [clipboard, setClipboard] = useState(undefined);
     const [alerts, setAlerts] = useState([]);
 
@@ -78,7 +76,6 @@ export const Application = () => {
     useEffect(() => {
         cockpit.user().then(user => {
             const userPath = user.home.split("/");
-            setHistory(h => [...h, userPath]);
 
             if (options.path === undefined) {
                 cockpit.location.go("/", { path: encodeURIComponent(user.home) });
@@ -128,16 +125,14 @@ export const Application = () => {
             : [currentDir + selectedContext.name]);
     };
     const _pasteItem = (targetPath, asSymlink) => pasteItem(clipboard, targetPath.join("/") + "/", asSymlink, addAlert);
-    const _renameItem = () => renameItem(Dialogs, { selected: selectedContext, path, setHistory, setHistoryIndex });
+    const _renameItem = () => renameItem(Dialogs, { selected: selectedContext, path });
     const _editPermissions = () => editPermissions(Dialogs, { selected: selectedContext || rootInfo, path });
     const _deleteItem = () => {
         deleteItem(
             Dialogs,
             {
                 selected,
                 itemPath: currentDir + selectedContext.name,
-                setHistory,
-                setHistoryIndex,
                 path: currentDir,
                 setSelected
             }
@@ -206,8 +201,6 @@ export const Application = () => {
             <NavigatorBreadcrumbs
               path={path}
               currentDir={currentDir}
-              setHistory={setHistory} history={history}
-              historyIndex={historyIndex} setHistoryIndex={setHistoryIndex}
             />
             <PageSection onContextMenu={() => {
                 setSelectedContext(null);
@@ -229,8 +222,7 @@ export const Application = () => {
                               currentDir={currentDir}
                               isGrid={isGrid} sortBy={sortBy}
                               selected={selected} setSelected={setSelected}
-                              setSelectedContext={setSelectedContext} setHistory={setHistory}
-                              setHistoryIndex={setHistoryIndex} historyIndex={historyIndex}
+                              setSelectedContext={setSelectedContext}
                               loadingFiles={loadingFiles}
                             />
                             {!loadingFiles &&
@@ -270,7 +262,6 @@ export const Application = () => {
                                   }
                               }
                           }
-                          setHistory={setHistory} setHistoryIndex={setHistoryIndex}
                           showHidden={showHidden} setSelected={setSelected}
                           setShowHidden={setShowHidden}
                           clipboard={clipboard} setClipboard={setClipboard}

diff --git a/src/fileActions.jsx b/src/fileActions.jsx
@@ -74,7 +74,6 @@ export const deleteItem = (Dialogs, options) => {
         <ConfirmDeletionDialog
           selected={options.selected} itemPath={options.itemPath}
           path={options.path} setSelected={options.setSelected}
-          setHistory={options.setHistory} setHistoryIndex={options.setHistoryIndex}
           currentDirectory={options.currentDirectory}
         />
     );
@@ -85,7 +84,6 @@ export const renameItem = (Dialogs, options) => {
         <RenameItemModal
           path={options.path}
           selected={options.selected}
-          setHistory={options.setHistory} setHistoryIndex={options.setHistoryIndex}
         />
     );
 };
@@ -110,8 +108,6 @@ export const ConfirmDeletionDialog = ({
     itemPath,
     path,
     selected,
-    setHistory,
-    setHistoryIndex,
     setSelected,
     currentDirectory
 }) => {
@@ -136,7 +132,7 @@ export const ConfirmDeletionDialog = ({
     }
 
     const deleteItem = () => {
-        spawnDeleteItem({ Dialogs, selected, itemPath, path, setHistory, setHistoryIndex, setSelected });
+        spawnDeleteItem({ Dialogs, selected, itemPath, path, setSelected });
     };
 
     return (
@@ -252,7 +248,7 @@ export const CreateDirectoryModal = ({ selected, currentPath }) => {
     );
 };
 
-export const RenameItemModal = ({ path, selected, setHistory, setHistoryIndex }) => {
+export const RenameItemModal = ({ path, selected }) => {
     const Dialogs = useDialogs();
     const [name, setName] = useState(selected.name);
     const [errorMessage, setErrorMessage] = useState(undefined);
@@ -269,7 +265,7 @@ export const RenameItemModal = ({ path, selected, setHistory, setHistoryIndex })
     }
 
     const renameItem = () => {
-        spawnRenameItem({ Dialogs, path, selected, name, setErrorMessage, setHistory, setHistoryIndex });
+        spawnRenameItem({ Dialogs, path, selected, name, setErrorMessage });
     };
 
     return (

diff --git a/src/navigator-card-body.jsx b/src/navigator-card-body.jsx
@@ -78,12 +78,9 @@ const compare = (sortBy) => {
 export const NavigatorCardBody = ({
     currentFilter,
     files,
-    historyIndex,
     isGrid,
     path,
     selected,
-    setHistory,
-    setHistoryIndex,
     setSelected,
     setSelectedContext,
     sortBy,
@@ -116,16 +113,10 @@ export const NavigatorCardBody = ({
     const onDoubleClickNavigate = useCallback((file) => {
         const newPath = [...path, file.name].join("/");
         if (file.type === "dir" || file.to === "dir") {
-            setHistory(h => [...h.slice(0, historyIndex + 1), [...path, file.name]]);
-            setHistoryIndex(h => h + 1);
-
             cockpit.location.go("/", { path: encodeURIComponent(newPath) });
         }
     }, [
         path,
-        historyIndex,
-        setHistoryIndex,
-        setHistory
     ]);
 
     useEffect(() => {

diff --git a/src/navigatorBreadcrumbs.jsx b/src/navigatorBreadcrumbs.jsx
@@ -19,68 +19,31 @@
 import cockpit from "cockpit";
 import React from "react";
 
-import { Button, Flex, FlexItem, PageBreadcrumb } from "@patternfly/react-core";
-import { ArrowLeftIcon, ArrowRightIcon } from "@patternfly/react-icons";
-
-export const NavigatorBreadcrumbs = ({ currentDir, path, history, setHistory, historyIndex, setHistoryIndex }) => {
-    const navigateBack = () => {
-        if (historyIndex > 0) {
-            cockpit.location.go("/", { path: encodeURIComponent(history[historyIndex - 1].join("/")) });
-            setHistoryIndex(i => i - 1);
-        }
-    };
-
-    const navigateForward = () => {
-        if (historyIndex < history.length) {
-            cockpit.location.go("/", { path: encodeURIComponent(history[historyIndex + 1].join("/")) });
-            setHistoryIndex(i => i + 1);
-        }
-    };
+import { Button, Flex, PageBreadcrumb } from "@patternfly/react-core";
 
+export const NavigatorBreadcrumbs = ({ currentDir, path }) => {
     const navigateBreadcrumb = (i) => {
-        setHistory(h => [...h.slice(0, historyIndex + 1), path.slice(0, i)]);
-        setHistoryIndex(i => i + 1);
         cockpit.location.go("/", { path: encodeURIComponent(path.slice(0, i).join("/")) });
     };
 
     return (
         <PageBreadcrumb stickyOnBreakpoint={{ default: "top" }}>
-            <Flex>
-                <FlexItem>
-                    <Button
-                      variant="secondary" onClick={navigateBack}
-                      isDisabled={historyIndex === 0} id="navigate-back"
-                    >
-                        <ArrowLeftIcon />
-                    </Button>
-                </FlexItem>
-                <FlexItem>
-                    <Button
-                      variant="secondary" onClick={navigateForward}
-                      isDisabled={history.length === historyIndex + 1} id="navigate-forward"
-                    >
-                        <ArrowRightIcon />
-                    </Button>
-                </FlexItem>
-                <FlexItem>
-                    <Flex spaceItems={{ default: "spaceItemsXs" }}>
-                        {path.map((dir, i) => {
-                            return (
-                                <React.Fragment key={dir || "/"}>
-                                    {i !== path.length - 1 &&
-                                        <Button
-                                          variant="link" onClick={() => { navigateBreadcrumb(i + 1) }}
-                                          key={dir} className="breadcrumb-button"
-                                        >
-                                            {dir || "/"}
-                                        </Button>}
-                                    {i === path.length - 1 && <p className="last-breadcrumb-button">{dir || "/"}</p>}
-                                    {dir !== "" && <p key={i}>/</p>}
-                                </React.Fragment>
-                            );
-                        })}
-                    </Flex>
-                </FlexItem>
+            <Flex spaceItems={{ default: "spaceItemsXs" }}>
+                {path.map((dir, i) => {
+                    return (
+                        <React.Fragment key={dir || "/"}>
+                            {i !== path.length - 1 &&
+                                <Button
+                                  variant="link" onClick={() => { navigateBreadcrumb(i + 1) }}
+                                  key={dir} className="breadcrumb-button"
+                                >
+                                    {dir || "/"}
+                                </Button>}
+                            {i === path.length - 1 && <p className="last-breadcrumb-button">{dir || "/"}</p>}
+                            {dir !== "" && <p key={i}>/</p>}
+                        </React.Fragment>
+                    );
+                })}
             </Flex>
         </PageBreadcrumb>
     );

diff --git a/src/sidebar.jsx b/src/sidebar.jsx
@@ -113,8 +113,6 @@ export const SidebarPanelDetails = ({
     files,
     path,
     selected,
-    setHistory,
-    setHistoryIndex,
     setPath,
     setShowHidden,
     showHidden,
@@ -171,8 +169,7 @@ export const SidebarPanelDetails = ({
                 <DropdownWithKebab
                   selected={selected} path={path}
                   setPath={setPath} showHidden={showHidden}
-                  setShowHidden={setShowHidden} setHistory={setHistory}
-                  setHistoryIndex={setHistoryIndex} files={files}
+                  setShowHidden={setShowHidden} files={files}
                   clipboard={clipboard} setClipboard={setClipboard}
                   setSelected={setSelected} currentDirectory={currentDirectory}
                   addAlert={addAlert}
@@ -207,8 +204,6 @@ const DropdownWithKebab = ({
     path,
     showHidden,
     setShowHidden,
-    setHistory,
-    setHistoryIndex,
     files,
     clipboard,
     setClipboard,
@@ -328,7 +323,7 @@ const DropdownWithKebab = ({
         {
             id: "rename-item",
             onClick: () => {
-                renameItem(Dialogs, { selected: selected[0] || currentDirectory, path, setHistory, setHistoryIndex });
+                renameItem(Dialogs, { selected: selected[0] || currentDirectory, path });
             },
             title: _("Rename")
         },
@@ -342,8 +337,6 @@ const DropdownWithKebab = ({
                         ? ""
                         : selected[0].name),
                     path,
-                    setHistory,
-                    setHistoryIndex,
                     setSelected,
                     currentDirectory
                 });
@@ -365,8 +358,6 @@ const DropdownWithKebab = ({
                 deleteItem(Dialogs, {
                     selected,
                     path: currentPath,
-                    setHistory,
-                    setHistoryIndex,
                     setSelected
                 });
             },

diff --git a/test/check-application b/test/check-application
@@ -224,25 +224,25 @@ class TestNavigator(testlib.MachineCase):
         b.wait_text(".last-breadcrumb-button", "home")
         b.wait_visible("[data-item='admin']")
         # navigate back
-        b.click("#navigate-back")
+        b.eval_js("window.history.back()")
         b.wait_in_text("#sidebar-card-header", "newdir2")
         b.wait_not_present("[data-item='admin']")
-        b.click("#navigate-back")
+        b.eval_js("window.history.back()")
         b.wait_in_text("#sidebar-card-header", "newdir")
         b.wait_visible("[data-item='newdir2']")
-        b.click("#navigate-back")
+        b.eval_js("window.history.back()")
         b.wait_in_text("#sidebar-card-header", "admin")
         b.wait_visible("[data-item='newdir']")
         # navigate forward
-        b.click("#navigate-forward")
+        b.eval_js("window.history.forward()")
         b.wait_in_text("#sidebar-card-header", "newdir")
         b.wait_not_present("[data-item='admin']")
         b.wait_text(".last-breadcrumb-button", "newdir")
-        b.click("#navigate-forward")
+        b.eval_js("window.history.forward()")
         b.wait_in_text("#sidebar-card-header", "newdir2")
         b.wait_not_present("[data-item='newdir']")
         b.wait_text(".last-breadcrumb-button", "newdir2")
-        b.click("#navigate-forward")
+        b.eval_js("window.history.forward()")
         b.click("#navigator-card-body")  # unselect
         b.wait_in_text("#sidebar-card-header", "home")
         b.wait_not_present("[data-item='newdir']")