[Fix] Inactive State on 'Discover' Tab in Side Navigation Menu (opens…

…earch-project#5432) * Fix active background render error * Update CHANGELOG.md * Revise test suit * Add isActive logic * Remove redundant tests and add comment for future reference * Update src/core/public/chrome/ui/header/nav_link.tsx * Seperate special cases from logic && revise test * Update CHANGELOG.md * Update src/core/public/chrome/ui/header/nav_link.tsx Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: Willie Hung <[email protected]> --------- Signed-off-by: Willie Hung <[email protected]> Signed-off-by: Josh Romero <[email protected]> Co-authored-by: Josh Romero <[email protected]> Co-authored-by: SuZhou-Joe <[email protected]>
gaurav7830 · Dec 13, 2023 · 478176a · 478176a
1 parent b8d30dc
commit 478176a
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Cleanup unused url ([#3847](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3847))
 - [TSVB, Dashboards] Fix inconsistent dark mode code editor themes ([#4609](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4609))
 - Fix `maps.proxyOpenSearchMapsServiceInMaps` config definition so it can be set ([#5170](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5170))
+- [Discover] Fix inactive state on 'Discover' tab in side navigation menu ([#5432](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5432))
 - [BUG] Add platform "darwin-arm64" to unit test ([#5290](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5290))
 - [BUG][Dev Tool] Add dev tool documentation link to dev tool's help menu [#5166](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5166)
 - Fix missing border for header navigation control on right ([#5450](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5450))

diff --git a/src/core/public/chrome/ui/header/nav_link.test.tsx b/src/core/public/chrome/ui/header/nav_link.test.tsx
@@ -0,0 +1,63 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { isActiveNavLink, createEuiListItem } from './nav_link';
+import { ChromeNavLink } from '../../..';
+import { httpServiceMock } from '../../../http/http_service.mock';
+
+describe('isActiveNavLink', () => {
+  it('should return true if the appId is "discover" and linkId is "discover"', () => {
+    expect(isActiveNavLink('discover', 'discover')).toBe(true);
+  });
+
+  it('should return true if the appId is "data-explorer" and linkId is "data-explorer"', () => {
+    expect(isActiveNavLink('data-explorer', 'data-explorer')).toBe(true);
+  });
+
+  it('should return true if the appId is "data-explorer" and linkId is "discover"', () => {
+    expect(isActiveNavLink('data-explorer', 'discover')).toBe(true);
+  });
+
+  it('should return false if the appId and linkId do not match', () => {
+    expect(isActiveNavLink('dashboard', 'discover')).toBe(false);
+  });
+});
+
+const mockBasePath = httpServiceMock.createSetupContract({ basePath: '/test' }).basePath;
+
+describe('createEuiListItem', () => {
+  const mockLink: Partial<ChromeNavLink> = {
+    href: 'test',
+    id: 'test',
+    title: 'Test App',
+    disabled: false,
+    euiIconType: 'inputOutput',
+    icon: 'testIcon',
+    tooltip: 'Test App Tooltip',
+  };
+
+  const mockProps = {
+    link: mockLink as ChromeNavLink,
+    appId: 'test',
+    basePath: mockBasePath,
+    dataTestSubj: 'test-subj',
+    onClick: jest.fn(),
+    navigateToApp: jest.fn(),
+    externalLink: false,
+  };
+
+  it('creates a list item with the correct properties', () => {
+    const listItem = createEuiListItem(mockProps);
+    expect(listItem).toHaveProperty('label', mockProps.link.tooltip);
+    expect(listItem).toHaveProperty('href', mockProps.link.href);
+    expect(listItem).toHaveProperty('data-test-subj', mockProps.dataTestSubj);
+    expect(listItem).toHaveProperty('onClick');
+    expect(listItem).toHaveProperty(
+      'isActive',
+      isActiveNavLink(mockProps.appId, mockProps.link.id)
+    );
+    expect(listItem).toHaveProperty('isDisabled', mockProps.link.disabled);
+  });
+});
diff --git a/src/core/public/chrome/ui/header/nav_link.tsx b/src/core/public/chrome/ui/header/nav_link.tsx
@@ -39,6 +39,14 @@ import { relativeToAbsolute } from '../../nav_links/to_nav_link';
 export const isModifiedOrPrevented = (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) =>
   event.metaKey || event.altKey || event.ctrlKey || event.shiftKey || event.defaultPrevented;
 
+// TODO: replace hard-coded values with a registration function, so that apps can control active nav links similar to breadcrumbs
+const aliasedApps: { [key: string]: string[] } = {
+  discover: ['data-explorer'],
+};
+
+export const isActiveNavLink = (appId: string | undefined, linkId: string): boolean =>
+  !!(appId === linkId || aliasedApps[linkId]?.includes(appId || ''));
+
 interface Props {
   link: ChromeNavLink;
   appId?: string;
@@ -82,7 +90,7 @@ export function createEuiListItem({
         navigateToApp(id);
       }
     },
-    isActive: appId === id,
+    isActive: isActiveNavLink(appId, id),
     isDisabled: disabled,
     'data-test-subj': dataTestSubj,
     ...(basePath && {