Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Fix] Inactive State on 'Discover' Tab in Side Navigation Menu (#5432)
Browse files Browse the repository at this point in the history
* 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]>
(cherry picked from commit 478176a)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
3 people authored and manasvinibs committed Jan 31, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 7102989 commit e3f5077
Showing 2 changed files with 72 additions and 1 deletion.
63 changes: 63 additions & 0 deletions src/core/public/chrome/ui/header/nav_link.test.tsx
Original file line number Diff line number Diff line change
@@ -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);
});
});
10 changes: 9 additions & 1 deletion src/core/public/chrome/ui/header/nav_link.tsx
Original file line number Diff line number Diff line change
@@ -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 && {

0 comments on commit e3f5077

Please sign in to comment.