From b152b9f17b8da7c7c0fd78a844aa48e660f1398e Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Sun, 15 May 2022 15:40:49 +0100
Subject: [PATCH] fix: Possible extra separator in filtered context menus Todo:
 We need to combine this logic with the menus in the editor, but not today
 closes #3506

---
 app/components/ContextMenu/Template.tsx    | 44 +++++-----
 app/menus/CollectionMenu.tsx               | 99 ++++++++++------------
 shared/i18n/locales/en_US/translation.json |  4 +-
 3 files changed, 66 insertions(+), 81 deletions(-)

diff --git a/app/components/ContextMenu/Template.tsx b/app/components/ContextMenu/Template.tsx
index cd92a12bf..6cdbb5320 100644
--- a/app/components/ContextMenu/Template.tsx
+++ b/app/components/ContextMenu/Template.tsx
@@ -69,29 +69,27 @@ const Submenu = React.forwardRef(
 );
 
 export function filterTemplateItems(items: TMenuItem[]): TMenuItem[] {
-  let filtered = items.filter((item) => item.visible !== false);
-
-  // this block literally just trims unnecessary separators
-  filtered = filtered.reduce((acc, item, index) => {
-    // trim separators from start / end
-    if (item.type === "separator" && index === 0) {
-      return acc;
-    }
-    if (item.type === "separator" && index === filtered.length - 1) {
-      return acc;
-    }
-
-    // trim double separators looking ahead / behind
-    const prev = filtered[index - 1];
-    if (prev && prev.type === "separator" && item.type === "separator") {
-      return acc;
-    }
-
-    // otherwise, continue
-    return [...acc, item];
-  }, []);
-
-  return filtered;
+  return items
+    .filter((item) => item.visible !== false)
+    .reduce((acc, item) => {
+      // trim separator if the previous item was a separator
+      if (
+        item.type === "separator" &&
+        acc[acc.length - 1]?.type === "separator"
+      ) {
+        return acc;
+      }
+      return [...acc, item];
+    }, [] as TMenuItem[])
+    .filter((item, index, arr) => {
+      if (
+        item.type === "separator" &&
+        (index === 0 || index === arr.length - 1)
+      ) {
+        return false;
+      }
+      return true;
+    });
 }
 
 function Template({ items, actions, context, ...menu }: Props) {
diff --git a/app/menus/CollectionMenu.tsx b/app/menus/CollectionMenu.tsx
index e5ecb1239..eecd9bcc6 100644
--- a/app/menus/CollectionMenu.tsx
+++ b/app/menus/CollectionMenu.tsx
@@ -25,7 +25,6 @@ import CollectionPermissions from "~/scenes/CollectionPermissions";
 import ContextMenu, { Placement } from "~/components/ContextMenu";
 import OverflowMenuButton from "~/components/ContextMenu/OverflowMenuButton";
 import Template from "~/components/ContextMenu/Template";
-import Modal from "~/components/Modal";
 import useCurrentTeam from "~/hooks/useCurrentTeam";
 import usePolicy from "~/hooks/usePolicy";
 import useStores from "~/hooks/useStores";
@@ -54,27 +53,43 @@ function CollectionMenu({
     modal,
     placement,
   });
-  const [renderModals, setRenderModals] = React.useState(false);
   const team = useCurrentTeam();
   const { documents, dialogs } = useStores();
   const { showToast } = useToasts();
   const { t } = useTranslation();
   const history = useHistory();
   const file = React.useRef<HTMLInputElement>(null);
-  const [
-    showCollectionPermissions,
-    setShowCollectionPermissions,
-  ] = React.useState(false);
-  const [showCollectionEdit, setShowCollectionEdit] = React.useState(false);
-  const [showCollectionExport, setShowCollectionExport] = React.useState(false);
 
-  const handleOpen = React.useCallback(() => {
-    setRenderModals(true);
+  const handlePermissions = React.useCallback(() => {
+    dialogs.openModal({
+      title: t("Collection permissions"),
+      content: <CollectionPermissions collection={collection} />,
+    });
+  }, [collection, dialogs, t]);
 
-    if (onOpen) {
-      onOpen();
-    }
-  }, [onOpen]);
+  const handleEdit = React.useCallback(() => {
+    dialogs.openModal({
+      title: t("Edit collection"),
+      content: (
+        <CollectionEdit
+          collectionId={collection.id}
+          onSubmit={dialogs.closeAllModals}
+        />
+      ),
+    });
+  }, [collection.id, dialogs, t]);
+
+  const handleExport = React.useCallback(() => {
+    dialogs.openModal({
+      title: t("Export collection"),
+      content: (
+        <CollectionExport
+          collection={collection}
+          onSubmit={dialogs.closeAllModals}
+        />
+      ),
+    });
+  }, [collection, dialogs, t]);
 
   const handleNewDocument = React.useCallback(
     (ev: React.SyntheticEvent) => {
@@ -238,21 +253,21 @@ function CollectionMenu({
         type: "button",
         title: `${t("Edit")}…`,
         visible: can.update,
-        onClick: () => setShowCollectionEdit(true),
+        onClick: handleEdit,
         icon: <EditIcon />,
       },
       {
         type: "button",
         title: `${t("Permissions")}…`,
         visible: can.update,
-        onClick: () => setShowCollectionPermissions(true),
+        onClick: handlePermissions,
         icon: <PadlockIcon />,
       },
       {
         type: "button",
         title: `${t("Export")}…`,
         visible: !!(collection && canUserInTeam.export),
-        onClick: () => setShowCollectionExport(true),
+        onClick: handleExport,
         icon: <ExportIcon />,
       },
       {
@@ -269,19 +284,22 @@ function CollectionMenu({
     ],
     [
       t,
+      handleUnstar,
+      collection,
+      can.unstar,
+      can.star,
       can.update,
       can.delete,
-      can.star,
-      can.unstar,
       handleStar,
-      handleUnstar,
-      alphabeticalSort,
-      handleChangeSort,
       handleNewDocument,
       handleImportDocument,
-      handleDelete,
-      collection,
+      alphabeticalSort,
+      handleEdit,
+      handlePermissions,
       canUserInTeam.export,
+      handleExport,
+      handleDelete,
+      handleChangeSort,
     ]
   );
 
@@ -311,43 +329,12 @@ function CollectionMenu({
       )}
       <ContextMenu
         {...menu}
-        onOpen={handleOpen}
+        onOpen={onOpen}
         onClose={onClose}
         aria-label={t("Collection")}
       >
         <Template {...menu} items={items} />
       </ContextMenu>
-      {renderModals && (
-        <>
-          <Modal
-            title={t("Collection permissions")}
-            onRequestClose={() => setShowCollectionPermissions(false)}
-            isOpen={showCollectionPermissions}
-          >
-            <CollectionPermissions collection={collection} />
-          </Modal>
-          <Modal
-            title={t("Edit collection")}
-            isOpen={showCollectionEdit}
-            onRequestClose={() => setShowCollectionEdit(false)}
-          >
-            <CollectionEdit
-              onSubmit={() => setShowCollectionEdit(false)}
-              collectionId={collection.id}
-            />
-          </Modal>
-          <Modal
-            title={t("Export collection")}
-            isOpen={showCollectionExport}
-            onRequestClose={() => setShowCollectionExport(false)}
-          >
-            <CollectionExport
-              onSubmit={() => setShowCollectionExport(false)}
-              collection={collection}
-            />
-          </Modal>
-        </>
-      )}
     </>
   );
 }
diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json
index 023bfaf90..decbb0b1d 100644
--- a/shared/i18n/locales/en_US/translation.json
+++ b/shared/i18n/locales/en_US/translation.json
@@ -248,6 +248,8 @@
   "Path to document": "Path to document",
   "Group member options": "Group member options",
   "Remove": "Remove",
+  "Collection permissions": "Collection permissions",
+  "Export collection": "Export collection",
   "Delete collection": "Are you sure you want to delete this collection?",
   "Sort in sidebar": "Sort in sidebar",
   "Alphabetical sort": "Alphabetical sort",
@@ -255,8 +257,6 @@
   "Edit": "Edit",
   "Permissions": "Permissions",
   "Delete": "Delete",
-  "Collection permissions": "Collection permissions",
-  "Export collection": "Export collection",
   "Document restored": "Document restored",
   "Document unpublished": "Document unpublished",
   "Document options": "Document options",