fix: 🐛 don't add separator befor group on no main items (elastic#83166)

Co-authored-by: Kibana Machine <[email protected]>
gmmorris · Dec 3, 2020 · bb76989 · bb76989
1 parent 37e9070
commit bb76989
Show file tree

Hide file tree

Showing 2 changed files with 207 additions and 5 deletions.
diff --git a/src/plugins/ui_actions/public/context_menu/build_eui_context_menu_panels.test.ts b/src/plugins/ui_actions/public/context_menu/build_eui_context_menu_panels.test.ts
@@ -20,25 +20,31 @@
 import { EuiContextMenuPanelDescriptor } from '@elastic/eui';
 import { buildContextMenuForActions } from './build_eui_context_menu_panels';
 import { Action, createAction } from '../actions';
+import { PresentableGrouping } from '../util';
 
 const createTestAction = ({
   type,
   dispayName,
   order,
+  grouping = undefined,
 }: {
   type?: string;
   dispayName: string;
   order?: number;
+  grouping?: PresentableGrouping;
 }) =>
   createAction({
     type: type as any, // mapping doesn't matter for this test
     getDisplayName: () => dispayName,
     order,
     execute: async () => {},
+    grouping,
   });
 
 const resultMapper = (panel: EuiContextMenuPanelDescriptor) => ({
-  items: panel.items ? panel.items.map((item) => ({ name: item.name })) : [],
+  items: panel.items
+    ? panel.items.map((item) => ({ name: item.isSeparator ? 'SEPARATOR' : item.name }))
+    : [],
 });
 
 test('sorts items in DESC order by "order" field first, then by display name', async () => {
@@ -237,3 +243,197 @@ test('hides items behind in "More" submenu if there are more than 4 actions', as
     ]
   `);
 });
+
+test('separates grouped items from main items with a separator', async () => {
+  const actions = [
+    createTestAction({
+      dispayName: 'Foo 1',
+    }),
+    createTestAction({
+      dispayName: 'Foo 2',
+    }),
+    createTestAction({
+      dispayName: 'Foo 3',
+    }),
+    createTestAction({
+      dispayName: 'Foo 4',
+      grouping: [
+        {
+          id: 'testGroup',
+          getDisplayName: () => 'Test group',
+        },
+      ],
+    }),
+  ];
+  const menu = await buildContextMenuForActions({
+    actions: actions.map((action) => ({ action, context: {}, trigger: 'TEST' as any })),
+  });
+
+  expect(menu.map(resultMapper)).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 1",
+          },
+          Object {
+            "name": "Foo 2",
+          },
+          Object {
+            "name": "Foo 3",
+          },
+          Object {
+            "name": "SEPARATOR",
+          },
+          Object {
+            "name": "Foo 4",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 4",
+          },
+        ],
+      },
+    ]
+  `);
+});
+
+test('separates multiple groups each with its own separator', async () => {
+  const actions = [
+    createTestAction({
+      dispayName: 'Foo 1',
+    }),
+    createTestAction({
+      dispayName: 'Foo 2',
+    }),
+    createTestAction({
+      dispayName: 'Foo 3',
+    }),
+    createTestAction({
+      dispayName: 'Foo 4',
+      grouping: [
+        {
+          id: 'testGroup',
+          getDisplayName: () => 'Test group',
+        },
+      ],
+    }),
+    createTestAction({
+      dispayName: 'Foo 5',
+      grouping: [
+        {
+          id: 'testGroup2',
+          getDisplayName: () => 'Test group 2',
+        },
+      ],
+    }),
+  ];
+  const menu = await buildContextMenuForActions({
+    actions: actions.map((action) => ({ action, context: {}, trigger: 'TEST' as any })),
+  });
+
+  expect(menu.map(resultMapper)).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 1",
+          },
+          Object {
+            "name": "Foo 2",
+          },
+          Object {
+            "name": "Foo 3",
+          },
+          Object {
+            "name": "SEPARATOR",
+          },
+          Object {
+            "name": "Foo 4",
+          },
+          Object {
+            "name": "SEPARATOR",
+          },
+          Object {
+            "name": "Foo 5",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 4",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 5",
+          },
+        ],
+      },
+    ]
+  `);
+});
+
+test('does not add separator for first grouping if there are no main items', async () => {
+  const actions = [
+    createTestAction({
+      dispayName: 'Foo 4',
+      grouping: [
+        {
+          id: 'testGroup',
+          getDisplayName: () => 'Test group',
+        },
+      ],
+    }),
+    createTestAction({
+      dispayName: 'Foo 5',
+      grouping: [
+        {
+          id: 'testGroup2',
+          getDisplayName: () => 'Test group 2',
+        },
+      ],
+    }),
+  ];
+  const menu = await buildContextMenuForActions({
+    actions: actions.map((action) => ({ action, context: {}, trigger: 'TEST' as any })),
+  });
+
+  expect(menu.map(resultMapper)).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 4",
+          },
+          Object {
+            "name": "SEPARATOR",
+          },
+          Object {
+            "name": "Foo 5",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 4",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Foo 5",
+          },
+        ],
+      },
+    ]
+  `);
+});
diff --git a/src/plugins/ui_actions/public/context_menu/build_eui_context_menu_panels.tsx b/src/plugins/ui_actions/public/context_menu/build_eui_context_menu_panels.tsx
@@ -201,10 +201,12 @@ export async function buildContextMenuForActions({
 
   for (const panel of Object.values(panels)) {
     if (panel._level === 0) {
-      panels.mainMenu.items.push({
-        isSeparator: true,
-        key: panel.id + '__separator',
-      });
+      if (panels.mainMenu.items.length > 0) {
+        panels.mainMenu.items.push({
+          isSeparator: true,
+          key: panel.id + '__separator',
+        });
+      }
       if (panel.items.length > 3) {
         panels.mainMenu.items.push({
           name: panel.title || panel.id,