chore(scss): migrate core mixin `kibanaFullBodyHeight` to Emotion by afharo · Pull Request #220494 · elastic/kibana

afharo · 2025-05-08T11:31:00Z

Summary

#214729 started migrating away from the mixin kibanaFullBodyHeight.
This PR removes the other usages and the mixin itself.

elasticmachine · 2025-05-08T15:16:25Z

Pinging @elastic/kibana-management (Team:Kibana Management)

elasticmachine · 2025-05-08T15:16:33Z

Pinging @elastic/kibana-presentation (Team:Presentation)

afharo · 2025-05-08T15:20:33Z

x-pack/platform/plugins/private/painless_lab/public/styles/_index.scss

-.painlessLabMainContainer {
-  @include kibanaFullBodyHeight($bodyOffset);
-}


I could have removed only this one... but I got carried over and ported all the CSS in this file 😬

afharo · 2025-05-08T15:21:15Z

x-pack/platform/plugins/shared/maps/public/_main.scss

-
 .euiColorPicker__emptySwatch {
  position: relative;
 }


I was not sure where this is leveraged, so I decided to keep it for now.

That appears to be an EUI-specific css class; however, I find no reference of it the EUI repo. I tried jumping back in time to older versions, but its hard to search that way... still didn't find anything.

Given it is scoped to the Maps app (#maps-plugin id), and the color pickers on the layers seem to behave the same with it commented out (tested that), I personally feel confident in removing it.

ryankeairns

Tested full screen in Discover, Dashboard, and Maps. All work as expected.
Changes lgtm.

mbondyra · 2025-05-09T08:48:29Z

x-pack/platform/plugins/shared/maps/public/routes/map_page/map_app/map_app.tsx

-        <div id="react-maps-root">
+        <div
+          id="react-maps-root"
+          css={css`


For performance reasons, we avoid inlining css inside components. Instead, we declare styles outside the component or memoize them when possible. Otherwise, the styles are re-serialized on every render. These are small (sub-millisecond) operations, but they accumulate—so as a rule, we should always avoid inline css unless absolutely necessary.
(Optimization I here: https://engineering.deptagency.com/optimizing-emotion-in-react)

mbondyra · 2025-05-09T08:56:25Z

x-pack/platform/plugins/shared/maps/public/routes/map_page/map_app/map_app.tsx

-      <div id="maps-plugin" className={this.props.isFullScreen ? 'mapFullScreen' : ''}>
+      <div
+        id="maps-plugin"
+        css={css`


For performance, avoid composing styles directly inside the css`` template. Instead, define static and conditional styles outside, and pass them as an array. This prevents re-serializing the template on each render and improves readability.

const mapsPluginStyles = { display: 'flex', flexDirection: 'column', width: '100%', overflow: 'hidden', }; const fullscreenStyles = { height: '100vh !important' }; css={[kibanaFullBodyHeightCss(), mapsPluginStyles, isFullScreen && fullscreenStyles]}

(reference read: https://douges.dev/blog/taming-the-beast-that-is-css-in-js)

mbondyra · 2025-05-09T09:02:38Z

x-pack/platform/plugins/shared/searchprofiler/public/application/app.tsx

+        css={css`
+          ${kibanaFullBodyHeightCss(bodyOffset)}
+          overflow: hidden;
+          flex-shrink: 1;
+        `}


The overflow and flex-shrink styles should be moved outside the component as a static object. But there's another issue here—we’ve recently agreed to avoid dynamic styles in css prop and use the style prop instead.

So the preferred approach is:

const mapsPluginStyles = { overflow: 'hidden', flexShrink: 1, }; css={mapsPluginStyles} style={kibanaFullBodyHeightCss(bodyOffset)}

Note: kibanaFullBodyHeightCss should be refactored to return a plain object instead of a css string, so it can be used with the style prop:

export const kibanaFullBodyHeightCss = (additionalOffset = 0) => ({ height: `calc( 100vh - var(--kbnAppHeadersOffset, var(--euiFixedHeadersOffset, 0)) - ${additionalOffset}px )`, });

🔗 Discussion reference

afharo · 2025-05-09T16:16:45Z

src/core/public/css_utils.ts

+export const kibanaFullBodyHeightCss = (additionalOffset = '0px') => ({
+  height: `calc(100vh - var(--kbnAppHeadersOffset, var(--euiFixedHeadersOffset, 0)) - ${additionalOffset})`,
+});


allowing a more free form input rather than integer px. This allows linking to vars, making other calculations, etc.

afharo · 2025-05-09T16:17:10Z

src/core/public/css_utils.ts

@@ -19,11 +19,9 @@ import bg_bottom_branded_dark from './styles/core_app/images/bg_bottom_branded_d
 // The `--kbnAppHeadersOffset` CSS variable is automatically updated by


File renamed because the linter complained about the name not following snake_case format.

afharo · 2025-05-09T16:17:46Z

src/platform/plugins/shared/discover/public/application/context/context_app.tsx

-const dscDocsPageCss = css`
-  ${kibanaFullBodyHeightCss(54)}; // 54px is the action bar height
-`;
+const dscDocsPageCss = kibanaFullBodyHeightCss('54px'); // 54px is the action bar height


no need to wrap it in a css template. Slight perf improvement.

afharo · 2025-05-09T16:20:34Z

x-pack/platform/plugins/shared/maps/public/routes/map_page/map_app/map_app_styles.ts

+export const mapFullScreenStyles = {
+  height: '100vh !important',
+};


intentionally not adding the type CSSObject because this type is not compatible with the style prop 🤷, and I'm using it there because it's a dynamic style.

jughosta

Data Discovery changes LGTM 👍

mattkime

Code changes look good and work well, thanks!

jloleysens

Nice job! Core changes LGTM. Did a code-only review and left a suggestion I'd like to get your thoughts on.

~~(I'm assuming code owners will check the appearance of their apps are as expected 🤞🏻 )~~ appears they did 😉

src/core/public/css_utils.ts

jloleysens · 2025-05-19T15:11:12Z

src/core/public/css_utils.ts

+ *   }
+ *   const styles = useMemoizedStyles(componentStyles);
+ */
+export const useMemoizedStyles = (styleMap: StyleMap) => {


This is neat!

Not intended as a blocker, but I wonder if we don't achieve a decent enough developer UX when grabbing the euiTheme object from css prop on components (e.g.).

Then to get the style definition "statically" colocated outside components we could use TS like:

// css_utils.ts import type { Attributes } from 'react'; // Helper type for getting CSS attribute from React export type CSSAttribute = Required<Attributes>['css']; export type StyleMap = Record<string, CSSAttribute>; // somewhere_else.ts import type { CSSAttribute, StyleMap } from 'css_utils' const styles: CSSAttribute = ({ euiTheme }) => ({ '& + &': { marginTop: euiTheme.size.s }, backgroundColor: euiTheme.colors.backgroundBasePrimary, body: '1vh', // ... }); <MyComponent css={styles} /> // or const styles = { container: () => ... } satisfies StyleMap; <MyComponent css={styles.container} />

...but I'm not an emotion expert! Perhaps your way is more performant/easier to use.

@mbondyra, can you help me answer this question? 😇

Hey @jloleysens — it's mostly a performance thing 🙂 With this approach, all styles are memoized.

That syntactic sugar you mentioned can actually hurt performance, since the function runs and re-serializes styles on every render—even though it looks cleaner in the JSX. There's also a small mistake in this comment: elastic/eui#6828 (comment) — defining the function outside the component doesn't help, because it still runs on each render as long as it’s a function.

Thanks for the explanation, that makes sense 👍🏻

Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>

kibanamachine · 2025-05-19T21:42:15Z

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/15123770596

elasticmachine · 2025-05-19T21:42:44Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: b0225da

Failed CI Steps

FTR Configs #18

Test Failures

[job] [logs] FTR Configs #18 / Actions and Triggers app Connectors General connector functionality should test a connector and display a successful result

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`painlessLab`	38	25	-13

Async chunks

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

id	before	after	diff
`discover`	1006.9KB	1007.0KB	+31.0B
`maps`	3.0MB	3.0MB	-309.0B
`painlessLab`	15.8KB	16.2KB	+444.0B
`searchprofiler`	43.0KB	43.2KB	+182.0B
total			+348.0B

Page load bundle

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

id	before	after	diff
`core`	439.6KB	439.8KB	+170.0B
`painlessLab`	11.0KB	5.7KB	-5.3KB
`searchprofiler`	16.5KB	16.4KB	-123.0B
total			-5.3KB

History

💔 Build #300629 failed f7a0f7a
💚 Build #300565 succeeded aaa37c5
💔 Build #300526 failed c7be8cb
💔 Build #300452 failed 7bbee3c
💔 Build #300449 failed 914192c
💚 Build #300171 succeeded ba8e7b7

cc @afharo

kibanamachine · 2025-05-19T21:48:29Z

💔 All backports failed

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 220494

Questions ?

Please refer to the Backport tool documentation

afharo · 2025-05-20T06:43:04Z

Cancelling backport because there are breaking changes in some of the modified files.

…astic#220494) Co-authored-by: mbondyra <marta.bondyra@elastic.co> Co-authored-by: Marta Bondyra <4283304+mbondyra@users.noreply.github.com> Co-authored-by: Matthew Kime <matt@mattki.me> Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>

kibanamachine · 2025-06-18T14:30:40Z

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/15735705068

kibanamachine · 2025-06-18T14:39:26Z

💔 All backports failed

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 220494

Questions ?

Please refer to the Backport tool documentation

chore(scss): migrate core mixin kibanaFullBodyHeight to Emotion

fbe4ca7

afharo self-assigned this May 8, 2025

afharo added backport:version Backport to applied version labels v8.19.0 labels May 8, 2025

Remove mixin import

8fc5718

afharo marked this pull request as ready for review May 8, 2025 15:15

afharo requested review from a team as code owners May 8, 2025 15:15

afharo added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// label May 8, 2025

afharo commented May 8, 2025

View reviewed changes

ThomThomson requested a review from mbondyra May 8, 2025 16:32

ryankeairns approved these changes May 8, 2025

View reviewed changes

mbondyra reviewed May 9, 2025

View reviewed changes

Address PR feedback

ba8e7b7

afharo requested review from a team as code owners May 9, 2025 16:15

afharo requested a review from mbondyra May 9, 2025 16:15

afharo commented May 9, 2025

View reviewed changes

jughosta approved these changes May 12, 2025

View reviewed changes

fix

15e6cab

Merge branch 'main' into core/remove-scss-mixin-kibanaFullBodyHeight

aaa37c5

mattkime approved these changes May 19, 2025

View reviewed changes

mattkime and others added 2 commits May 18, 2025 21:11

Merge branch 'main' into core/remove-scss-mixin-kibanaFullBodyHeight

f7a0f7a

Merge branch 'main' into core/remove-scss-mixin-kibanaFullBodyHeight

051f3e5

jloleysens approved these changes May 19, 2025

View reviewed changes

Update css_utils.ts

b0225da

Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>

afharo enabled auto-merge (squash) May 19, 2025 20:34

afharo merged commit 9c20f23 into elastic:main May 19, 2025
8 checks passed

afharo deleted the core/remove-scss-mixin-kibanaFullBodyHeight branch May 19, 2025 21:41

kibanamachine added the v9.1.0 label May 19, 2025

afharo added backport:skip This PR does not require backporting and removed backport:version Backport to applied version labels v8.19.0 labels May 20, 2025

mbondyra added backport:version Backport to applied version labels v8.19.0 and removed backport:skip This PR does not require backporting labels Jun 18, 2025

mbondyra added backport:skip This PR does not require backporting and removed backport:version Backport to applied version labels v8.19.0 labels Jun 18, 2025

kapral18 mentioned this pull request Sep 25, 2025

[Management] Refactor scss styles to emotion styles #205027

Closed

80 tasks

This was referenced Sep 30, 2025

[8.19] [Platform security] Replace SCSS with CSS in JS (part 2) (#233508) #236859

Closed

[8.18] [Platform security] Replace SCSS with CSS in JS (part 2) (#233508) #236860

Closed

		@@ -19,11 +19,9 @@ import bg_bottom_branded_dark from './styles/core_app/images/bg_bottom_branded_d
		// The `--kbnAppHeadersOffset` CSS variable is automatically updated by

Conversation

afharo commented May 8, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

elasticmachine commented May 8, 2025

Uh oh!

elasticmachine commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryankeairns left a comment

Choose a reason for hiding this comment

Uh oh!

mbondyra May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbondyra May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

mattkime left a comment

Choose a reason for hiding this comment

Uh oh!

jloleysens left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented May 19, 2025

Uh oh!

elasticmachine commented May 19, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Uh oh!

kibanamachine commented May 19, 2025

💔 All backports failed

Manual backport

Questions ?

Uh oh!

afharo commented May 20, 2025

Uh oh!

kibanamachine commented Jun 18, 2025

Uh oh!

kibanamachine commented Jun 18, 2025

💔 All backports failed

Manual backport

afharo commented May 8, 2025 •

edited by kibanamachine

Loading

mbondyra May 9, 2025 •

edited

Loading

mbondyra May 9, 2025 •

edited

Loading

jloleysens left a comment •

edited

Loading