From 0edc73bc693430cb2575dfbcfe358efda8986614 Mon Sep 17 00:00:00 2001
From: Patryk Kopycinski <contact@patrykkopycinski.com>
Date: Mon, 12 Oct 2020 16:33:12 +0200
Subject: [PATCH] PR comments

---
 .../public/app/home/index.tsx                 |  17 ++-
 .../common/components/header_global/index.tsx | 139 +++++++++---------
 .../public/common/components/page/index.tsx   |   5 -
 .../common/components/wrapper_page/index.tsx  |  11 +-
 4 files changed, 89 insertions(+), 83 deletions(-)

diff --git a/x-pack/plugins/security_solution/public/app/home/index.tsx b/x-pack/plugins/security_solution/public/app/home/index.tsx
index 079c34114dfd6..054667e0ab9c7 100644
--- a/x-pack/plugins/security_solution/public/app/home/index.tsx
+++ b/x-pack/plugins/security_solution/public/app/home/index.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useRef } from 'react';
+import React, { useEffect, useMemo, useRef, useState } from 'react';
 import styled from 'styled-components';
 
 import { TimelineId } from '../../../common/types/timeline';
@@ -44,9 +44,18 @@ interface HomePageProps {
 }
 
 const HomePageComponent: React.FC<HomePageProps> = ({ children }) => {
-  const { application } = useKibana().services;
+  const { application, overlays } = useKibana().services;
   const subPluginId = useRef<string>('');
   const { ref, height = 0 } = useThrottledResizeObserver(300);
+  const banners$ = overlays.banners.get$();
+  const [headerFixed, setHeaderFixed] = useState<boolean>(true);
+  const mainPaddingTop = useMemo(() => (headerFixed ? height : 0), [headerFixed, height]);
+
+  useEffect(() => {
+    const subscription = banners$.subscribe((banners) => setHeaderFixed(!banners.length));
+    return () => subscription.unsubscribe();
+  }, [banners$]); // Only un/re-subscribe if the Observable changes
+
   application.currentAppId$.subscribe((appId) => {
     subPluginId.current = appId ?? '';
   });
@@ -72,9 +81,9 @@ const HomePageComponent: React.FC<HomePageProps> = ({ children }) => {
 
   return (
     <SecuritySolutionAppWrapper>
-      <HeaderGlobal ref={ref} />
+      <HeaderGlobal ref={ref} isFixed={headerFixed} />
 
-      <Main paddingTop={height} data-test-subj="pageContainer">
+      <Main paddingTop={mainPaddingTop} data-test-subj="pageContainer">
         <DragDropContextWrapper browserFields={browserFields}>
           <UseUrlState indexPattern={indexPattern} navTabs={navTabs} />
           {indicesExist && showTimeline && (
diff --git a/x-pack/plugins/security_solution/public/common/components/header_global/index.tsx b/x-pack/plugins/security_solution/public/common/components/header_global/index.tsx
index 0c6a54d4434d2..11623e1367574 100644
--- a/x-pack/plugins/security_solution/public/common/components/header_global/index.tsx
+++ b/x-pack/plugins/security_solution/public/common/components/header_global/index.tsx
@@ -24,13 +24,13 @@ import { APP_ID, ADD_DATA_PATH, APP_DETECTIONS_PATH } from '../../../../common/c
 import { useGlobalHeaderPortal } from '../../hooks/use_global_header_portal';
 import { LinkAnchor } from '../links';
 
-const Wrapper = styled.header`
-  ${({ theme }) => `
+const Wrapper = styled.header<{ $isFixed: boolean }>`
+  ${({ theme, $isFixed }) => `
     background: ${theme.eui.euiColorEmptyShade};
     border-bottom: ${theme.eui.euiBorderThin};
     width: 100%;
     z-index: ${theme.eui.euiZNavigation};
-    position: fixed;
+    position: ${$isFixed ? 'fixed' : 'relative'};
   `}
 `;
 Wrapper.displayName = 'Wrapper';
@@ -62,75 +62,78 @@ FlexGroup.displayName = 'FlexGroup';
 
 interface HeaderGlobalProps {
   hideDetectionEngine?: boolean;
+  isFixed?: boolean;
 }
 export const HeaderGlobal = React.memo(
-  forwardRef<HTMLDivElement, HeaderGlobalProps>(({ hideDetectionEngine = false }, ref) => {
-    const { globalHeaderPortalNode } = useGlobalHeaderPortal();
-    const { globalFullScreen } = useFullScreen();
-    const search = useGetUrlSearch(navTabs.overview);
-    const { application, http } = useKibana().services;
-    const { navigateToApp } = application;
-    const basePath = http.basePath.get();
-    const goToOverview = useCallback(
-      (ev) => {
-        ev.preventDefault();
-        navigateToApp(`${APP_ID}:${SecurityPageName.overview}`, { path: search });
-      },
-      [navigateToApp, search]
-    );
-    return (
-      <Wrapper ref={ref} className="siemHeaderGlobal">
-        <WrapperContent $globalFullScreen={globalFullScreen}>
-          <FlexGroup
-            alignItems="center"
-            $hasSibling={globalHeaderPortalNode.hasChildNodes()}
-            justifyContent="spaceBetween"
-            wrap
-          >
-            <FlexItem>
-              <EuiFlexGroup alignItems="center" responsive={false}>
-                <FlexItem grow={false}>
-                  <LinkAnchor onClick={goToOverview} href={getAppOverviewUrl(search)}>
-                    <EuiIcon aria-label={i18n.SECURITY_SOLUTION} type="logoSecurity" size="l" />
-                  </LinkAnchor>
-                </FlexItem>
-
-                <FlexItem component="nav">
-                  <SiemNavigation
-                    display="condensed"
-                    navTabs={
-                      hideDetectionEngine
-                        ? pickBy((_, key) => key !== SecurityPageName.detections, navTabs)
-                        : navTabs
-                    }
-                  />
-                </FlexItem>
-              </EuiFlexGroup>
-            </FlexItem>
-            <FlexItem grow={false}>
-              <EuiFlexGroup alignItems="center" gutterSize="s" responsive={false} wrap>
-                {window.location.pathname.includes(APP_DETECTIONS_PATH) && (
+  forwardRef<HTMLDivElement, HeaderGlobalProps>(
+    ({ hideDetectionEngine = false, isFixed = true }, ref) => {
+      const { globalHeaderPortalNode } = useGlobalHeaderPortal();
+      const { globalFullScreen } = useFullScreen();
+      const search = useGetUrlSearch(navTabs.overview);
+      const { application, http } = useKibana().services;
+      const { navigateToApp } = application;
+      const basePath = http.basePath.get();
+      const goToOverview = useCallback(
+        (ev) => {
+          ev.preventDefault();
+          navigateToApp(`${APP_ID}:${SecurityPageName.overview}`, { path: search });
+        },
+        [navigateToApp, search]
+      );
+      return (
+        <Wrapper ref={ref} $isFixed={isFixed}>
+          <WrapperContent $globalFullScreen={globalFullScreen}>
+            <FlexGroup
+              alignItems="center"
+              $hasSibling={globalHeaderPortalNode.hasChildNodes()}
+              justifyContent="spaceBetween"
+              wrap
+            >
+              <FlexItem>
+                <EuiFlexGroup alignItems="center" responsive={false}>
                   <FlexItem grow={false}>
-                    <MlPopover />
+                    <LinkAnchor onClick={goToOverview} href={getAppOverviewUrl(search)}>
+                      <EuiIcon aria-label={i18n.SECURITY_SOLUTION} type="logoSecurity" size="l" />
+                    </LinkAnchor>
+                  </FlexItem>
+
+                  <FlexItem component="nav">
+                    <SiemNavigation
+                      display="condensed"
+                      navTabs={
+                        hideDetectionEngine
+                          ? pickBy((_, key) => key !== SecurityPageName.detections, navTabs)
+                          : navTabs
+                      }
+                    />
                   </FlexItem>
-                )}
+                </EuiFlexGroup>
+              </FlexItem>
+              <FlexItem grow={false}>
+                <EuiFlexGroup alignItems="center" gutterSize="s" responsive={false} wrap>
+                  {window.location.pathname.includes(APP_DETECTIONS_PATH) && (
+                    <FlexItem grow={false}>
+                      <MlPopover />
+                    </FlexItem>
+                  )}
 
-                <FlexItem grow={false}>
-                  <EuiButtonEmpty
-                    data-test-subj="add-data"
-                    href={`${basePath}${ADD_DATA_PATH}`}
-                    iconType="plusInCircle"
-                  >
-                    {i18n.BUTTON_ADD_DATA}
-                  </EuiButtonEmpty>
-                </FlexItem>
-              </EuiFlexGroup>
-            </FlexItem>
-          </FlexGroup>
-          <OutPortal node={globalHeaderPortalNode} />
-        </WrapperContent>
-      </Wrapper>
-    );
-  })
+                  <FlexItem grow={false}>
+                    <EuiButtonEmpty
+                      data-test-subj="add-data"
+                      href={`${basePath}${ADD_DATA_PATH}`}
+                      iconType="plusInCircle"
+                    >
+                      {i18n.BUTTON_ADD_DATA}
+                    </EuiButtonEmpty>
+                  </FlexItem>
+                </EuiFlexGroup>
+              </FlexItem>
+            </FlexGroup>
+            <OutPortal node={globalHeaderPortalNode} />
+          </WrapperContent>
+        </Wrapper>
+      );
+    }
+  )
 );
 HeaderGlobal.displayName = 'HeaderGlobal';
diff --git a/x-pack/plugins/security_solution/public/common/components/page/index.tsx b/x-pack/plugins/security_solution/public/common/components/page/index.tsx
index d4e44371f918b..8a8eda3e20185 100644
--- a/x-pack/plugins/security_solution/public/common/components/page/index.tsx
+++ b/x-pack/plugins/security_solution/public/common/components/page/index.tsx
@@ -30,11 +30,6 @@ export const AppGlobalStyle = createGlobalStyle<{ theme: { eui: { euiColorPrimar
     background-color: rgba(0,0,0,0);
   }
 
-  // Hide global banners and display them manually inside WrapperPage
-  #globalBannerList {
-    display:none;
-  }
-
   div.application {
     background-color: rgba(0,0,0,0);
 
diff --git a/x-pack/plugins/security_solution/public/common/components/wrapper_page/index.tsx b/x-pack/plugins/security_solution/public/common/components/wrapper_page/index.tsx
index b30eecacd565c..2f673ee62219a 100644
--- a/x-pack/plugins/security_solution/public/common/components/wrapper_page/index.tsx
+++ b/x-pack/plugins/security_solution/public/common/components/wrapper_page/index.tsx
@@ -34,14 +34,16 @@ const Wrapper = styled.div`
     flex-direction: column;
     flex: 1 1 auto;
   }
+`;
+
+Wrapper.displayName = 'Wrapper';
 
-  .kbnGlobalBannerList {
+const BannersWrapper = styled.div`
+  > div {
     padding: 0 0 ${({ theme }) => `${theme.eui.paddingSizes.l}`} 0;
   }
 `;
 
-Wrapper.displayName = 'Wrapper';
-
 interface WrapperPageProps {
   children: React.ReactNode;
   restrictWidth?: boolean | number | string;
@@ -58,8 +60,6 @@ const WrapperPageComponent: React.FC<WrapperPageProps & CommonProps> = ({
   noTimeline,
   ...otherProps
 }) => {
-  const Banners = useKibana().services.overlays.banners.getComponent();
-
   const { globalFullScreen, setGlobalFullScreen } = useFullScreen();
   useEffect(() => {
     setGlobalFullScreen(false); // exit full screen mode on page load
@@ -74,7 +74,6 @@ const WrapperPageComponent: React.FC<WrapperPageProps & CommonProps> = ({
 
   return (
     <Wrapper className={classes} style={style} {...otherProps}>
-      {Banners}
       {children}
       <AppGlobalStyle />
     </Wrapper>