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

[Backport 2.x] [navigation]Refactor: flatten left nav in Analytics(all) use case #8466

Merged
merged 1 commit into from
Oct 4, 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
3 changes: 3 additions & 0 deletions changelogs/fragments/8332.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
feat:
- [navigation] flatten left nav in Analytics(all) use case ([#8332](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8332))
- [navigation] Adjust the appearances of the left navigation menu and the landing page ([#8332](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8332))
103 changes: 60 additions & 43 deletions src/core/public/chrome/nav_group/nav_group_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,66 @@ describe('ChromeNavGroupService#start()', () => {
expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1);
});

it('should populate links with custom category if the nav link is inside second level but no entry in all use case', async () => {
const chromeNavGroupService = new ChromeNavGroupService();
const uiSettings = uiSettingsServiceMock.createSetupContract();
const chromeNavGroupServiceSetup = chromeNavGroupService.setup({ uiSettings });

chromeNavGroupServiceSetup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.all, [
{
id: 'foo',
},
]);
chromeNavGroupServiceSetup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.essentials, [
{
id: 'bar',
title: 'bar',
},
{
id: 'foo',
title: 'foo',
},
]);
const navLinkServiceStart = mockedNavLink.start({
http: mockedHttpService,
application: mockedApplicationService,
});
navLinkServiceStart.getNavLinks$ = jest.fn().mockReturnValue(
new Rx.BehaviorSubject([
{
id: 'foo',
},
{
id: 'bar',
},
{
id: 'customized_app',
},
])
);
const chromeStart = await chromeNavGroupService.start({
navLinks: navLinkServiceStart,
application: mockedApplicationService,
breadcrumbsEnricher$: new Rx.BehaviorSubject<ChromeBreadcrumbEnricher | undefined>(undefined),
workspaces: workspacesServiceMock.createStartContract(),
});
const groupsMap = await chromeStart.getNavGroupsMap$().pipe(first()).toPromise();
expect(groupsMap[ALL_USE_CASE_ID].navLinks).toEqual([
{
id: 'foo',
},
{
id: 'bar',
title: 'bar',
category: { id: 'custom', label: 'Custom', order: 8500 },
},
{
id: 'customized_app',
category: { id: 'custom', label: 'Custom', order: 8500 },
},
]);
});

it('should return navGroupEnabled from ui settings', async () => {
const chromeNavGroupService = new ChromeNavGroupService();
const uiSettings = uiSettingsServiceMock.createSetupContract();
Expand Down Expand Up @@ -381,49 +441,6 @@ describe('ChromeNavGroupService#start()', () => {
expect(currentNavGroup?.title).toEqual('barGroupTitle');
});

it('should be able to find the right nav group when visible nav group is all', async () => {
const uiSettings = uiSettingsServiceMock.createSetupContract();
const navGroupEnabled$ = new Rx.BehaviorSubject(true);
uiSettings.get$.mockImplementation(() => navGroupEnabled$);

const chromeNavGroupService = new ChromeNavGroupService();
const chromeNavGroupServiceSetup = chromeNavGroupService.setup({ uiSettings });

chromeNavGroupServiceSetup.addNavLinksToGroup(
{
id: ALL_USE_CASE_ID,
title: 'fooGroupTitle',
description: 'foo description',
},
[mockedNavLinkFoo]
);

chromeNavGroupServiceSetup.addNavLinksToGroup(
{
id: 'bar-group',
title: 'barGroupTitle',
description: 'bar description',
status: NavGroupStatus.Hidden,
},
[mockedNavLinkFoo, mockedNavLinkBar]
);

const chromeNavGroupServiceStart = await chromeNavGroupService.start({
navLinks: mockedNavLinkService,
application: mockedApplicationService,
breadcrumbsEnricher$: new Rx.BehaviorSubject<ChromeBreadcrumbEnricher | undefined>(undefined),
workspaces: workspacesServiceMock.createStartContract(),
});
mockedApplicationService.navigateToApp(mockedNavLinkBar.id);

const currentNavGroup = await chromeNavGroupServiceStart
.getCurrentNavGroup$()
.pipe(first())
.toPromise();

expect(currentNavGroup?.id).toEqual('bar-group');
});

it('should be able to find the right nav group when visible nav group length is 1 and is not all nav group', async () => {
const uiSettings = uiSettingsServiceMock.createSetupContract();
const navGroupEnabled$ = new Rx.BehaviorSubject(true);
Expand Down
124 changes: 107 additions & 17 deletions src/core/public/chrome/nav_group/nav_group_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
ChromeNavLink,
WorkspacesStart,
} from 'opensearch-dashboards/public';
import { i18n } from '@osd/i18n';
import { map, switchMap, takeUntil } from 'rxjs/operators';
import { IUiSettingsClient } from '../../ui_settings';
import {
Expand All @@ -22,7 +23,7 @@
import { InternalApplicationStart } from '../../application';
import { NavGroupStatus, NavGroupType } from '../../../../core/types';
import { ChromeBreadcrumb, ChromeBreadcrumbEnricher } from '../chrome_service';
import { ALL_USE_CASE_ID } from '../../../utils';
import { ALL_USE_CASE_ID, DEFAULT_APP_CATEGORIES } from '../../../utils';

export const CURRENT_NAV_GROUP_ID = 'core.chrome.currentNavGroupId';

Expand Down Expand Up @@ -74,6 +75,14 @@
setCurrentNavGroup: (navGroupId: string | undefined) => void;
}

// Custom category is used for those features not belong to any of use cases in all use case.
// and the custom category should always sit after manage category
const customCategory: AppCategory = {
id: 'custom',
label: i18n.translate('core.ui.customNavList.label', { defaultMessage: 'Custom' }),
order: (DEFAULT_APP_CATEGORIES.manage.order || 0) + 500,
};

/** @internal */
export class ChromeNavGroupService {
private readonly navGroupsMap$ = new BehaviorSubject<Record<string, NavGroupItemInMap>>({});
Expand Down Expand Up @@ -114,12 +123,87 @@
}

private sortNavGroupNavLinks(
navGroup: NavGroupItemInMap,
navLinks: NavGroupItemInMap['navLinks'],
allValidNavLinks: Array<Readonly<ChromeNavLink>>
) {
return getSortedNavLinks(
fulfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, allValidNavLinks)
);
return getSortedNavLinks(fulfillRegistrationLinksToChromeNavLinks(navLinks, allValidNavLinks));
}

private getNavLinksForAllUseCase(
navGroupsMap: Record<string, NavGroupItemInMap>,
navLinks: Array<Readonly<ChromeNavLink>>
) {
// Note: we need to use a new pointer when `assign navGroupsMap[ALL_USE_CASE_ID]?.navLinks`
// because we will mutate the array directly in the following code.
const navLinksResult: ChromeRegistrationNavLink[] = [
...(navGroupsMap[ALL_USE_CASE_ID]?.navLinks || []),
];

// Append all the links that do not have use case info to keep backward compatible
const linkIdsWithNavGroupInfo = Object.values(navGroupsMap).reduce((accumulator, navGroup) => {
// Nav groups without type will be regarded as use case,
// we should transform use cases to a category and append links with `showInAllNavGroup: true` under the category
if (!navGroup.type) {
// Append use case section into left navigation
const categoryInfo = {
id: navGroup.id,
label: navGroup.title,
order: navGroup.order,
};

const fulfilledLinksOfNavGroup = fulfillRegistrationLinksToChromeNavLinks(
navGroup.navLinks,
navLinks
);

const linksForAllUseCaseWithinNavGroup: ChromeRegistrationNavLink[] = [];

fulfilledLinksOfNavGroup.forEach((navLink) => {
if (!navLink.showInAllNavGroup) {
return;
}

linksForAllUseCaseWithinNavGroup.push({

Check warning on line 166 in src/core/public/chrome/nav_group/nav_group_service.ts

View check run for this annotation

Codecov / codecov/patch

src/core/public/chrome/nav_group/nav_group_service.ts#L166

Added line #L166 was not covered by tests
...navLink,
category: categoryInfo,
});
});

navLinksResult.push(...linksForAllUseCaseWithinNavGroup);

if (!linksForAllUseCaseWithinNavGroup.length) {
/**
* Find if there are any links inside a use case but without a `see all` entry.
* If so, append these features into custom category as a fallback
*/
fulfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, navLinks).forEach(
(navLink) => {
// Links that already exists in all use case do not need to reappend
if (navLinksResult.find((navLinkInAll) => navLinkInAll.id === navLink.id)) {
return;
}
navLinksResult.push({
...navLink,
category: customCategory,
});
}
);
}
}

return [...accumulator, ...navGroup.navLinks.map((navLink) => navLink.id)];
}, [] as string[]);
navLinks.forEach((navLink) => {
if (linkIdsWithNavGroupInfo.includes(navLink.id)) {
return;
}
navLinksResult.push({
...navLink,
category: customCategory,
});
});

return navLinksResult;
}

private getSortedNavGroupsMap$() {
Expand All @@ -129,10 +213,20 @@
map(([navGroupsMap, navLinks]) => {
return Object.keys(navGroupsMap).reduce((sortedNavGroupsMap, navGroupId) => {
const navGroup = navGroupsMap[navGroupId];
sortedNavGroupsMap[navGroupId] = {
...navGroup,
navLinks: this.sortNavGroupNavLinks(navGroup, navLinks),
};
if (navGroupId === ALL_USE_CASE_ID) {
sortedNavGroupsMap[navGroupId] = {
...navGroup,
navLinks: this.sortNavGroupNavLinks(
this.getNavLinksForAllUseCase(navGroupsMap, navLinks),
navLinks
),
};
} else {
sortedNavGroupsMap[navGroupId] = {
...navGroup,
navLinks: this.sortNavGroupNavLinks(navGroup.navLinks, navLinks),
};
}
return sortedNavGroupsMap;
}, {} as Record<string, NavGroupItemInMap>);
})
Expand Down Expand Up @@ -270,14 +364,10 @@
});
};
if (visibleUseCases.length === 1) {
if (visibleUseCases[0].id === ALL_USE_CASE_ID) {
// If the only visible use case is all use case
// All the other nav groups will be visible because all use case can visit all of the nav groups.
Object.values(navGroupMap).forEach((navGroup) => mapAppIdToNavGroup(navGroup));
} else {
// It means we are in a workspace, we should only use the visible use cases
visibleUseCases.forEach((navGroup) => mapAppIdToNavGroup(navGroup));
}
// The length will be 1 if inside a workspace
// as workspace plugin will register a filter to only make the selected nav group visible.
// In order to tell which nav group we are in, we should use the only visible use case if the visibleUseCases.length equals 1.
visibleUseCases.forEach((navGroup) => mapAppIdToNavGroup(navGroup));
} else {
// Nav group of Hidden status should be filtered out when counting navGroups the currentApp belongs to
Object.values(navGroupMap).forEach((navGroup) => {
Expand Down
Loading
Loading