From 1524784ee403712c9db99b01610e147fe685eb07 Mon Sep 17 00:00:00 2001 From: David Sinclair <24573542+sikhote@users.noreply.github.com> Date: Wed, 24 May 2023 15:03:53 -0700 Subject: [PATCH] enhance grouping for context menu options (#3924) * enhance grouping for context menu options * build panels tests and more comments Signed-off-by: David Sinclair --------- Signed-off-by: David Sinclair Signed-off-by: David Sinclair Signed-off-by: Josh Romero Co-authored-by: Josh Romero --- CHANGELOG.md | 1 + .../context_menu_examples.tsx | 2 + .../ui_actions/public/actions/action.ts | 2 +- .../public/actions/action_internal.ts | 2 +- .../build_eui_context_menu_panels.test.ts | 202 +++++++++++++++++- .../build_eui_context_menu_panels.tsx | 57 +++-- 6 files changed, 249 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfa5aebd2ab..2bb0133dfe6 100644 --- 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 index d38d80d0fac..b01d04c1608 100644 --- 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 = () => {

Below examples show how context menu panels look with varying number of actions and how the actions can be grouped into different panels using grouping 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.

diff --git a/src/plugins/ui_actions/public/actions/action.ts b/src/plugins/ui_actions/public/actions/action.ts index b38e2bd4e40..76fc2ddb3be 100644 --- 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 * Returns a title to be displayed to the user. * @param context */ - getDisplayName(context: ActionExecutionContext): string; + getDisplayName(context: ActionExecutionContext): 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 index fb39a01ed69..5610051dc3f 100644 --- 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 return this.definition.getIconType(context); } - public getDisplayName(context: Context): string { + public getDisplayName(context: Context): 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 index 949595d6093..b9afca9fb99 100644 --- 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 index 054900f52b7..6d69be1f3fa 100644 --- 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 { const panels: Record = { + // 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 = { ...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',