Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[navigation-next] Fix issues #7377

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

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 @@ -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();

Expand Down Expand Up @@ -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'
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 @@ -106,7 +106,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 @@ -177,6 +177,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 @@ -264,6 +289,7 @@ describe('Workspace plugin', () => {
title: 'Foo',
features: ['system-feature'],
systematic: true,
description: '',
},
]);
jest.spyOn(UseCaseService.prototype, 'start').mockImplementationOnce(() => ({
Expand All @@ -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 () => {
Expand Down Expand Up @@ -334,6 +360,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 @@
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 @@
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 @@
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
Loading