Skip to content

Commit

Permalink
[Serverless Nav] Fix issues with sticky app menu subheader (#168372)
Browse files Browse the repository at this point in the history
## Summary

- Fixes sticky kql bar in serverless security project
#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 <[email protected]>
  • Loading branch information
Dosant authored Oct 13, 2023
1 parent a48b58f commit 326ef31
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,20 @@ export class ChromeService {
};

const headerBanner$ = new BehaviorSubject<ChromeUserBanner | undefined>(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);
})
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface AppMenuBarProps {
}
export const AppMenuBar = ({ headerActionMenuMounter }: AppMenuBarProps) => {
const { euiTheme } = useEuiTheme();

return (
<div
className="header__actionMenu"
Expand All @@ -28,7 +29,8 @@ export const AppMenuBar = ({ headerActionMenuMounter }: AppMenuBarProps) => {
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;
Expand Down
19 changes: 18 additions & 1 deletion packages/react/kibana_mount/mount_point_portal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,23 @@ describe('MountPointPortal', () => {
expect(setMountPoint).toHaveBeenCalledTimes(1);
});

it('calls the provided `setMountPoint` with undefined during unmount', async () => {
dom = mount(
<MountPointPortal setMountPoint={setMountPoint}>
<span>portal content</span>
</MountPointPortal>
);

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(
<MountPointPortal setMountPoint={setMountPoint}>
Expand Down Expand Up @@ -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<HTMLElement>) => void;
setMount: (mountPoint: MountPoint<HTMLElement> | undefined) => void;
portalContent: string;
}> = ({ setMount, portalContent }) => {
return (
Expand Down
3 changes: 2 additions & 1 deletion packages/react/kibana_mount/mount_point_portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { MountPoint } from '@kbn/core/public';
import { useIfMounted } from './utils';

export interface MountPointPortalProps {
setMountPoint: (mountPoint: MountPoint<HTMLElement>) => void;
setMountPoint: (mountPoint: MountPoint<HTMLElement> | undefined) => void;
}

/**
Expand Down Expand Up @@ -47,6 +47,7 @@ export const MountPointPortal: React.FC<MountPointPortalProps> = ({ children, se
setShouldRender(false);
el.current = undefined;
});
setMountPoint(undefined);
};
}, [setMountPoint, ifMounted]);

Expand Down
2 changes: 2 additions & 0 deletions src/core/public/_css_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 7 additions & 3 deletions src/core/public/_mixins.scss
Original file line number Diff line number Diff line change
@@ -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}
);
}
25 changes: 20 additions & 5 deletions src/core/public/styles/rendering/_base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/plugins/kibana_react/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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` */
Expand Down
16 changes: 1 addition & 15 deletions src/plugins/kibana_react/public/util/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
8 changes: 2 additions & 6 deletions src/plugins/navigation/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@
"id": "navigation",
"server": false,
"browser": true,
"requiredPlugins": [
"unifiedSearch"
],
"requiredBundles": [
"kibanaReact"
]
"requiredPlugins": ["unifiedSearch"],
"requiredBundles": []
}
}
19 changes: 12 additions & 7 deletions src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -138,14 +138,19 @@ export function TopNavMenu<QT extends AggregateQuery | Query = Query>(
'kbnTopNavMenu__wrapper--hidden': visible === false,
});
if (setMenuMountPoint) {
const badgesEl = renderBadges();
const menuEl = renderMenu(menuClassName);
return (
<>
<MountPointPortal setMountPoint={setMenuMountPoint}>
<span className={`${wrapperClassName} kbnTopNavMenu__badgeWrapper`}>
{renderBadges()}
{renderMenu(menuClassName)}
</span>
</MountPointPortal>
{(badgesEl || menuEl) && (
<MountPointPortal setMountPoint={setMenuMountPoint}>
<span className={`${wrapperClassName} kbnTopNavMenu__badgeWrapper`}>
{badgesEl}
{menuEl}
</span>
</MountPointPortal>
)}

{renderSearchBar()}
</>
);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/navigation/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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/**/*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down

0 comments on commit 326ef31

Please sign in to comment.