[SecuritySolution][Navigation] Prevent initial re-render using projec…

…t nav (elastic#201431) ## Summary Prevents an initial re-render when the `project` navigation style is set (serverless and new solution nav for stateful). This re-render was affecting all the top-level pages in Security Solution. --------- Co-authored-by: Michael Olorunnisola <[email protected]>
nreese · Nov 25, 2024 · d3ee297 · d3ee297
1 parent cdeb1e9
commit d3ee297
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 36 deletions.
diff --git a/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.test.tsx b/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.test.tsx
@@ -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 },
@@ -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 () => {

diff --git a/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.tsx b/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.tsx
@@ -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
@@ -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

diff --git a/...nts/navigation/use_security_solution_navigation/use_security_solution_navigation.test.tsx b/...nts/navigation/use_security_solution_navigation/use_security_solution_navigation.test.tsx
@@ -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'));
@@ -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', () => {

diff --git a/...mponents/navigation/use_security_solution_navigation/use_security_solution_navigation.tsx b/...mponents/navigation/use_security_solution_navigation/use_security_solution_navigation.tsx
@@ -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 {