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

[SecuritySolution][Navigation] Prevent initial re-render using project nav #201431

Merged
merged 3 commits into from
Nov 25, 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
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,11 @@ jest.mock('./timeline', () => ({
Timeline: () => <div>{'Timeline'}</div>,
}));

jest.mock('../../../common/components/navigation/use_security_solution_navigation', () => {
return {
useSecuritySolutionNavigation: () => ({
icon: 'logoSecurity',
items: [
{
id: 'investigate',
name: 'Investigate',
items: [
{
'data-href': 'some-data-href',
'data-test-subj': 'navigation-cases',
disabled: false,
href: 'some-href',
id: 'cases',
isSelected: true,
name: 'Cases',
},
],
tabIndex: undefined,
},
],
name: 'Security',
}),
};
});
const navProps = { icon: 'logoSecurity', items: [], name: 'Security' };
const mockUseSecuritySolutionNavigation = jest.fn();
jest.mock('../../../common/components/navigation/use_security_solution_navigation', () => ({
useSecuritySolutionNavigation: () => mockUseSecuritySolutionNavigation(),
}));

const mockUseRouteSpy = jest.fn((): [{ pageName: string }] => [
{ pageName: SecurityPageName.alerts },
Expand All @@ -69,6 +48,39 @@ const renderComponent = ({
describe('SecuritySolutionTemplateWrapper', () => {
beforeEach(() => {
jest.clearAllMocks();
mockUseSecuritySolutionNavigation.mockReturnValue(navProps);
});

describe('when navigation props are defined (classic nav)', () => {
beforeEach(() => {
mockUseSecuritySolutionNavigation.mockReturnValue(navProps);
});
it('should render the children', async () => {
const { queryByText } = renderComponent();
expect(queryByText('child of wrapper')).toBeInTheDocument();
});
});

describe('when navigation props are null (project nav)', () => {
beforeEach(() => {
mockUseSecuritySolutionNavigation.mockReturnValue(null);
});

it('should render the children', async () => {
const { queryByText } = renderComponent();
expect(queryByText('child of wrapper')).toBeInTheDocument();
});
});

describe('when navigation props are undefined (loading)', () => {
beforeEach(() => {
mockUseSecuritySolutionNavigation.mockReturnValue(undefined);
});

it('should not render the children', async () => {
const { queryByText } = renderComponent();
expect(queryByText('child of wrapper')).not.toBeInTheDocument();
});
});

it('Should render with bottom bar when user allowed', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ export const SecuritySolutionTemplateWrapper: React.FC<SecuritySolutionTemplateW
const { euiTheme, colorMode: globalColorMode } = useEuiTheme();

// There is some logic in the StyledKibanaPageTemplate that checks for children presence, and we dont even need to render the children
// here if isEmptyState is set
const isNotEmpty = !rest.isEmptyState;
// solutionNavProps is momentarily initialized to undefined, this check prevents the children from being re-rendered in the initial load
const renderChildren = !rest.isEmptyState && solutionNavProps !== undefined;

/*
* StyledKibanaPageTemplate is a styled EuiPageTemplate. Security solution currently passes the header
Expand All @@ -83,11 +83,11 @@ export const SecuritySolutionTemplateWrapper: React.FC<SecuritySolutionTemplateW
theme={euiTheme}
$isShowingTimelineOverlay={isShowingTimelineOverlay}
paddingSize="none"
solutionNav={solutionNavProps}
solutionNav={solutionNavProps ?? undefined}
restrictWidth={false}
{...rest}
>
{isNotEmpty && (
{renderChildren && (
<>
<GlobalKQLHeader />
<KibanaPageTemplate.Section
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@ describe('Security Solution Navigation', () => {
beforeEach(() => {
jest.clearAllMocks();
});
describe('while chrome style is undefined', () => {
beforeAll(() => {
mockGetChromeStyle$.mockReturnValue(of());
});

it('should return proper navigation props', async () => {
const { result } = renderHook(useSecuritySolutionNavigation);
expect(result.current).toEqual(undefined);

// check rendering of SecuritySideNav children
expect(mockSecuritySideNav).not.toHaveBeenCalled();
});
});

describe('when classic navigation is enabled', () => {
beforeAll(() => {
mockGetChromeStyle$.mockReturnValue(of('classic'));
Expand Down Expand Up @@ -77,9 +91,9 @@ describe('Security Solution Navigation', () => {
mockGetChromeStyle$.mockReturnValue(of('project'));
});

it('should return undefined props when disabled', () => {
it('should return null props when disabled', () => {
const { result } = renderHook(useSecuritySolutionNavigation);
expect(result.current).toEqual(undefined);
expect(result.current).toEqual(null);
});

it('should initialize breadcrumbs', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,20 @@ const translatedNavTitle = i18n.translate('xpack.securitySolution.navigation.mai
defaultMessage: 'Security',
});

export const useSecuritySolutionNavigation = (): KibanaPageTemplateProps['solutionNav'] => {
export const useSecuritySolutionNavigation = (): KibanaPageTemplateProps['solutionNav'] | null => {
const { chrome } = useKibana().services;
const chromeStyle$ = useMemo(() => chrome.getChromeStyle$(), [chrome]);
const chromeStyle = useObservable(chromeStyle$, 'classic');
const chromeStyle = useObservable(chromeStyle$, undefined);

useBreadcrumbsNav();

if (chromeStyle === undefined) {
return undefined; // wait for chromeStyle to be initialized
}

if (chromeStyle === 'project') {
// new shared-ux 'project' navigation enabled, return undefined to disable the 'classic' navigation
return undefined;
// new shared-ux 'project' navigation enabled, return null to disable the 'classic' navigation
return null;
}

return {
Expand Down