From 9c766362ede151f201edd80206dafb7dbe54d269 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Mon, 4 Apr 2022 22:35:28 -0700
Subject: [PATCH] fix: Hanging separators in filtered block menu

---
 app/editor/components/CommandMenu.tsx         |  2 +-
 .../editor/lib/filterExcessSeparators.test.ts | 43 +++++++++++++++
 shared/editor/lib/filterExcessSeparators.ts   | 54 ++++++++-----------
 3 files changed, 66 insertions(+), 33 deletions(-)
 create mode 100644 shared/editor/lib/filterExcessSeparators.test.ts

diff --git a/app/editor/components/CommandMenu.tsx b/app/editor/components/CommandMenu.tsx
index 9b9a4e2bb..e275ea9d8 100644
--- a/app/editor/components/CommandMenu.tsx
+++ b/app/editor/components/CommandMenu.tsx
@@ -419,7 +419,7 @@ class CommandMenu<T = MenuItem> extends React.Component<Props<T>, State> {
       commands,
       filterable = true,
     } = this.props;
-    let items: (EmbedDescriptor | MenuItem)[] = this.props.items;
+    let items: (EmbedDescriptor | MenuItem)[] = [...this.props.items];
     const embedItems: EmbedDescriptor[] = [];
 
     for (const embed of embeds) {
diff --git a/shared/editor/lib/filterExcessSeparators.test.ts b/shared/editor/lib/filterExcessSeparators.test.ts
new file mode 100644
index 000000000..8099a92b5
--- /dev/null
+++ b/shared/editor/lib/filterExcessSeparators.test.ts
@@ -0,0 +1,43 @@
+import filterExcessSeparators from "./filterExcessSeparators";
+
+const embedDescriptor = {
+  icon: () => null,
+  matcher: () => true,
+  component: () => null,
+};
+
+describe("filterExcessSeparators", () => {
+  test("filter hanging end separators", () => {
+    expect(
+      filterExcessSeparators([
+        embedDescriptor,
+        { name: "separator" },
+        { name: "separator" },
+        { name: "separator" },
+        { name: "separator" },
+      ])
+    ).toStrictEqual([embedDescriptor]);
+  });
+
+  test("filter hanging start separators", () => {
+    expect(
+      filterExcessSeparators([
+        { name: "separator" },
+        { name: "separator" },
+        { name: "separator" },
+        { name: "separator" },
+        embedDescriptor,
+      ])
+    ).toStrictEqual([embedDescriptor]);
+  });
+
+  test("filter surrounding separators", () => {
+    expect(
+      filterExcessSeparators([
+        { name: "separator" },
+        embedDescriptor,
+        { name: "separator" },
+      ])
+    ).toStrictEqual([embedDescriptor]);
+  });
+});
diff --git a/shared/editor/lib/filterExcessSeparators.ts b/shared/editor/lib/filterExcessSeparators.ts
index 4288fb771..e599ccd9f 100644
--- a/shared/editor/lib/filterExcessSeparators.ts
+++ b/shared/editor/lib/filterExcessSeparators.ts
@@ -1,36 +1,26 @@
 import { EmbedDescriptor, MenuItem } from "../types";
 
-export default function filterExcessSeparators(
-  items: (MenuItem | EmbedDescriptor)[]
-): (MenuItem | EmbedDescriptor)[] {
-  return items.reduce((acc, item, index) => {
-    // trim separators from start / end
-    if (item.name === "separator" && index === 0) {
-      return acc;
-    }
-    if (item.name === "separator" && index === items.length - 1) {
-      return acc;
-    }
+type Item = MenuItem | EmbedDescriptor;
 
-    // trim double separators looking behind
-    const prev = items[index - 1];
-    if (prev && prev.name === "separator" && item.name === "separator") {
-      return acc;
-    }
-
-    // trim double separators looking ahead only if we're in the second to last
-    // position and the last position is also a separator (special case)
-    const next = items[index + 1];
-    if (
-      next &&
-      next.name === "separator" &&
-      item.name === "separator" &&
-      index === items.length - 2
-    ) {
-      return acc;
-    }
-
-    // otherwise, continue
-    return [...acc, item];
-  }, []);
+export default function filterExcessSeparators(items: Item[]): Item[] {
+  return items
+    .reduce((acc, item) => {
+      // trim separator if the previous item was a separator
+      if (
+        item.name === "separator" &&
+        acc[acc.length - 1]?.name === "separator"
+      ) {
+        return acc;
+      }
+      return [...acc, item];
+    }, [] as Item[])
+    .filter((item, index, arr) => {
+      if (
+        item.name === "separator" &&
+        (index === 0 || index === arr.length - 1)
+      ) {
+        return false;
+      }
+      return true;
+    });
 }