-
Notifications
You must be signed in to change notification settings - Fork 8.5k
chore(scss): migrate core mixin kibanaFullBodyHeight to Emotion
#220494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fbe4ca7
8fc5718
ba8e7b7
15e6cab
0e6f790
914192c
7bbee3c
1396517
c7be8cb
aaa37c5
f7a0f7a
051f3e5
b0225da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,9 @@ | |
|
|
||
| // This file replaces scss core/public/_mixins.scss | ||
|
|
||
| import { css, keyframes } from '@emotion/react'; | ||
| import { COLOR_MODES_STANDARD, UseEuiTheme, euiCanAnimate } from '@elastic/eui'; | ||
| import { Interpolation, Theme, css, keyframes } from '@emotion/react'; | ||
| import { COLOR_MODES_STANDARD, UseEuiTheme, euiCanAnimate, useEuiTheme } from '@elastic/eui'; | ||
| import { useMemo } from 'react'; | ||
| import bg_top_branded from './styles/core_app/images/bg_top_branded.svg'; | ||
| import bg_top_branded_dark from './styles/core_app/images/bg_top_branded_dark.svg'; | ||
| import bg_bottom_branded from './styles/core_app/images/bg_bottom_branded.svg'; | ||
|
|
@@ -19,11 +20,10 @@ import bg_bottom_branded_dark from './styles/core_app/images/bg_bottom_branded_d | |
| // The `--kbnAppHeadersOffset` CSS variable is automatically updated by | ||
| // styles/rendering/_base.scss, based on whether the Kibana chrome has a | ||
| // header banner, app menu, and is visible or hidden | ||
| export const kibanaFullBodyHeightCss = (additionalOffset = 0) => css` | ||
| height: calc( | ||
| 100vh - var(--kbnAppHeadersOffset, var(--euiFixedHeadersOffset, 0)) - ${additionalOffset}px | ||
| ); | ||
| `; | ||
| export const kibanaFullBodyHeightCss = (additionalOffset = '0px') => | ||
| css({ | ||
| height: `calc(100vh - var(--kbnAppHeadersOffset, var(--euiFixedHeadersOffset, 0)) - ${additionalOffset})`, | ||
| }); | ||
|
|
||
| export const fullScreenGraphicsMixinStyles = (euiZLevel: number, euiTheme: UseEuiTheme) => { | ||
| const lightOrDarkTheme = (lightSvg: any, darkSvg: any) => { | ||
|
|
@@ -79,3 +79,40 @@ export const fullScreenGraphicsMixinStyles = (euiZLevel: number, euiTheme: UseEu | |
| }, | ||
| }); | ||
| }; | ||
|
|
||
| type StyleMap = Record< | ||
| string, | ||
| Interpolation<Theme> | ((theme: UseEuiTheme) => Interpolation<Theme>) | ||
| >; | ||
|
|
||
| type StaticStyleMap = Record<string, Interpolation<Theme>>; | ||
|
|
||
| /** | ||
| * Custom hook to reduce boilerplate when working with Emotion styles that may depend on | ||
| * the EUI theme. | ||
| * | ||
| * Accepts a map of styles where each entry is either a static Emotion style (via `css`) | ||
| * or a function that returns styles based on the current `euiTheme`. | ||
| * | ||
| * It returns a memoized version of the style map with all values resolved to static | ||
| * Emotion styles, allowing components to use a clean and unified object for styling. | ||
| * | ||
| * This helps simplify component code by centralizing theme-aware style logic. | ||
| * | ||
| * Example usage: | ||
| * const componentStyles = { | ||
| * container: css({ overflow: hidden }), | ||
| * leftPane: ({ euiTheme }) => css({ paddingTop: euiTheme.size.m }), | ||
| * } | ||
| * const styles = useMemoizedStyles(componentStyles); | ||
| */ | ||
| export const useMemoizedStyles = (styleMap: StyleMap) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is neat! Not intended as a blocker, but I wonder if we don't achieve a decent enough developer UX when grabbing the 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mbondyra, can you help me answer this question? 😇
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the explanation, that makes sense 👍🏻 |
||
| const euiThemeContext = useEuiTheme(); | ||
| const outputStyles = useMemo(() => { | ||
| return Object.entries(styleMap).reduce<StaticStyleMap>((acc, [key, value]) => { | ||
| acc[key] = typeof value === 'function' ? value(euiThemeContext) : value; | ||
| return acc; | ||
| }, {}); | ||
| }, [euiThemeContext, styleMap]); | ||
| return outputStyles; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,2 @@ | ||
| @import './css_variables'; | ||
| @import './mixins'; | ||
| @import './styles/index'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,3 @@ | ||
| @import '../../../../../../../src/core/public/mixins'; | ||
|
|
||
| .dshUnsavedListingItem { | ||
| margin-top: $euiSizeM; | ||
| } | ||
|
|
||
This file was deleted.
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File renamed because the linter complained about the name not following snake_case format.