Skip to content

Commit

Permalink
[navigation-next] Fix issues. (#7356) (#7377) (#7379)
Browse files Browse the repository at this point in the history
* [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

* Changeset file for PR #7305 created/updated

* feat: update

* feat: update with comment

* feat: update order and remove reset logic

* feat: update

* feat: update

* feat: update snapshot

* feat: some category change

* feat: update category

---------

* fix: bugs found in integration test

* Changeset file for PR #7356 created/updated

* feat: show workspace detail in all use case

* fix: unit test error

* feat: revert back detect category

---------

(cherry picked from commit 8b46c44)

Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit 6551af7)

Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>
(cherry picked from commit 7adc980)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 5944bd7 commit 62bdc11
Show file tree
Hide file tree
Showing 11 changed files with 103 additions and 20 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/7356.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- [navigation-next] Fix issues. ([#7356](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7356))

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 13 additions & 9 deletions src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export function CollapsibleNavGroupEnabled({
basePath,
id,
isLocked,
isNavOpen,
isNavOpen: isNavOpenProps,
storage = window.localStorage,
onIsLockedUpdate,
closeNav,
Expand Down Expand Up @@ -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;
Expand All @@ -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<HTMLButtonElement, MouseEvent>,
group: NavGroupItemInMap
Expand Down
9 changes: 4 additions & 5 deletions src/core/utils/default_app_categories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,12 @@ export const DEFAULT_APP_CATEGORIES: Record<string, AppCategory> = 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',
Expand Down
2 changes: 1 addition & 1 deletion src/core/utils/default_nav_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/advanced_settings/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { AppMountParameters, CoreSetup, Plugin } from 'opensearch-dashboards/pub
import { FeatureCatalogueCategory } from '../../home/public';
import { ComponentRegistry } from './component_registry';
import { AdvancedSettingsSetup, AdvancedSettingsStart, AdvancedSettingsPluginSetup } from './types';
import { DEFAULT_NAV_GROUPS, AppNavLinkStatus } from '../../../core/public';
import { DEFAULT_NAV_GROUPS, AppNavLinkStatus, WorkspaceAvailability } from '../../../core/public';

const component = new ComponentRegistry();

Expand Down Expand Up @@ -68,6 +68,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'
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/dev_tools/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -97,6 +98,7 @@ export class DevToolsPlugin implements Plugin<DevToolsSetup> {
/* 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');
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/management/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
AppStatus,
AppNavLinkStatus,
DEFAULT_NAV_GROUPS,
WorkspaceAvailability,
} from '../../../core/public';

import { MANAGEMENT_APP_ID } from '../common/contants';
Expand Down Expand Up @@ -124,6 +125,7 @@ export class ManagementPlugin implements Plugin<ManagementSetup, ManagementStart
navLinkStatus: core.chrome.navGroup.getNavGroupEnabled()
? AppNavLinkStatus.visible
: AppNavLinkStatus.hidden,
workspaceAvailability: WorkspaceAvailability.outsideWorkspace,
mount: async (params: AppMountParameters) => {
const { renderApp } = await import('./landing_page_application');
const [coreStart] = await core.getStartServices();
Expand Down Expand Up @@ -156,6 +158,7 @@ export class ManagementPlugin implements Plugin<ManagementSetup, ManagementStart
navLinkStatus: core.chrome.navGroup.getNavGroupEnabled()
? AppNavLinkStatus.visible
: AppNavLinkStatus.hidden,
workspaceAvailability: WorkspaceAvailability.outsideWorkspace,
mount: async (params: AppMountParameters) => {
const { renderApp } = await import('./landing_page_application');
const [coreStart] = await core.getStartServices();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
};

const currentWorkspaceButton = currentWorkspace ? (
<EuiButtonEmpty flush="left" onClick={openPopover} data-test-subj="current-workspace-button">
<EuiButtonEmpty onClick={openPopover} data-test-subj="current-workspace-button">
<EuiAvatar
size="s"
type="space"
Expand Down
29 changes: 28 additions & 1 deletion src/plugins/workspace/public/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,31 @@ describe('Workspace plugin', () => {
);
});

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 = {
Expand Down Expand Up @@ -263,6 +288,7 @@ describe('Workspace plugin', () => {
title: 'Foo',
features: ['system-feature'],
systematic: true,
description: '',
},
]);
jest.spyOn(UseCaseService.prototype, 'start').mockImplementationOnce(() => ({
Expand All @@ -285,7 +311,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 () => {
Expand Down Expand Up @@ -333,6 +359,7 @@ describe('Workspace plugin', () => {
title: 'Foo',
features: ['system-feature'],
systematic: true,
description: '',
},
]);
jest.spyOn(UseCaseService.prototype, 'start').mockImplementationOnce(() => ({
Expand Down
20 changes: 19 additions & 1 deletion src/plugins/workspace/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Check warning on line 115 in src/plugins/workspace/public/plugin.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/workspace/public/plugin.ts#L115

Added line #L115 was not covered by tests
}
if (isAppAccessibleInWorkspace(app, currentWorkspace, registeredUseCases)) {
return;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down

0 comments on commit 62bdc11

Please sign in to comment.