enhance grouping for context menu options (#3924)

* enhance grouping for context menu options * build panels tests and more comments Signed-off-by: David Sinclair <dsincla@rei.com> --------- Signed-off-by: David Sinclair <david@sinclair.tech> Signed-off-by: David Sinclair <dsincla@rei.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com>
opensearch-project · May 24, 2023 · 1524784 · 1524784
1 parent 61ea841
commit 1524784
Show file tree

Hide file tree

Showing 6 changed files with 249 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -250,6 +250,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Multi DataSource] UX enhancement on Data source management stack ([#2521](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2521))
 - [Multi DataSource] UX enhancement on Update stored password modal for Data source management stack ([#2532](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2532))
 - [Monaco editor] Add json worker support ([#3424](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3424))
+- Enhance grouping for context menus ([#3169](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3169))
 - Replace re2 with RegExp in timeline and add unit tests ([#3908](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3908))
 
 ### 🐛 Bug Fixes

diff --git a/examples/ui_actions_explorer/public/context_menu_examples/context_menu_examples.tsx b/examples/ui_actions_explorer/public/context_menu_examples/context_menu_examples.tsx
@@ -44,6 +44,8 @@ export const ContextMenuExamples: React.FC = () => {
       <p>
         Below examples show how context menu panels look with varying number of actions and how the
         actions can be grouped into different panels using <EuiCode>grouping</EuiCode> field.
+        Grouping can only be one layer deep. A group needs to have at least two items for grouping
+        to work. A separator is automatically added between groups.
       </p>
 
       <EuiFlexGroup>

diff --git a/src/plugins/ui_actions/public/actions/action.ts b/src/plugins/ui_actions/public/actions/action.ts
@@ -92,7 +92,7 @@ export interface Action<Context extends BaseContext = {}, T = ActionType>
    * Returns a title to be displayed to the user.
    * @param context
    */
-  getDisplayName(context: ActionExecutionContext<Context>): string;
+  getDisplayName(context: ActionExecutionContext<Context>): JSX.Element | string;
 
   /**
    * `UiComponent` to render when displaying this action as a context menu item.

diff --git a/src/plugins/ui_actions/public/actions/action_internal.ts b/src/plugins/ui_actions/public/actions/action_internal.ts
@@ -59,7 +59,7 @@ export class ActionInternal<A extends ActionDefinition = ActionDefinition>
     return this.definition.getIconType(context);
   }
 
-  public getDisplayName(context: Context<A>): string {
+  public getDisplayName(context: Context<A>): JSX.Element | string {
     if (!this.definition.getDisplayName) return `Action: ${this.id}`;
     return this.definition.getDisplayName(context);
   }

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
@@ -36,20 +36,28 @@ const createTestAction = ({
   type,
   dispayName,
   order,
+  grouping,
 }: {
   type?: string;
   dispayName: string;
   order?: number;
+  grouping?: any[];
 }) =>
   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) => ({
+        ...(item.name ? { name: item.name } : {}),
+        ...(item.isSeparator ? { isSeparator: true } : {}),
+      }))
+    : [],
 });
 
 test('sorts items in DESC order by "order" field first, then by display name', async () => {
@@ -248,3 +256,195 @@ test('hides items behind in "More" submenu if there are more than 4 actions', as
     ]
   `);
 });
+
+test('flattening of group with only one action', async () => {
+  const grouping1 = [
+    {
+      id: 'test-group',
+      getDisplayName: () => 'Test group',
+      getIconType: () => 'bell',
+    },
+  ];
+  const actions = [
+    createTestAction({
+      dispayName: 'Foo 1',
+    }),
+    createTestAction({
+      dispayName: 'Bar 1',
+      grouping: grouping1,
+    }),
+  ];
+  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 {
+            "isSeparator": true,
+          },
+          Object {
+            "name": "Bar 1",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Bar 1",
+          },
+        ],
+      },
+    ]
+  `);
+});
+
+test('grouping with only two actions', async () => {
+  const grouping1 = [
+    {
+      id: 'test-group',
+      getDisplayName: () => 'Test group',
+      getIconType: () => 'bell',
+    },
+  ];
+  const actions = [
+    createTestAction({
+      dispayName: 'Foo 1',
+    }),
+    createTestAction({
+      dispayName: 'Bar 1',
+      grouping: grouping1,
+    }),
+    createTestAction({
+      dispayName: 'Bar 2',
+      grouping: grouping1,
+    }),
+  ];
+  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 {
+            "isSeparator": true,
+          },
+          Object {
+            "name": "Test group",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Bar 1",
+          },
+          Object {
+            "name": "Bar 2",
+          },
+        ],
+      },
+    ]
+  `);
+});
+
+test('groups with deep nesting', async () => {
+  const grouping1 = [
+    {
+      id: 'test-group',
+      getDisplayName: () => 'Test group',
+      getIconType: () => 'bell',
+    },
+  ];
+  const grouping2 = [
+    {
+      id: 'test-group-2',
+      getDisplayName: () => 'Test group 2',
+      getIconType: () => 'bell',
+    },
+    {
+      id: 'test-group-3',
+      getDisplayName: () => 'Test group 3',
+      getIconType: () => 'bell',
+    },
+  ];
+
+  const actions = [
+    createTestAction({
+      dispayName: 'Foo 1',
+    }),
+    createTestAction({
+      dispayName: 'Bar 1',
+      grouping: grouping1,
+    }),
+    createTestAction({
+      dispayName: 'Bar 2',
+      grouping: grouping1,
+    }),
+    createTestAction({
+      dispayName: 'Qux 1',
+      grouping: grouping2,
+    }),
+  ];
+  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 {
+            "isSeparator": true,
+          },
+          Object {
+            "name": "Test group",
+          },
+          Object {
+            "isSeparator": true,
+          },
+          Object {
+            "name": "Test group 3",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Bar 1",
+          },
+          Object {
+            "name": "Bar 2",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Test group 3",
+          },
+        ],
+      },
+      Object {
+        "items": Array [
+          Object {
+            "name": "Qux 1",
+          },
+        ],
+      },
+    ]
+  `);
+});
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
@@ -146,6 +146,7 @@ export async function buildContextMenuForActions({
   closeMenu = () => {},
 }: BuildContextMenuParams): Promise<EuiContextMenuPanelDescriptor[]> {
   const panels: Record<string, PanelDescriptor> = {
+    // This is the first panel which links out to all others via items property
     mainMenu: {
       id: 'mainMenu',
       title,
@@ -157,35 +158,51 @@ export async function buildContextMenuForActions({
     const context: ActionExecutionContext<object> = { ...item.context, trigger: item.trigger };
     const isCompatible = await item.action.isCompatible(context);
     if (!isCompatible) return;
-    let parentPanel = '';
-    let currentPanel = '';
+
+    // Reference to the last/parent/upper group.
+    // Groups are provided in order of parent to children.
+    let parentGroupId = '';
+
     if (action.grouping) {
       for (let i = 0; i < action.grouping.length; i++) {
         const group = action.grouping[i];
-        currentPanel = group.id;
-        if (!panels[currentPanel]) {
+        const groupId = group.id;
+
+        // If a panel does not exist for the current group, then create it
+        if (!panels[groupId]) {
           const name = group.getDisplayName ? group.getDisplayName(context) : group.id;
-          panels[currentPanel] = {
-            id: currentPanel,
+
+          // Create panel for group
+          panels[groupId] = {
+            id: groupId,
             title: name,
             items: [],
             _level: i,
             _icon: group.getIconType ? group.getIconType(context) : 'empty',
           };
-          if (parentPanel) {
-            panels[parentPanel].items!.push({
+
+          // If there are multiple groups and this is not the first group,
+          // then add an item to the parent group relating to this group
+          if (parentGroupId) {
+            panels[parentGroupId].items!.push({
               name,
-              panel: currentPanel,
+              panel: groupId,
               icon: group.getIconType ? group.getIconType(context) : 'empty',
               _order: group.order || 0,
               _title: group.getDisplayName ? group.getDisplayName(context) : '',
             });
           }
         }
-        parentPanel = currentPanel;
+
+        // Save the current group, because it will be used as the parent group
+        // for adding items to it for any additional groups in the array
+        parentGroupId = groupId;
       }
     }
-    panels[parentPanel || 'mainMenu'].items!.push({
+
+    // Add a context menu item for this action so it shows up on a context menu panel.
+    // We add this within the parent group or default to the mainMenu panel.
+    panels[parentGroupId || 'mainMenu'].items!.push({
       name: action.MenuItem
         ? React.createElement(uiToReactComponent(action.MenuItem), { context })
         : action.getDisplayName(context),
@@ -197,8 +214,10 @@ export async function buildContextMenuForActions({
       _title: action.getDisplayName(context),
     });
   });
+
   await Promise.all(promises);
 
+  // For each panel, sort items by order and title
   for (const panel of Object.values(panels)) {
     const items = panel.items.filter(Boolean) as ItemDescriptor[];
     panel.items = _.sortBy(
@@ -208,13 +227,23 @@ export async function buildContextMenuForActions({
     );
   }
 
+  // On the mainMenu, before adding in items for other groups, the first 4 items are shown.
+  // Any additional items are hidden behind a "more" item
   wrapMainPanelItemsIntoSubmenu(panels, 'mainMenu');
 
   for (const panel of Object.values(panels)) {
+    // If the panel is a root-level panel, such as the parent of a group,
+    // then create mainMenu item for this panel
     if (panel._level === 0) {
-      // TODO: Add separator line here once it is available in EUI.
-      // See https://github.com/elastic/eui/pull/4018
-      if (panel.items.length > 3) {
+      // Add separator with unique key if needed
+      if (panels.mainMenu.items.length) {
+        panels.mainMenu.items.push({ isSeparator: true, key: `${panel.id}separator` });
+      }
+
+      // If a panel has more than one child, then allow items to be grouped
+      // and link to it in the mainMenu. Otherwise, flatten the group.
+      // Note: this only happens on the root level panels, not for inner groups.
+      if (panel.items.length > 1) {
         panels.mainMenu.items.push({
           name: panel.title || panel.id,
           icon: panel._icon || 'empty',