From 326ef31677a6df2fe3cfaebd4e3fe251d8b26c12 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Fri, 13 Oct 2023 16:11:31 +0200 Subject: [PATCH] [Serverless Nav] Fix issues with sticky app menu subheader (#168372) ## Summary - Fixes sticky kql bar in serverless security project https://github.com/elastic/kibana/issues/167908 - Fixes double scroll in serverless discover caused by incorrect app container height cc @elastic/kibana-data-discovery ![Screenshot 2023-10-10 at 17 23 58](https://github.com/elastic/kibana/assets/7784120/3bf50299-7d9f-4c38-953a-33a6a75815c6) - Fixes empty app header for top_nav component, for example, discover doc page: ![Screenshot 2023-10-10 at 17 24 45](https://github.com/elastic/kibana/assets/7784120/4965deac-9472-402f-8e8e-66ede83ce1bb) --------- Co-authored-by: Cee Chen --- .../src/chrome_service.tsx | 12 ++++++--- .../src/ui/project/app_menu.tsx | 4 ++- .../kibana_mount/mount_point_portal.test.tsx | 19 +++++++++++++- .../react/kibana_mount/mount_point_portal.tsx | 3 ++- src/core/public/_css_variables.scss | 2 ++ src/core/public/_mixins.scss | 10 +++++--- src/core/public/styles/rendering/_base.scss | 25 +++++++++++++++---- src/plugins/kibana_react/public/index.ts | 2 +- .../kibana_react/public/util/index.tsx | 16 +----------- src/plugins/navigation/kibana.jsonc | 8 ++---- .../public/top_nav_menu/top_nav_menu.tsx | 19 ++++++++------ src/plugins/navigation/tsconfig.json | 2 +- .../global_kql_header/index.tsx | 2 +- 13 files changed, 79 insertions(+), 45 deletions(-) diff --git a/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx b/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx index 47abc6c5646fe8..bf44390d132945 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx +++ b/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx @@ -203,14 +203,20 @@ export class ChromeService { }; const headerBanner$ = new BehaviorSubject(undefined); - const bodyClasses$ = combineLatest([headerBanner$, this.isVisible$!, chromeStyle$]).pipe( - map(([headerBanner, isVisible, chromeStyle]) => { + const bodyClasses$ = combineLatest([ + headerBanner$, + this.isVisible$!, + chromeStyle$, + application.currentActionMenu$, + ]).pipe( + map(([headerBanner, isVisible, chromeStyle, actionMenu]) => { return [ 'kbnBody', headerBanner ? 'kbnBody--hasHeaderBanner' : 'kbnBody--noHeaderBanner', isVisible ? 'kbnBody--chromeVisible' : 'kbnBody--chromeHidden', + chromeStyle === 'project' && actionMenu ? 'kbnBody--hasProjectActionMenu' : '', getKbnVersionClass(), - ]; + ].filter((className) => !!className); }) ); diff --git a/packages/core/chrome/core-chrome-browser-internal/src/ui/project/app_menu.tsx b/packages/core/chrome/core-chrome-browser-internal/src/ui/project/app_menu.tsx index 22ff7c9415ba8e..0c0e4dbdf91678 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/ui/project/app_menu.tsx +++ b/packages/core/chrome/core-chrome-browser-internal/src/ui/project/app_menu.tsx @@ -17,6 +17,7 @@ interface AppMenuBarProps { } export const AppMenuBar = ({ headerActionMenuMounter }: AppMenuBarProps) => { const { euiTheme } = useEuiTheme(); + return (
{ display: flex; justify-content: end; align-items: center; - padding: ${euiTheme.size.s}; + padding: 0 ${euiTheme.size.s}; + height: var(--kbnProjectHeaderAppActionMenuHeight, ${euiTheme.size.xxxl}); margin-bottom: -${euiTheme.border.width.thin}; /* fixates the elements position in the viewport, removes the element from the flow of the page */ position: sticky; diff --git a/packages/react/kibana_mount/mount_point_portal.test.tsx b/packages/react/kibana_mount/mount_point_portal.test.tsx index c7200a934f5a67..04fbf12ecdde56 100644 --- a/packages/react/kibana_mount/mount_point_portal.test.tsx +++ b/packages/react/kibana_mount/mount_point_portal.test.tsx @@ -58,6 +58,23 @@ describe('MountPointPortal', () => { expect(setMountPoint).toHaveBeenCalledTimes(1); }); + it('calls the provided `setMountPoint` with undefined during unmount', async () => { + dom = mount( + + portal content + + ); + + await refresh(); + + dom.unmount(); + + await refresh(); + + expect(setMountPoint).toHaveBeenCalledTimes(2); + expect(setMountPoint).toHaveBeenLastCalledWith(undefined); + }); + it('renders the portal content when calling the mountPoint ', async () => { dom = mount( @@ -127,7 +144,7 @@ describe('MountPointPortal', () => { it('updates the content of the portal element when the content of MountPointPortal changes', async () => { const Wrapper: FC<{ - setMount: (mountPoint: MountPoint) => void; + setMount: (mountPoint: MountPoint | undefined) => void; portalContent: string; }> = ({ setMount, portalContent }) => { return ( diff --git a/packages/react/kibana_mount/mount_point_portal.tsx b/packages/react/kibana_mount/mount_point_portal.tsx index c5bc994279c41e..dcfeddc428d095 100644 --- a/packages/react/kibana_mount/mount_point_portal.tsx +++ b/packages/react/kibana_mount/mount_point_portal.tsx @@ -13,7 +13,7 @@ import { MountPoint } from '@kbn/core/public'; import { useIfMounted } from './utils'; export interface MountPointPortalProps { - setMountPoint: (mountPoint: MountPoint) => void; + setMountPoint: (mountPoint: MountPoint | undefined) => void; } /** @@ -47,6 +47,7 @@ export const MountPointPortal: React.FC = ({ children, se setShouldRender(false); el.current = undefined; }); + setMountPoint(undefined); }; }, [setMountPoint, ifMounted]); diff --git a/src/core/public/_css_variables.scss b/src/core/public/_css_variables.scss index cef1be40d12393..5fc2c4dbfc2d37 100644 --- a/src/core/public/_css_variables.scss +++ b/src/core/public/_css_variables.scss @@ -5,6 +5,8 @@ --kbnHeaderOffset: var(--euiFixedHeadersOffset, 0); // total height of everything when the banner is present --kbnHeaderOffsetWithBanner: calc(var(--kbnHeaderBannerHeight) + var(--kbnHeaderOffset)); + // height of the action menu in the header in serverless projects + --kbnProjectHeaderAppActionMenuHeight: #{$euiSize * 3}; } // Quick note: This shouldn't be mixed with Sass variable declarations, diff --git a/src/core/public/_mixins.scss b/src/core/public/_mixins.scss index 9d533a87d1843e..0c6a8571f9e75f 100644 --- a/src/core/public/_mixins.scss +++ b/src/core/public/_mixins.scss @@ -1,6 +1,10 @@ @mixin kibanaFullBodyHeight($additionalOffset: 0) { - // The `--euiFixedHeadersOffset` CSS variable is automatically updated by + // The `--kbnAppHeadersOffset` CSS variable is automatically updated by // styles/rendering/_base.scss, based on whether the Kibana chrome has a - // header banner, and is visible or hidden - height: calc(100vh - var(--euiFixedHeadersOffset, 0) - #{$additionalOffset}); + // header banner, app menu, and is visible or hidden + height: calc( + 100vh + - var(--kbnAppHeadersOffset, var(--euiFixedHeadersOffset, 0)) + - #{$additionalOffset} + ); } diff --git a/src/core/public/styles/rendering/_base.scss b/src/core/public/styles/rendering/_base.scss index 8a7b14242f8bfc..1bcfaab71ea17b 100644 --- a/src/core/public/styles/rendering/_base.scss +++ b/src/core/public/styles/rendering/_base.scss @@ -19,7 +19,7 @@ pointer-events: none; visibility: hidden; position: fixed; - top: var(--euiFixedHeadersOffset, 0); + top: var(--kbnAppHeadersOffset, var(--euiFixedHeadersOffset, 0)); right: 0; bottom: 0; left: 0; @@ -41,7 +41,6 @@ // Conditionally override :root CSS fixed header variable. Updating `--euiFixedHeadersOffset` // on the body will cause all child EUI components to automatically update their offsets - .kbnBody--hasHeaderBanner { --euiFixedHeadersOffset: var(--kbnHeaderOffsetWithBanner); @@ -56,9 +55,25 @@ top: var(--kbnHeaderBannerHeight); } } + +// Set a body CSS variable for the app container to use - calculates the total +// height of all fixed headers + the sticky action menu toolbar +.kbnBody--hasProjectActionMenu { + --kbnAppHeadersOffset: calc(var(--kbnHeaderOffset) + var(--kbnProjectHeaderAppActionMenuHeight)); + + &.kbnBody--hasHeaderBanner { + --kbnAppHeadersOffset: calc(var(--kbnHeaderOffsetWithBanner) + var(--kbnProjectHeaderAppActionMenuHeight)); + } +} + .kbnBody--chromeHidden { --euiFixedHeadersOffset: 0; -} -.kbnBody--chromeHidden.kbnBody--hasHeaderBanner { - --euiFixedHeadersOffset: var(--kbnHeaderBannerHeight); + + &.kbnBody--hasHeaderBanner { + --euiFixedHeadersOffset: var(--kbnHeaderBannerHeight); + } + + &.kbnBody--hasProjectActionMenu { + --kbnAppHeadersOffset: var(--euiFixedHeadersOffset, 0); + } } diff --git a/src/plugins/kibana_react/public/index.ts b/src/plugins/kibana_react/public/index.ts index 7954559206bb75..2803c266da0f4b 100644 --- a/src/plugins/kibana_react/public/index.ts +++ b/src/plugins/kibana_react/public/index.ts @@ -76,7 +76,7 @@ export { createNotifications } from './notifications'; /** @deprecated use `Markdown` from `@kbn/shared-ux-markdown` */ export { Markdown, MarkdownSimple } from './markdown'; -export { toMountPoint, MountPointPortal } from './util'; +export { toMountPoint } from './util'; export type { ToMountPointOptions } from './util'; /** @deprecated Use `RedirectAppLinks` from `@kbn/shared-ux-link-redirect-app` */ diff --git a/src/plugins/kibana_react/public/util/index.tsx b/src/plugins/kibana_react/public/util/index.tsx index ab2ce5a5a81c86..d709f06837c0cb 100644 --- a/src/plugins/kibana_react/public/util/index.tsx +++ b/src/plugins/kibana_react/public/util/index.tsx @@ -15,11 +15,7 @@ import type { I18nStart } from '@kbn/core-i18n-browser'; import type { CoreTheme, ThemeServiceStart } from '@kbn/core-theme-browser'; import { defaultTheme } from '@kbn/react-kibana-context-common'; -import { - toMountPoint as _toMountPoint, - MountPointPortal as _MountPointPortal, - useIfMounted as _useIfMounted, -} from '@kbn/react-kibana-mount'; +import { toMountPoint as _toMountPoint } from '@kbn/react-kibana-mount'; // The `theme` start contract should always be included to ensure // dark mode is applied correctly. This code is for compatibility purposes, @@ -52,13 +48,3 @@ export const toMountPoint = ( const theme = theme$ ? { theme$ } : themeStart; return _toMountPoint(node, { theme, i18n }); }; - -/** - * @deprecated use `MountPointPortal` from `@kbn/react-kibana-mount` - */ -export const MountPointPortal = _MountPointPortal; - -/** - * @deprecated use `useIfMounted` from `@kbn/react-kibana-mount` - */ -export const useIfMounted = _useIfMounted; diff --git a/src/plugins/navigation/kibana.jsonc b/src/plugins/navigation/kibana.jsonc index 90ced649980a5b..26edb0999699d5 100644 --- a/src/plugins/navigation/kibana.jsonc +++ b/src/plugins/navigation/kibana.jsonc @@ -6,11 +6,7 @@ "id": "navigation", "server": false, "browser": true, - "requiredPlugins": [ - "unifiedSearch" - ], - "requiredBundles": [ - "kibanaReact" - ] + "requiredPlugins": ["unifiedSearch"], + "requiredBundles": [] } } diff --git a/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx b/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx index 3b3cac7921813f..de060db9b6e3b3 100644 --- a/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx +++ b/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx @@ -18,7 +18,7 @@ import { import classNames from 'classnames'; import { MountPoint } from '@kbn/core/public'; -import { MountPointPortal } from '@kbn/kibana-react-plugin/public'; +import { MountPointPortal } from '@kbn/react-kibana-mount'; import { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public'; import { StatefulSearchBarProps } from '@kbn/unified-search-plugin/public'; import { AggregateQuery, Query } from '@kbn/es-query'; @@ -138,14 +138,19 @@ export function TopNavMenu( 'kbnTopNavMenu__wrapper--hidden': visible === false, }); if (setMenuMountPoint) { + const badgesEl = renderBadges(); + const menuEl = renderMenu(menuClassName); return ( <> - - - {renderBadges()} - {renderMenu(menuClassName)} - - + {(badgesEl || menuEl) && ( + + + {badgesEl} + {menuEl} + + + )} + {renderSearchBar()} ); diff --git a/src/plugins/navigation/tsconfig.json b/src/plugins/navigation/tsconfig.json index b23ee2de840ebe..d10df84c43f8b3 100644 --- a/src/plugins/navigation/tsconfig.json +++ b/src/plugins/navigation/tsconfig.json @@ -6,11 +6,11 @@ "include": ["public/**/*"], "kbn_references": [ "@kbn/core", - "@kbn/kibana-react-plugin", "@kbn/unified-search-plugin", "@kbn/es-query", "@kbn/i18n-react", "@kbn/test-jest-helpers", + "@kbn/react-kibana-mount", ], "exclude": [ "target/**/*", diff --git a/x-pack/plugins/security_solution/public/app/home/template_wrapper/global_kql_header/index.tsx b/x-pack/plugins/security_solution/public/app/home/template_wrapper/global_kql_header/index.tsx index f5f9b55e59e7cc..80ddc7769e91e3 100644 --- a/x-pack/plugins/security_solution/public/app/home/template_wrapper/global_kql_header/index.tsx +++ b/x-pack/plugins/security_solution/public/app/home/template_wrapper/global_kql_header/index.tsx @@ -12,7 +12,7 @@ import { useGlobalHeaderPortal } from '../../../../common/hooks/use_global_heade const StyledStickyWrapper = styled.div` position: sticky; z-index: ${(props) => props.theme.eui.euiZHeaderBelowDataGrid}; - top: var(--euiFixedHeadersOffset, 0); + top: var(--kbnAppHeadersOffset, var(--euiFixedHeadersOffset, 0)); `; export const GlobalKQLHeader = React.memo(() => {