Skip to content

[Workspace Chrome] Bootstrap grid layout for classic nav#224255

Merged
Dosant merged 9 commits intoelastic:mainfrom
Dosant:d/2025-06-13-application-layout-2
Jun 27, 2025
Merged

[Workspace Chrome] Bootstrap grid layout for classic nav#224255
Dosant merged 9 commits intoelastic:mainfrom
Dosant:d/2025-06-13-application-layout-2

Conversation

@Dosant
Copy link
Copy Markdown
Contributor

@Dosant Dosant commented Jun 17, 2025

Important

Should be no user-facing changes!!! The new layout work is behind a feature flag!

Important

This bootstraps new grid layout for chrome using a feature flag. It only works with classic nav and hack a lot of bugs and EUI-related workarounds, but the overall code structure and approach can be reviewed and merged to main.

Summary

Part of workspace chrome work. In this PR we lay down the ground work for new grid layout that will power Kibana's chrome. This is done by introducing a feature flag with which Kibana can switch between "legacy-fixed" layout and new "grid" layout.

Image

Proper detailed figma link: https://www.figma.com/design/10ca4AhnWDkyJklUDXnHg5/Sidebar?node-id=5192-259808&p=f&m=dev

kibana.yml:

feature_flags.overrides:
  core.chrome.layoutType: 'grid'

For this, in-between rendering_service and chrome_service a new layout_service was introduced the goal of which is to aggregate stuff from chrome service and compose it together using the needed layout. There are two implementations for layout_service:

  • LegacyFixedLayout - old one, just code refactor, should still work as in main
  • GridLayout- new one, mostly works, but only for classic nav, for now, and with bunch of hacks and bugs that we will resolve over time

The switch is in rendering_service based on a feature flag:

const layout: LayoutService =
      layoutType === 'grid'
        ? new GridLayout(renderCoreDeps)
        : new LegacyFixedLayout(renderCoreDeps);

    const Layout = layout.getComponent();

    ReactDOM.render(
      <KibanaRootContextProvider {...startServices} globalStyles={true}>
        <Layout />
      </KibanaRootContextProvider>,
      targetDomElement
    );`

To see the grid and new layout in action there is a helpful debug flag that displays not yet used elements of new layout:

kibana.yml:

feature_flags.overrides:
  core.chrome.layoutType: 'grid'
  core.chrome.layoutDebug: true
demo-grid.mov

Other clean ups

  • Migrate .chrHeaderBadge__wrapper, . chrHeaderHelpMenu__version, breadcrumbsWithExtensionContainer to emotion on simplify global css of chrome
  • remove getIsNavDrawerLocked and related css since not used
  • Small unzyme

TODO

  • fix solution nav in management
  • make sure solution nav works with header
  • fix dashboard full screen mode
  • check discover eui grid full screen
  • check chromeless mode
  • Follow up on EUI related hacks [Meta][Kibana Workspace Chrome] Improvements for new Kibana layout eui#8820
  • Misaligned console in search solution
  • Miaaligned secondary nav in security solutions
  • double scroll in discover push flyout

How to review

  1. Most importantly, we need to ensure that nothing is broken in the old layout during the refactor. - Functional tests + visual/manual testing
  2. Then for the new layout:

kibana.yml:

feature_flags.overrides:
  core.chrome.layoutType: 'grid'
  core.chrome.layoutDebug: true
  • Check that it mostly works (some specific edge cases and bugs are fine)
  • Code-review: focus on the layout implementation split approach

@Dosant Dosant changed the title D/2025 06 13 application layout 2 [Workspace Chrome] Bootstrap grid layout Jun 20, 2025
@Dosant Dosant changed the title [Workspace Chrome] Bootstrap grid layout [Workspace Chrome] Bootstrap grid layout for classic nav Jun 20, 2025
@Dosant Dosant force-pushed the d/2025-06-13-application-layout-2 branch from 71abd68 to a4101de Compare June 23, 2025 10:03
const badge$ = new BehaviorSubject<ChromeBadge | undefined>(undefined);
const customNavLink$ = new BehaviorSubject<ChromeNavLink | undefined>(undefined);
const helpSupportUrl$ = new BehaviorSubject<string>(docLinks.links.kibana.askElastic);
const isNavDrawerLocked$ = new BehaviorSubject(localStorage.getItem(IS_LOCKED_KEY) === 'true');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used; it's just a cleanup.

recentlyAccessed,
docTitle,
getHeaderComponent,
getLegacyHeaderComponentForFixedLayout,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • getLegacyHeaderComponentForFixedLayout (getHeaderComponent) - is mostly untouched and still responsible for rendering the header in the old layout which is fixed, includes a banner, swaps between classic and project nav, handles chromeless state
  • getClassicHeaderComponentForGridLayout - header to use in grid layout for classic navigation, doesn't include banner and chromeless state that is now is handled by the layout service
  • getHeaderBanner, getChromelessHeader - used by the new layout

}

const styles = {
breadcrumbsWithExtensionContainer: css`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplifying global chrome styles by moving what is possible to the components and emotion


return (
<div className="chrHeaderBadge__wrapper">
<div css={({ euiTheme }) => ({ alignSelf: 'center', marginLeft: euiTheme.size.base })}>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplifying global chrome styles by moving what is possible to the components and emotion

<EuiFlexItem
grow={false}
className="chrHeaderHelpMenu__version"
css={{ textTransform: 'none' }}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplifying global chrome styles by moving what is possible to the components and emotion


// Due to pure HTML and the scope being large, we decided to temporarily apply following 3 style blocks globally.
// TODO: refactor within github issue #223571
const hackGlobalFieldFormattersPluginStyles = (euiTheme: UseEuiTheme['euiTheme']) => css`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just moving to a more appropriate place to share between both layouts

import { CommonGlobalAppStyles } from '../common/global_app_styles';
import { useSyncPushFlyoutHack, euiPushFlyoutPaddingInlineEnd } from './hack_use_sync_push_flyout';

const globalLayoutStyles = (euiTheme: UseEuiTheme['euiTheme']) => css`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we consolidate and split important global styles for each layout. grid and legacy-fixed layouts have their own copy and are different.

`;

// temporary hacks that need to be removed after better flyout and global sidenav customization support in EUI
const globalTempHackStyles = (euiTheme: UseEuiTheme['euiTheme']) => css`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

global css overrides hacks for eui are consolidated here. the goal is to merge as is, but to get rid of it on EUI side before this goes to prod. elastic/eui#8820

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add link to eui issue.

* Currently, EUI push flyouts visually push the content to the right by adding a padding to the body
* This hook listens to styles changes on the body and updates a CSS variable that is used to push the workspace content
*/
export function useSyncPushFlyoutHack() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dirty hack to position push flyout inside the the application container. the goal is to merge as is, but to get rid of it on EUI side before this goes to prod. elastic/eui#8820

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we might consider naming things like this useHackSyncPushFlyout rather than useSyncPushFlyoutHack.

If there wasn't a lint rule around Rules of Hooks™️ I would name it hack__useSyncPushFlyout. 🤷🏻

And please add a link to the issue in the comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wanted to start with hack...

const sideBarProps = { ...pageSideBarProps };
sideBarProps.sticky = true;
// TODO: instead of using sticky = true here, we reproduce the same behavior to account for both legacy fixed layout and new grid layout.
sideBarProps.style = {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another temp hack for eui: how sticky: true works doesn't work properly with new layout, so I had to customize styles here so that it works for both fixed and grid kibana layout elastic/eui#8820

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: link to eui issue in the comment.

@Dosant Dosant added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// labels Jun 24, 2025
@Dosant Dosant marked this pull request as ready for review June 24, 2025 09:11
@Dosant Dosant requested review from a team as code owners June 24, 2025 09:11
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

Copy link
Copy Markdown
Contributor

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is impressive work, Anton. Really great, and fast!

I'm approving to unblock, but please address the nits. My biggest is eliminating useObservable by subscribing to the Observables in the Layout services, passing their values as props. It's a subtle (but proven important) optimization.

return <HeaderComponent />;
};

const getClassicHeaderComponentForGridLayout = () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ChromeService has been driving me crazy with all the methods it contains, apparently scoped to different use cases.

It may not be germane to this PR, but we've got to find some time to clean this thing up... or at the very least, split it up somehow. Perhaps make this an abstract class with use-case implementations?

Again: I think something for later. But I'm resigned to see another pair of use-case specific, complex-named methods added to this service.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree! I think now with a feature flag, would make sense to rollout the grid layout -> clean up the old layout -> then do a proper refactor of chrome service because code and testing scope would be smaller. I captured an issue #225264 and will leave it in the code to make it clearer

/>

<EuiHeader position="fixed" className="header__secondBar">
<EuiHeader position={isFixed ? 'fixed' : 'static'} className="header__secondBar">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this classname still apply?

* Currently, EUI push flyouts visually push the content to the right by adding a padding to the body
* This hook listens to styles changes on the body and updates a CSS variable that is used to push the workspace content
*/
export function useSyncPushFlyoutHack() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we might consider naming things like this useHackSyncPushFlyout rather than useSyncPushFlyoutHack.

If there wasn't a lint rule around Rules of Hooks™️ I would name it hack__useSyncPushFlyout. 🤷🏻

And please add a link to the issue in the comment.


return React.memo(() => {
// TODO: optimize initial value to avoid unnecessary re-renders
const chromeVisible = useObservable(chromeVisible$, false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get rid of the Observable and this gets easy.

const { chrome, featureFlags } = renderCoreDeps;
const layoutType = featureFlags.getStringValue<LayoutFeatureFlag>(
LAYOUT_FEATURE_FLAG_KEY,
'legacy-fixed'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider a constant? Or is this strongly-typed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typed!

const sideBarProps = { ...pageSideBarProps };
sideBarProps.sticky = true;
// TODO: instead of using sticky = true here, we reproduce the same behavior to account for both legacy fixed layout and new grid layout.
sideBarProps.style = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: link to eui issue in the comment.

`;

// temporary hacks that need to be removed after better flyout and global sidenav customization support in EUI
const globalTempHackStyles = (euiTheme: UseEuiTheme['euiTheme']) => css`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add link to eui issue.

Copy link
Copy Markdown
Member

@gsoldevila gsoldevila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core changes LGTM (code review only)

Copy link
Copy Markdown
Contributor

@ek-so ek-so left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm (I checked visually, don't see the difference)

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #85 / InfraOps App Metrics UI Node Details #Asset Type: host without metrics Overview Tab "before all" hook for "cpuUsage tile should be shown"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
alerting 221 220 -1
apm 1891 1890 -1
cases 1136 1135 -1
core 404 442 +38
datasetQuality 740 739 -1
discover 1340 1339 -1
embeddableAlertsTable 409 408 -1
esUiShared 192 191 -1
fleet 1366 1365 -1
infra 1437 1436 -1
ml 2417 2416 -1
monitoring 633 632 -1
observability 1324 1323 -1
onechat 94 93 -1
securitySolution 7777 7776 -1
slo 1189 1188 -1
synthetics 1253 1252 -1
transform 696 695 -1
triggersActionsUi 878 877 -1
uptime 775 774 -1
total +19

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/core-application-common 2 1 -1
@kbn/core-chrome-layout - 3 +3
@kbn/core-chrome-layout-components 4 6 +2
total +4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
alerting 93.7KB 91.4KB -2.2KB
apm 2.6MB 2.6MB +4.0B
automaticImport 1.1MB 1.1MB +89.0B
canvas 1.1MB 1.1MB -75.0B
cases 1.3MB 1.3MB -2.2KB
contentConnectors 470.0KB 470.1KB +89.0B
dashboard 617.7KB 617.8KB +89.0B
datasetQuality 420.6KB 418.4KB -2.2KB
discover 1.1MB 1.1MB -2.3KB
enterpriseSearch 1.2MB 1.2MB +89.0B
eventAnnotationListing 204.5KB 204.5KB +89.0B
filesManagement 101.5KB 101.6KB +89.0B
graph 370.5KB 370.6KB +89.0B
home 105.9KB 106.0KB +89.0B
indexManagement 673.2KB 673.3KB +89.0B
infra 1.0MB 1.0MB +89.0B
kibanaOverview 38.5KB 38.6KB +89.0B
management 33.2KB 33.3KB +89.0B
maps 3.1MB 3.1MB +59.0B
ml 5.4MB 5.4MB -2.2KB
monitoring 630.4KB 630.4KB +2.0B
observability 1.3MB 1.3MB -2.2KB
observabilityShared 37.7KB 37.8KB +89.0B
onechat 16.8KB 16.8KB +89.0B
osquery 1.0MB 1.0MB +89.0B
painlessLab 16.4KB 16.3KB -30.0B
searchHomepage 34.4KB 34.5KB +89.0B
searchIndices 193.5KB 193.6KB +89.0B
searchInferenceEndpoints 92.3KB 92.4KB +89.0B
searchPlayground 196.7KB 196.8KB +89.0B
searchprofiler 29.2KB 29.1KB -30.0B
searchQueryRules 118.8KB 118.9KB +89.0B
searchSynonyms 52.1KB 52.1KB +89.0B
security 502.4KB 502.5KB +89.0B
securitySolution 9.8MB 9.8MB -2.2KB
securitySolutionEss 35.0KB 35.1KB +89.0B
securitySolutionServerless 55.0KB 55.1KB +89.0B
spaces 209.0KB 209.1KB +89.0B
transform 623.8KB 621.6KB -2.2KB
uptime 490.9KB 490.9KB +5.0B
visualizations 373.7KB 373.7KB +89.0B
workchatApp 126.1KB 126.2KB +89.0B
total -15.5KB

Count of Enzyme imports

Enzyme is no longer supported, and we should switch to @testing-library/react instead.

id before after diff
@kbn/core-chrome-browser-internal 6 5 -1
@kbn/core-rendering-browser-internal 1 0 -1
total -2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 456.7KB 468.7KB +11.9KB
esUiShared 89.0KB 89.1KB +89.0B
fleet 168.6KB 166.4KB -2.2KB
onechat 10.9KB 10.9KB -21.0B
uptime 42.8KB 42.8KB +3.0B
total +9.8KB
Unknown metric groups

API count

id before after diff
@kbn/core-application-common 9 8 -1
@kbn/core-chrome-browser 212 211 -1
@kbn/core-chrome-layout - 3 +3
@kbn/core-chrome-layout-components 9 13 +4
total +5

History

@delanni delanni removed the release_note:skip Skip the PR/issue when compiling release notes label Jun 27, 2025
@delanni delanni added the release_note:skip Skip the PR/issue when compiling release notes label Jun 27, 2025
@jbudz jbudz added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting and removed release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Jun 27, 2025
@Dosant Dosant added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting and removed release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Jun 27, 2025
@Dosant Dosant merged commit fe9dcf7 into elastic:main Jun 27, 2025
33 checks passed
Dosant added a commit that referenced this pull request Jul 3, 2025
> [!IMPORTANT]
> **Should be no user-facing changes!!!** The new layout work is behind
a feature flag!

## Summary

Part of [workspace
chrome](elastic/kibana-team#1581 ) work.
This is follow up to #224255,
#225824 which mostly implements
the new layout for project (serverless) navigation.


![Screenshot 2025-07-01 at 15 04
45](https://github.com/user-attachments/assets/270a9871-4eec-49ac-bb75-75ea1b68916e)

### Implementation notes:

- The project navigation sidebar still uses the EuiCollapsibleNavBeta
component. It works as a push flyout on larger screens and as an overlay
on smaller screens.
- In the next steps, we will start using the new navigation grid cell
for the upcoming side navigation component.


## How to review 

1. Most importantly, we need to ensure that nothing is broken in the old
layout during the refactor. - Functional tests + visual/manual testing
2. Then for the new layout: 

kibana.yml:
```
feature_flags.overrides:
  core.chrome.layoutType: 'grid'
  core.chrome.layoutDebug: true
```

- Check that it mostly works (some specific edge cases and bugs are
expected and are gathered in
#225263)
- Code-review:  focus on the layout implementation split approach
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
)

> [!IMPORTANT]
> **Should be no user-facing changes!!!** The new layout work is behind
a feature flag!

## Summary

Part of [workspace
chrome](elastic/kibana-team#1581 ) work.
This is follow up to elastic#224255,
elastic#225824 which mostly implements
the new layout for project (serverless) navigation.


![Screenshot 2025-07-01 at 15 04
45](https://github.com/user-attachments/assets/270a9871-4eec-49ac-bb75-75ea1b68916e)

### Implementation notes:

- The project navigation sidebar still uses the EuiCollapsibleNavBeta
component. It works as a push flyout on larger screens and as an overlay
on smaller screens.
- In the next steps, we will start using the new navigation grid cell
for the upcoming side navigation component.


## How to review 

1. Most importantly, we need to ensure that nothing is broken in the old
layout during the refactor. - Functional tests + visual/manual testing
2. Then for the new layout: 

kibana.yml:
```
feature_flags.overrides:
  core.chrome.layoutType: 'grid'
  core.chrome.layoutDebug: true
```

- Check that it mostly works (some specific edge cases and bugs are
expected and are gathered in
elastic#225263)
- Code-review:  focus on the layout implementation split approach
@Dosant Dosant added the chrome-grid Work related to Chrome refactor to grid layout label Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting chrome-grid Work related to Chrome refactor to grid layout release_note:skip Skip the PR/issue when compiling release notes scss-removal Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants