From f2f363a6a76847abbd965bf8e641f5031b18e698 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:09:22 +0800 Subject: [PATCH] [navigation]feat: make parent item unclickable and fix duplicate items in landing page. (#7619) (#7670) * feat: make parent item unclickable * Changeset file for PR #7619 created/updated * feat: do not show parent item in landing page * fix: unit test * temp: save * feat: update * feat: update * feat: update * feat: optimize code * fix: nav group not reflected when switching to analytics workspace * feat: optimize code * feat: optimize code based on comment * feat: optimize code based on comment --------- (cherry picked from commit 389fd28b3441df1d48aa49c11f9bcd1fea887289) Signed-off-by: SuZhou-Joe Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7619.yml | 2 + src/core/public/chrome/index.ts | 2 +- .../chrome/nav_group/nav_group_service.ts | 29 +++--- ...ollapsible_nav_group_enabled.test.tsx.snap | 71 ++++++++++----- .../header/collapsible_nav_group_enabled.scss | 31 +++++-- .../collapsible_nav_group_enabled.test.tsx | 11 ++- .../header/collapsible_nav_group_enabled.tsx | 43 +++++++-- src/core/public/chrome/utils.test.ts | 21 +++-- src/core/public/chrome/utils.ts | 67 +++++++++++--- src/core/public/index.ts | 4 + src/plugins/management/public/plugin.ts | 88 ++++++++++++++----- src/plugins/workspace/public/plugin.ts | 6 +- 12 files changed, 279 insertions(+), 96 deletions(-) create mode 100644 changelogs/fragments/7619.yml diff --git a/changelogs/fragments/7619.yml b/changelogs/fragments/7619.yml new file mode 100644 index 000000000000..4ad2ddcee19b --- /dev/null +++ b/changelogs/fragments/7619.yml @@ -0,0 +1,2 @@ +feat: +- Make parent item unclickable and fix duplicate items in landing page. ([#7619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7619)) \ No newline at end of file diff --git a/src/core/public/chrome/index.ts b/src/core/public/chrome/index.ts index d37f14dc9e8a..5347266d9c33 100644 --- a/src/core/public/chrome/index.ts +++ b/src/core/public/chrome/index.ts @@ -60,4 +60,4 @@ export { ChromeNavControl, ChromeNavControls } from './nav_controls'; export { ChromeDocTitle } from './doc_title'; export { RightNavigationOrder } from './constants'; export { ChromeRegistrationNavLink, ChromeNavGroupUpdater, NavGroupItemInMap } from './nav_group'; -export { fulfillRegistrationLinksToChromeNavLinks } from './utils'; +export { fulfillRegistrationLinksToChromeNavLinks, LinkItemType, getSortedNavLinks } from './utils'; diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index 883eceacb871..88689d88f2fc 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -14,11 +14,7 @@ import { import { map, switchMap, takeUntil } from 'rxjs/operators'; import { i18n } from '@osd/i18n'; import { IUiSettingsClient } from '../../ui_settings'; -import { - flattenLinksOrCategories, - fulfillRegistrationLinksToChromeNavLinks, - getOrderedLinksOrCategories, -} from '../utils'; +import { fulfillRegistrationLinksToChromeNavLinks, getSortedNavLinks } from '../utils'; import { ChromeNavLinks } from '../nav_links'; import { InternalApplicationStart } from '../../application'; import { NavGroupStatus } from '../../../../core/types'; @@ -117,10 +113,8 @@ export class ChromeNavGroupService { navGroup: NavGroupItemInMap, allValidNavLinks: Array> ) { - return flattenLinksOrCategories( - getOrderedLinksOrCategories( - fulfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, allValidNavLinks) - ) + return getSortedNavLinks( + fulfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, allValidNavLinks) ); } @@ -267,14 +261,17 @@ export class ChromeNavGroupService { if (appId && navGroupMap) { const appIdNavGroupMap = new Map>(); // iterate navGroupMap - Object.keys(navGroupMap).forEach((navGroupId) => { - navGroupMap[navGroupId].navLinks.forEach((navLink) => { - const navLinkId = navLink.id; - const navGroupSet = appIdNavGroupMap.get(navLinkId) || new Set(); - navGroupSet.add(navGroupId); - appIdNavGroupMap.set(navLinkId, navGroupSet); + Object.keys(navGroupMap) + // Nav group of Hidden status should be filtered out when counting navGroups the currentApp belongs to + .filter((navGroupId) => navGroupMap[navGroupId].status !== NavGroupStatus.Hidden) + .forEach((navGroupId) => { + navGroupMap[navGroupId].navLinks.forEach((navLink) => { + const navLinkId = navLink.id; + const navGroupSet = appIdNavGroupMap.get(navLinkId) || new Set(); + navGroupSet.add(navGroupId); + appIdNavGroupMap.set(navLinkId, navGroupSet); + }); }); - }); const navGroups = appIdNavGroupMap.get(appId); if (navGroups && navGroups.size === 1) { 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 fd19a03cb08b..2c6835acdaeb 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 @@ -431,39 +431,29 @@ exports[` should render correctly 1`] = ` id="euiSideNavContent_generated-id" >
- +
+
+
+ +
+
@@ -510,11 +532,11 @@ exports[` should render correctly 1`] = ` class="euiSideNavItem__items" >
+
span { + flex-direction: row-reverse; + + > * { + margin-right: $euiSizeS; + margin-left: 2px; + } + } + } + + .nav-link-fake-item { + margin-top: 0; + } + + .nav-link-fake-item-button { + display: none; + } + .nav-nested-item { + margin-bottom: 4px; + + &::after { + height: unset; + } + .nav-link-item-btn { padding-left: 0; padding-right: 0; @@ -31,7 +52,7 @@ .left-navigation-wrapper { display: flex; flex-direction: column; - border-right: $ouiBorderThin; + border-right: $euiBorderThin; } .scrollable-container { diff --git a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx index 4f14100e29e3..141b3eaf5abe 100644 --- a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx +++ b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx @@ -43,7 +43,7 @@ describe('', () => { it('should render correctly', () => { const navigateToApp = jest.fn(); const onNavItemClick = jest.fn(); - const { container, getByTestId } = render( + const { container, getByTestId, queryByTestId } = render( ', () => { expect(container).toMatchSnapshot(); expect(container.querySelectorAll('.nav-link-item-btn').length).toEqual(5); fireEvent.click(getByTestId('collapsibleNavAppLink-pure')); - expect(navigateToApp).toBeCalledWith('pure'); + expect(navigateToApp).toBeCalledTimes(0); + // The accordion is collapsed + expect(queryByTestId('collapsibleNavAppLink-subLink')).toBeNull(); + + // Expand the accordion + fireEvent.click(getByTestId('collapsibleNavAppLink-pure')); + fireEvent.click(getByTestId('collapsibleNavAppLink-subLink')); + expect(navigateToApp).toBeCalledWith('subLink'); }); }); 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 c0d2a87635ed..ab24309551f4 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 @@ -77,6 +77,8 @@ const titleForSeeAll = i18n.translate('core.ui.primaryNav.seeAllLabel', { defaultMessage: 'See all...', }); +const LEVEL_FOR_ROOT_ITEMS = 1; + export function NavGroups({ navLinks, suffix, @@ -114,7 +116,11 @@ export function NavGroups({ 'aria-label': link.title, }; }; - const createSideNavItem = (navLink: LinkItem, className?: string): EuiSideNavItemType<{}> => { + const createSideNavItem = ( + navLink: LinkItem, + level: number, + className?: string + ): EuiSideNavItemType<{}> => { if (navLink.itemType === LinkItemType.LINK) { if (navLink.link.title === titleForSeeAll) { const navItem = createNavItem({ @@ -135,18 +141,43 @@ export function NavGroups({ } if (navLink.itemType === LinkItemType.PARENT_LINK && navLink.link) { - return { - ...createNavItem({ link: navLink.link }), + const props = createNavItem({ link: navLink.link }); + const parentItem = { + ...props, forceOpen: true, - items: navLink.links.map((subNavLink) => createSideNavItem(subNavLink, 'nav-nested-item')), + /** + * The href and onClick should both be undefined to make parent item rendered as accordion. + */ + href: undefined, + onClick: undefined, + className: classNames(props.className, 'nav-link-parent-item'), + buttonClassName: classNames(props.buttonClassName, 'nav-link-parent-item-button'), + items: navLink.links.map((subNavLink) => + createSideNavItem(subNavLink, level + 1, 'nav-nested-item') + ), }; + /** + * OuiSideBar will never render items of first level as accordion, + * in order to display accordion, we need to render a fake parent item. + */ + if (level === LEVEL_FOR_ROOT_ITEMS) { + return { + className: 'nav-link-fake-item', + buttonClassName: 'nav-link-fake-item-button', + name: '', + items: [parentItem], + id: `fake_${props.id}`, + }; + } + + return parentItem; } if (navLink.itemType === LinkItemType.CATEGORY) { return { id: navLink.category?.id ?? '', name:
{navLink.category?.label ?? ''}
, - items: navLink.links?.map((link) => createSideNavItem(link)), + items: navLink.links?.map((link) => createSideNavItem(link, level + 1)), 'aria-label': navLink.category?.label, }; } @@ -155,7 +186,7 @@ export function NavGroups({ }; const orderedLinksOrCategories = getOrderedLinksOrCategories(navLinks); const sideNavItems = orderedLinksOrCategories - .map((navLink) => createSideNavItem(navLink)) + .map((navLink) => createSideNavItem(navLink, LEVEL_FOR_ROOT_ITEMS)) .filter((item): item is EuiSideNavItemType<{}> => !!item); return ( diff --git a/src/core/public/chrome/utils.test.ts b/src/core/public/chrome/utils.test.ts index ed163b753eba..18a027e1bed3 100644 --- a/src/core/public/chrome/utils.test.ts +++ b/src/core/public/chrome/utils.test.ts @@ -10,7 +10,7 @@ import { fulfillRegistrationLinksToChromeNavLinks, getOrderedLinks, getOrderedLinksOrCategories, - flattenLinksOrCategories, + getSortedNavLinks, } from './utils'; const mockedNonCategoryLink = { @@ -47,6 +47,15 @@ const mockedNavLinkB = { order: 5, }; +const mockedSubNavLinkA = { + id: 'sub_a', + parentNavLinkId: 'a', + title: 'sub_a', + baseUrl: '', + href: '', + order: 10, +}; + describe('getAllCategories', () => { it('should return all categories', () => { const links = { @@ -124,15 +133,15 @@ describe('getOrderedLinksOrCategories', () => { }); }); -describe('flattenLinksOrCategories', () => { +describe('getSortedNavLinks', () => { it('should return flattened links', () => { - const navLinks = [mockedNonCategoryLink, mockedNavLinkA, mockedNavLinkB]; - const orderedLinks = getOrderedLinksOrCategories(navLinks); - const flattenedLinks = flattenLinksOrCategories(orderedLinks); - expect(flattenedLinks.map((item) => item.id)).toEqual([ + const navLinks = [mockedNonCategoryLink, mockedNavLinkA, mockedNavLinkB, mockedSubNavLinkA]; + const sortedNavLinks = getSortedNavLinks(navLinks); + expect(sortedNavLinks.map((item) => item.id)).toEqual([ mockedNavLinkB.id, mockedNonCategoryLink.id, mockedNavLinkA.id, + mockedSubNavLinkA.id, ]); }); }); diff --git a/src/core/public/chrome/utils.ts b/src/core/public/chrome/utils.ts index 8a9dec8b2145..4def78651f2c 100644 --- a/src/core/public/chrome/utils.ts +++ b/src/core/public/chrome/utils.ts @@ -73,21 +73,36 @@ export function fulfillRegistrationLinksToChromeNavLinks( export const getOrderedLinks = (navLinks: ChromeNavLink[]): ChromeNavLink[] => navLinks.sort(sortBy('order')); -export function flattenLinksOrCategories(linkItems: LinkItem[]): ChromeNavLink[] { - return linkItems.reduce((acc, item) => { - if (item.itemType === LinkItemType.LINK) { - acc.push(item.link); - } else if (item.itemType === LinkItemType.PARENT_LINK) { - if (item.link) { - acc.push(item.link); - } - acc.push(...flattenLinksOrCategories(item.links)); +function walkLinkItemsTree( + props: { + linkItems: LinkItem[]; + parentItem?: LinkItem; + }, + callback: (props: { currentItem: LinkItem; parentItem?: LinkItem }) => void +) { + props.linkItems.forEach((item) => { + callback({ + parentItem: props.parentItem, + currentItem: item, + }); + if (item.itemType === LinkItemType.PARENT_LINK) { + walkLinkItemsTree( + { + linkItems: item.links, + parentItem: item, + }, + callback + ); } else if (item.itemType === LinkItemType.CATEGORY) { - acc.push(...flattenLinksOrCategories(item.links || [])); + walkLinkItemsTree( + { + linkItems: item.links || [], + parentItem: item, + }, + callback + ); } - - return acc; - }, [] as ChromeNavLink[]); + }); } export const generateItemTypeByLink = ( @@ -173,3 +188,29 @@ export function getOrderedLinksOrCategories( return result.sort(sortBy('order')); } + +export const getSortedNavLinks = ( + navLinks: ChromeNavLink[], + enricher?: (currentItem: LinkItem, parentItem?: LinkItem) => LinkItem +) => { + const sortedNavLinksTree = getOrderedLinksOrCategories(navLinks); + const acc: ChromeNavLink[] = []; + walkLinkItemsTree( + { + linkItems: sortedNavLinksTree, + }, + (props) => { + const { currentItem, parentItem } = props; + const enricheredResult = enricher ? enricher(currentItem, parentItem) : currentItem; + if ( + enricheredResult.itemType === LinkItemType.LINK || + enricheredResult.itemType === LinkItemType.PARENT_LINK + ) { + if (enricheredResult.link) { + acc.push(enricheredResult.link); + } + } + } + ); + return acc; +}; diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 9da3aed89736..946babb2f893 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -77,6 +77,8 @@ import { NavGroupItemInMap, fulfillRegistrationLinksToChromeNavLinks, createRecentNavLink, + LinkItemType, + getSortedNavLinks, } from './chrome'; import { FatalErrorsSetup, FatalErrorsStart, FatalErrorInfo } from './fatal_errors'; import { HttpSetup, HttpStart } from './http'; @@ -381,6 +383,8 @@ export { NavGroupItemInMap, fulfillRegistrationLinksToChromeNavLinks, createRecentNavLink, + LinkItemType, + getSortedNavLinks, }; export { __osdBootstrap__ } from './osd_bootstrap'; diff --git a/src/plugins/management/public/plugin.ts b/src/plugins/management/public/plugin.ts index d376feeafe7e..ef91e776a962 100644 --- a/src/plugins/management/public/plugin.ts +++ b/src/plugins/management/public/plugin.ts @@ -46,6 +46,7 @@ import { AppNavLinkStatus, DEFAULT_NAV_GROUPS, WorkspaceAvailability, + ChromeNavLink, } from '../../../core/public'; import { MANAGEMENT_APP_ID } from '../common/contants'; @@ -56,7 +57,11 @@ import { import { ManagementOverViewPluginSetup } from '../../management_overview/public'; import { toMountPoint } from '../../opensearch_dashboards_react/public'; import { SettingsIcon } from './components/settings_icon'; -import { fulfillRegistrationLinksToChromeNavLinks } from '../../../core/public'; +import { + fulfillRegistrationLinksToChromeNavLinks, + LinkItemType, + getSortedNavLinks, +} from '../../../core/public'; interface ManagementSetupDependencies { home?: HomePublicPluginSetup; @@ -118,6 +123,59 @@ export class ManagementPlugin implements Plugin { + const [coreStart] = await core.getStartServices(); + const navGroupMap = await coreStart.chrome.navGroup + .getNavGroupsMap$() + .pipe(first()) + .toPromise(); + const navLinks = navGroupMap[navGroupId]?.navLinks; + return getSortedNavLinks( + fulfillRegistrationLinksToChromeNavLinks( + navLinks || [], + coreStart.chrome.navLinks.getAll() + ).filter((item) => !item.hidden), + (currentItem, parentItem) => { + // Hide all the sub items because we will only show parent item in landing page. + if ( + currentItem.itemType === LinkItemType.LINK && + parentItem?.itemType === LinkItemType.PARENT_LINK + ) { + return { + ...currentItem, + link: { + ...currentItem.link, + hidden: true, + }, + }; + } + + /** + * Jump to first sub items when click on parent item in landing page + */ + if (currentItem.itemType === LinkItemType.PARENT_LINK) { + let payload = currentItem.link; + if (payload) { + if (currentItem.links?.[0].itemType === LinkItemType.LINK) { + payload = { + ...payload, + ...currentItem.links?.[0].link, + title: payload.title, + }; + } + } + + return { + ...currentItem, + link: payload, + }; + } + + return currentItem; + } + ).filter((navLink) => !navLink.hidden); + }; + core.application.register({ id: settingsLandingPageId, title: settingsLandingPageTitle, @@ -129,21 +187,15 @@ export class ManagementPlugin implements Plugin { const { renderApp } = await import('./landing_page_application'); const [coreStart] = await core.getStartServices(); - const navGroupMap = await coreStart.chrome.navGroup - .getNavGroupsMap$() - .pipe(first()) - .toPromise(); - const navLinks = navGroupMap[DEFAULT_NAV_GROUPS.settingsAndSetup.id]?.navLinks; - const fulfilledNavLink = fulfillRegistrationLinksToChromeNavLinks( - navLinks || [], - coreStart.chrome.navLinks.getAll() - ).filter((navLink) => navLink.id !== settingsLandingPageId && !navLink.hidden); + const navLinks = ( + await getNavLinksByNavGroupId(DEFAULT_NAV_GROUPS.settingsAndSetup.id) + ).filter((navLink) => navLink.id !== settingsLandingPageId); return renderApp({ mountElement: params.element, props: { navigateToApp: coreStart.application.navigateToApp, - navLinks: fulfilledNavLink, + navLinks, pageTitle: settingsLandingPageTitle, getStartedCards: [], }, @@ -162,21 +214,15 @@ export class ManagementPlugin implements Plugin { const { renderApp } = await import('./landing_page_application'); const [coreStart] = await core.getStartServices(); - const navGroupMap = await coreStart.chrome.navGroup - .getNavGroupsMap$() - .pipe(first()) - .toPromise(); - const navLinks = navGroupMap[DEFAULT_NAV_GROUPS.dataAdministration.id]?.navLinks; - const fulfilledNavLink = fulfillRegistrationLinksToChromeNavLinks( - navLinks || [], - coreStart.chrome.navLinks.getAll() - ).filter((navLink) => navLink.id !== dataAdministrationLandingPageId && !navLink.hidden); + const navLinks = ( + await getNavLinksByNavGroupId(DEFAULT_NAV_GROUPS.dataAdministration.id) + ).filter((navLink) => navLink.id !== dataAdministrationLandingPageId); return renderApp({ mountElement: params.element, props: { navigateToApp: coreStart.application.navigateToApp, - navLinks: fulfilledNavLink, + navLinks, pageTitle: dataAdministrationPageTitle, getStartedCards: [], }, diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 5a208d33c0fc..de7efe10eb00 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -150,15 +150,15 @@ export class WorkspacePlugin /** * The following logic determines whether a navigation group should be hidden or not based on the workspace's feature configurations. * It checks the following conditions: - * 1. The navigation group is not a system-level group (system groups are always visible). + * 1. The navigation group is not a system-level group (system groups are always visible except all use case). * 2. The current workspace has feature configurations set up. - * 3. The current workspace's use case it not "All use case". + * 3. The current workspace's use case is not "All use case". * 4. The current navigation group is not included in the feature configurations of the workspace. * * If all these conditions are true, it means that the navigation group should be hidden. */ if ( - navGroup.type !== NavGroupType.SYSTEM && + (navGroup.type !== NavGroupType.SYSTEM || navGroup.id === ALL_USE_CASE_ID) && currentWorkspace.features && getFirstUseCaseOfFeatureConfigs(currentWorkspace.features) !== ALL_USE_CASE_ID && !isNavGroupInFeatureConfigs(navGroup.id, currentWorkspace.features)