From 8b46c44d272b1d920c032c0c54df7cbebff73a20 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 23 Jul 2024 08:08:04 +0800 Subject: [PATCH] [navigation-next] Fix issues. (#7356) * [navigation-next] fix: redirect to standard index pattern applications while nav group is enabled (#7305) * feat: fix the incorrect jumping logic for Index pattern management Signed-off-by: SuZhou-Joe * Changeset file for PR #7305 created/updated * feat: update Signed-off-by: SuZhou-Joe * feat: update with comment Signed-off-by: SuZhou-Joe * feat: update order and remove reset logic Signed-off-by: SuZhou-Joe * feat: update Signed-off-by: SuZhou-Joe * feat: update Signed-off-by: SuZhou-Joe * feat: update snapshot Signed-off-by: SuZhou-Joe * feat: some category change Signed-off-by: SuZhou-Joe * feat: update category Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> * fix: bugs found in integration test Signed-off-by: SuZhou-Joe * Changeset file for PR #7356 created/updated * feat: show workspace detail in all use case Signed-off-by: SuZhou-Joe * fix: unit test error Signed-off-by: SuZhou-Joe * feat: revert back detect category Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7356.yml | 2 ++ ...ollapsible_nav_group_enabled.test.tsx.snap | 29 ++++++++++++++++++- .../header/collapsible_nav_group_enabled.tsx | 22 ++++++++------ src/core/utils/default_app_categories.ts | 9 +++--- src/core/utils/default_nav_groups.ts | 2 +- .../advanced_settings/public/plugin.ts | 3 +- src/plugins/dev_tools/public/plugin.ts | 2 ++ src/plugins/management/public/plugin.ts | 3 ++ .../workspace_menu/workspace_menu.tsx | 2 +- src/plugins/workspace/public/plugin.test.ts | 29 ++++++++++++++++++- src/plugins/workspace/public/plugin.ts | 20 ++++++++++++- 11 files changed, 103 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/7356.yml diff --git a/changelogs/fragments/7356.yml b/changelogs/fragments/7356.yml new file mode 100644 index 000000000000..6c4b76d95ad9 --- /dev/null +++ b/changelogs/fragments/7356.yml @@ -0,0 +1,2 @@ +fix: +- [navigation-next] Fix issues. ([#7356](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7356)) \ No newline at end of file diff --git a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap index 56600b067583..1bd0458ef1b5 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap @@ -1,6 +1,33 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[` should hide left navigation when in home page when workspace is enabled 1`] = `
`; +exports[` should hide left navigation when in home page when workspace is enabled 1`] = ` +
+
+
+
+
+
+
+
+
+
+
+`; exports[` should render correctly 1`] = `
diff --git a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx index 68b031232370..c6669a6b99f8 100644 --- a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx +++ b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx @@ -177,7 +177,7 @@ export function CollapsibleNavGroupEnabled({ basePath, id, isLocked, - isNavOpen, + isNavOpen: isNavOpenProps, storage = window.localStorage, onIsLockedUpdate, closeNav, @@ -263,6 +263,18 @@ export function CollapsibleNavGroupEnabled({ return fulfillRegistrationLinksToChromeNavLinks(navLinksForAll, navLinks); }, [navLinks, navGroupsMap, currentNavGroup]); + const isNavOpen = useMemo(() => { + // For now, only home page need to always collapse left navigation + // when workspace is enabled. + // If there are more pages need to collapse left navigation in the future + // need to come up with a mechanism to register. + if (capabilities.workspaces.enabled && appId === 'home') { + return false; + } + + return isNavOpenProps; + }, [isNavOpenProps, capabilities.workspaces.enabled, appId]); + const width = useMemo(() => { if (!isNavOpen) { return 50; @@ -271,14 +283,6 @@ export function CollapsibleNavGroupEnabled({ return 270; }, [isNavOpen]); - // For now, only home page need to hide left navigation - // when workspace is enabled. - // If there are more pages need to hide left navigation in the future - // need to come up with a mechanism to register. - if (capabilities.workspaces.enabled && appId === 'home') { - return null; - } - const onGroupClick = ( e: React.MouseEvent, group: NavGroupItemInMap diff --git a/src/core/utils/default_app_categories.ts b/src/core/utils/default_app_categories.ts index dfcfdb128692..e68e1667e223 100644 --- a/src/core/utils/default_app_categories.ts +++ b/src/core/utils/default_app_categories.ts @@ -102,13 +102,12 @@ export const DEFAULT_APP_CATEGORIES: Record = Object.freeze }), order: 4000, }, - // TODO remove this default category detect: { - id: 'configure', - label: i18n.translate('core.ui.configure.label', { - defaultMessage: 'Configure', + id: 'detect', + label: i18n.translate('core.ui.detect.label', { + defaultMessage: 'Detect', }), - order: 2000, + order: 3000, }, configure: { id: 'configure', diff --git a/src/core/utils/default_nav_groups.ts b/src/core/utils/default_nav_groups.ts index da2e3c46f9fb..5e10ccbd8b4b 100644 --- a/src/core/utils/default_nav_groups.ts +++ b/src/core/utils/default_nav_groups.ts @@ -37,7 +37,7 @@ const defaultNavGroups = { defaultMessage: 'All use case', }), description: i18n.translate('core.ui.group.all.description', { - defaultMessage: 'This is a usse case contains all the features.', + defaultMessage: 'This is a use case contains all the features.', }), order: 3000, type: NavGroupType.SYSTEM, diff --git a/src/plugins/advanced_settings/public/plugin.ts b/src/plugins/advanced_settings/public/plugin.ts index 53ead80d8057..f2bf05da4114 100644 --- a/src/plugins/advanced_settings/public/plugin.ts +++ b/src/plugins/advanced_settings/public/plugin.ts @@ -34,7 +34,7 @@ import { FeatureCatalogueCategory } from '../../home/public'; import { ComponentRegistry } from './component_registry'; import { AdvancedSettingsSetup, AdvancedSettingsStart, AdvancedSettingsPluginSetup } from './types'; import { setupTopNavThemeButton } from './register_nav_control'; -import { DEFAULT_NAV_GROUPS, AppNavLinkStatus } from '../../../core/public'; +import { DEFAULT_NAV_GROUPS, AppNavLinkStatus, WorkspaceAvailability } from '../../../core/public'; const component = new ComponentRegistry(); @@ -69,6 +69,7 @@ export class AdvancedSettingsPlugin navLinkStatus: core.chrome.navGroup.getNavGroupEnabled() ? AppNavLinkStatus.visible : AppNavLinkStatus.hidden, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, mount: async (params: AppMountParameters) => { const { mountManagementSection } = await import( './management_app/mount_management_section' diff --git a/src/plugins/dev_tools/public/plugin.ts b/src/plugins/dev_tools/public/plugin.ts index 6355d769e66d..34bc74c8d518 100644 --- a/src/plugins/dev_tools/public/plugin.ts +++ b/src/plugins/dev_tools/public/plugin.ts @@ -49,6 +49,7 @@ import './index.scss'; import { ManagementOverViewPluginSetup } from '../../management_overview/public'; import { toMountPoint } from '../../opensearch_dashboards_react/public'; import { DevToolsIcon } from './dev_tools_icon'; +import { WorkspaceAvailability } from '../../../core/public'; export interface DevToolsSetupDependencies { urlForwarding: UrlForwardingSetup; @@ -97,6 +98,7 @@ export class DevToolsPlugin implements Plugin { /* the order of dev tools, it shows as last item of management section */ order: 9070, category: DEFAULT_APP_CATEGORIES.management, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, mount: async (params: AppMountParameters) => { const { element, history } = params; element.classList.add('devAppWrapper'); diff --git a/src/plugins/management/public/plugin.ts b/src/plugins/management/public/plugin.ts index c3a0f41bb56d..d376feeafe7e 100644 --- a/src/plugins/management/public/plugin.ts +++ b/src/plugins/management/public/plugin.ts @@ -45,6 +45,7 @@ import { AppStatus, AppNavLinkStatus, DEFAULT_NAV_GROUPS, + WorkspaceAvailability, } from '../../../core/public'; import { MANAGEMENT_APP_ID } from '../common/contants'; @@ -124,6 +125,7 @@ export class ManagementPlugin implements Plugin { const { renderApp } = await import('./landing_page_application'); const [coreStart] = await core.getStartServices(); @@ -156,6 +158,7 @@ export class ManagementPlugin implements Plugin { const { renderApp } = await import('./landing_page_application'); const [coreStart] = await core.getStartServices(); diff --git a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx index 77d1cd6e602e..05fe58d352a8 100644 --- a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx +++ b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx @@ -103,7 +103,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => { }; const currentWorkspaceButton = currentWorkspace ? ( - + { ); }); + it('#setup should register workspace detail with a visible application and register to all nav group', async () => { + const setupMock = coreMock.createSetup(); + setupMock.chrome.navGroup.getNavGroupEnabled.mockReturnValue(true); + const workspacePlugin = new WorkspacePlugin(); + await workspacePlugin.setup(setupMock, {}); + + expect(setupMock.application.register).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'workspace_detail', + navLinkStatus: AppNavLinkStatus.visible, + }) + ); + + expect(setupMock.chrome.navGroup.addNavLinksToGroup).toHaveBeenCalledWith( + DEFAULT_NAV_GROUPS.all, + expect.arrayContaining([ + { + id: 'workspace_detail', + title: 'Overview', + order: 100, + }, + ]) + ); + }); + it('#start add workspace detail page to breadcrumbs when start', async () => { const startMock = coreMock.createStart(); const workspaceObject = { @@ -264,6 +289,7 @@ describe('Workspace plugin', () => { title: 'Foo', features: ['system-feature'], systematic: true, + description: '', }, ]); jest.spyOn(UseCaseService.prototype, 'start').mockImplementationOnce(() => ({ @@ -286,7 +312,7 @@ describe('Workspace plugin', () => { const appUpdater = await appUpdater$.pipe(first()).toPromise(); - expect(appUpdater({ id: 'system-feature' })).toBeUndefined(); + expect(appUpdater({ id: 'system-feature', title: '', mount: () => () => {} })).toBeUndefined(); }); it('#start should update nav group status after currentWorkspace set', async () => { @@ -334,6 +360,7 @@ describe('Workspace plugin', () => { title: 'Foo', features: ['system-feature'], systematic: true, + description: '', }, ]); jest.spyOn(UseCaseService.prototype, 'start').mockImplementationOnce(() => ({ diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 1be221b546ad..23b6224dffe3 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -107,7 +107,13 @@ export class WorkspacePlugin this.registeredUseCases$, ]).subscribe(([currentWorkspace, registeredUseCases]) => { if (currentWorkspace) { + const isAllUseCase = + getFirstUseCaseOfFeatureConfigs(currentWorkspace.features || []) === ALL_USE_CASE_ID; this.appUpdater$.next((app) => { + // When in all workspace, the home should be replaced by workspace detail page + if (app.id === 'home' && isAllUseCase) { + return { navLinkStatus: AppNavLinkStatus.hidden }; + } if (isAppAccessibleInWorkspace(app, currentWorkspace, registeredUseCases)) { return; } @@ -327,7 +333,9 @@ export class WorkspacePlugin title: i18n.translate('workspace.settings.workspaceDetail', { defaultMessage: 'Workspace Detail', }), - navLinkStatus: AppNavLinkStatus.hidden, + navLinkStatus: core.chrome.navGroup.getNavGroupEnabled() + ? AppNavLinkStatus.visible + : AppNavLinkStatus.hidden, async mount(params: AppMountParameters) { const { renderDetailApp } = await import('./application'); return mountWorkspaceApp(params, renderDetailApp); @@ -353,6 +361,16 @@ export class WorkspacePlugin workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); + core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.all, [ + { + id: WORKSPACE_DETAIL_APP_ID, + order: 100, + title: i18n.translate('workspace.nav.workspaceDetail.title', { + defaultMessage: 'Overview', + }), + }, + ]); + core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.settingsAndSetup, [ { id: WORKSPACE_LIST_APP_ID,