From 44d996d9cd2a74e87601ca71345ef3b03285527f Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Mon, 28 Aug 2023 16:22:13 -0700 Subject: [PATCH 1/5] [EuiThemeProvider] Add context & state required for theme CSS variables architecture (#7132) --- .../__snapshots__/provider.test.tsx.snap | 6 ++ src/services/theme/context.ts | 2 + src/services/theme/hooks.test.tsx | 23 ++++- src/services/theme/hooks.tsx | 21 ++++ src/services/theme/index.ts | 7 +- src/services/theme/provider.stories.tsx | 98 +++++++++++++++++++ src/services/theme/provider.test.tsx | 81 ++++++++++++++- src/services/theme/provider.tsx | 82 ++++++++++++---- src/services/theme/types.ts | 6 ++ 9 files changed, 306 insertions(+), 20 deletions(-) create mode 100644 src/services/theme/provider.stories.tsx diff --git a/src/services/theme/__snapshots__/provider.test.tsx.snap b/src/services/theme/__snapshots__/provider.test.tsx.snap index 720b4738012..47336d01d76 100644 --- a/src/services/theme/__snapshots__/provider.test.tsx.snap +++ b/src/services/theme/__snapshots__/provider.test.tsx.snap @@ -1,5 +1,11 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`EuiThemeProvider CSS variables allows child components to set non-global theme CSS variables 1`] = ` + +`; + exports[`EuiThemeProvider nested EuiThemeProviders allows avoiding the extra span wrapper with \`wrapperProps.cloneElement\` 1`] = `
Top-level provider diff --git a/src/services/theme/context.ts b/src/services/theme/context.ts index fe50cda7ea1..d366a2ffba8 100644 --- a/src/services/theme/context.ts +++ b/src/services/theme/context.ts @@ -34,4 +34,6 @@ export const EuiNestedThemeContext = createContext({ hasDifferentColorFromGlobalTheme: false, bodyColor: '', colorClassName: '', + setGlobalCSSVariables: () => {}, + setNearestThemeCSSVariables: () => {}, }); diff --git a/src/services/theme/hooks.test.tsx b/src/services/theme/hooks.test.tsx index a931e9c5280..d3c5812ebd0 100644 --- a/src/services/theme/hooks.test.tsx +++ b/src/services/theme/hooks.test.tsx @@ -7,15 +7,18 @@ */ import React from 'react'; -import { render } from '@testing-library/react'; +import { render, act } from '@testing-library/react'; import { renderHook } from '../../test/rtl'; +import { EuiProvider } from '../../components'; + import { setEuiDevProviderWarning } from './warning'; import { useEuiTheme, UseEuiTheme, withEuiTheme, RenderWithEuiTheme, + useEuiThemeCSSVariables, } from './hooks'; describe('useEuiTheme', () => { @@ -82,3 +85,21 @@ describe('RenderWithEuiTheme', () => { ); }); }); + +describe('useEuiThemeCSSVariables', () => { + it('returns CSS variable related state setters/getters', () => { + const { result } = renderHook(useEuiThemeCSSVariables, { + wrapper: EuiProvider, + }); + expect(result.current.globalCSSVariables).toBeUndefined(); + expect(result.current.themeCSSVariables).toBeUndefined(); + + act(() => { + result.current.setNearestThemeCSSVariables({ '--hello': 'world' }); + }); + + // In this case, the nearest theme is the global one, so it should set both + expect(result.current.globalCSSVariables).toEqual({ '--hello': 'world' }); + expect(result.current.themeCSSVariables).toEqual({ '--hello': 'world' }); + }); +}); diff --git a/src/services/theme/hooks.tsx b/src/services/theme/hooks.tsx index 132d3a35f4f..bf87146612e 100644 --- a/src/services/theme/hooks.tsx +++ b/src/services/theme/hooks.tsx @@ -13,6 +13,7 @@ import { EuiModificationsContext, EuiColorModeContext, defaultComputedTheme, + EuiNestedThemeContext, } from './context'; import { emitEuiProviderWarning } from './warning'; import { @@ -95,3 +96,23 @@ export const RenderWithEuiTheme = ({ const theme = useEuiTheme(); return children(theme); }; + +/** + * Minor syntactical sugar hook for theme CSS variables. + * Primarily meant for internal EUI usage. + */ +export const useEuiThemeCSSVariables = () => { + const { + setGlobalCSSVariables, + globalCSSVariables, + setNearestThemeCSSVariables, + themeCSSVariables, + } = useContext(EuiNestedThemeContext); + + return { + setGlobalCSSVariables, + globalCSSVariables, + setNearestThemeCSSVariables, + themeCSSVariables, + }; +}; diff --git a/src/services/theme/index.ts b/src/services/theme/index.ts index 7c46c4f81ff..d290b7e0a46 100644 --- a/src/services/theme/index.ts +++ b/src/services/theme/index.ts @@ -14,7 +14,12 @@ export { EuiColorModeContext, } from './context'; export type { UseEuiTheme, WithEuiThemeProps } from './hooks'; -export { useEuiTheme, withEuiTheme, RenderWithEuiTheme } from './hooks'; +export { + useEuiTheme, + withEuiTheme, + RenderWithEuiTheme, + useEuiThemeCSSVariables, +} from './hooks'; export type { EuiThemeProviderProps } from './provider'; export { EuiThemeProvider } from './provider'; export { getEuiDevProviderWarning, setEuiDevProviderWarning } from './warning'; diff --git a/src/services/theme/provider.stories.tsx b/src/services/theme/provider.stories.tsx new file mode 100644 index 00000000000..2b2fbe33d7b --- /dev/null +++ b/src/services/theme/provider.stories.tsx @@ -0,0 +1,98 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { FunctionComponent, useEffect } from 'react'; +import type { Meta, StoryObj } from '@storybook/react'; + +import { useEuiThemeCSSVariables } from './hooks'; +import { EuiThemeProvider, EuiThemeProviderProps } from './provider'; + +const meta: Meta> = { + title: 'EuiThemeProvider', + component: EuiThemeProvider, +}; + +export default meta; +type Story = StoryObj>; + +export const WrapperCloneElement: Story = { + render: () => ( + <> + +
+ This example should only have 1 main wrapper rendered. +
+
+ + ), +}; + +export const CSSVariablesNearest: Story = { + render: () => ( + <> + + This component sets the nearest theme provider (the global theme) with a + red CSS variable color. Inspect the `:root` styles to see the variable + set. + + + + This component sets the nearest local theme provider with a blue CSS + variable color. Inspect the parent theme wrapper to see the variable + set. + + + + ), +}; + +export const CSSVariablesGlobal: Story = { + render: () => ( + <> + + This component sets the nearest theme provider (the global theme) with a + red CSS variable color. However, it should be overridden by the next + component. + + + + This component sets the global theme with a blue CSS variable color. + It should override the previous component. Inspect the `:root` styles + to see this behavior + + + + ), +}; + +/** + * Component for QA/testing purposes that mocks an EUI component + * that sets global or theme-level CSS variables + */ +const MockComponent: FunctionComponent<{ + global?: boolean; + color: string; + children: any; +}> = ({ global, color, children }) => { + const { setGlobalCSSVariables, setNearestThemeCSSVariables } = + useEuiThemeCSSVariables(); + + useEffect(() => { + if (global) { + setGlobalCSSVariables({ '--testColor': color }); + } else { + setNearestThemeCSSVariables({ '--testColor': color }); + } + }, [global, color, setGlobalCSSVariables, setNearestThemeCSSVariables]); + + return ( +

+ {children} +

+ ); +}; diff --git a/src/services/theme/provider.test.tsx b/src/services/theme/provider.test.tsx index 519cd645f87..3b0368723c5 100644 --- a/src/services/theme/provider.test.tsx +++ b/src/services/theme/provider.test.tsx @@ -6,11 +6,12 @@ * Side Public License, v 1. */ -import React from 'react'; +import React, { FunctionComponent, useContext, useEffect } from 'react'; import { render } from '@testing-library/react'; // Note - don't use the EUI custom RTL `render`, as it auto-wraps an `EuiProvider` import { css } from '@emotion/react'; import { EuiProvider } from '../../components/provider'; +import { EuiNestedThemeContext } from './context'; import { EuiThemeProvider } from './provider'; describe('EuiThemeProvider', () => { @@ -136,4 +137,82 @@ describe('EuiThemeProvider', () => { expect(container.querySelector('.hello.world')).toBeTruthy(); }); }); + + describe('CSS variables', () => { + const MockEuiComponent: FunctionComponent<{ global?: boolean }> = ({ + global, + }) => { + const { + globalCSSVariables, + setGlobalCSSVariables, + setNearestThemeCSSVariables, + } = useContext(EuiNestedThemeContext); + + useEffect(() => { + if (global) { + setGlobalCSSVariables({ '--hello': 'global-world' }); + } else { + setNearestThemeCSSVariables({ '--hello': 'world' }); + } + }, [global, setGlobalCSSVariables, setNearestThemeCSSVariables]); + + // Our current version of jsdom doesn't yet support :root (currently on v11, + // need to be on at least v20), so we'll mock something to assert on in the interim + return <>{JSON.stringify(globalCSSVariables)}; + }; + + const getThemeProvider = (container: HTMLElement) => + container.querySelector('.euiThemeProvider')!; + const getThemeClassName = (container: HTMLElement) => + getThemeProvider(container).className; + + it('allows child components to set non-global theme CSS variables', () => { + const { container } = render( + + + + + + ); + expect(getThemeClassName(container)).toContain('euiCSSVariables'); + expect(container.firstChild).toHaveStyleRule('--hello', 'world'); + expect(container.firstChild).toMatchSnapshot(); + }); + + it('sets global CSS variables when the nearest theme provider is the top-level one', () => { + const { container } = render( + + + + ); + expect(container.textContent).toContain('{"--hello":"world"}'); + }); + + it('allows child components to set global CSS variables from any nested theme provider', () => { + const { container } = render( + + + + + + ); + expect(getThemeClassName(container)).not.toContain('euiCSSVariables'); + expect(container.textContent).toContain('{"--hello":"global-world"}'); + }); + + it('can set both global and nearest theme variables without conflicting', () => { + const { container } = render( + + + + + + + + ); + expect(getThemeClassName(container)).toContain('euiCSSVariables'); + expect(getThemeProvider(container)).toHaveStyleRule('--hello', 'world'); + expect(container.textContent).toContain('{"--hello":"global-world"}'); + }); + }); }); diff --git a/src/services/theme/provider.tsx b/src/services/theme/provider.tsx index 13003b54d2e..0005f356d9f 100644 --- a/src/services/theme/provider.tsx +++ b/src/services/theme/provider.tsx @@ -12,14 +12,17 @@ import React, { useRef, useMemo, useState, + useCallback, PropsWithChildren, HTMLAttributes, } from 'react'; import classNames from 'classnames'; import { css } from '@emotion/css'; +import { Global, type CSSObject } from '@emotion/react'; import isEqual from 'lodash/isEqual'; import type { CommonProps } from '../../components/common'; +import { cloneElementWithCss } from '../emotion'; import { EuiSystemContext, @@ -63,7 +66,12 @@ export const EuiThemeProvider = ({ children, wrapperProps, }: EuiThemeProviderProps) => { - const { isGlobalTheme, bodyColor } = useContext(EuiNestedThemeContext); + const { + isGlobalTheme, + bodyColor, + globalCSSVariables, + setGlobalCSSVariables, + } = useContext(EuiNestedThemeContext); const parentSystem = useContext(EuiSystemContext); const parentModifications = useContext(EuiModificationsContext); const parentColorMode = useContext(EuiColorModeContext); @@ -137,6 +145,13 @@ export const EuiThemeProvider = ({ } }, [colorMode, system, modifications]); + const [themeCSSVariables, _setThemeCSSVariables] = useState(); + const setThemeCSSVariables = useCallback( + (variables: CSSObject) => + _setThemeCSSVariables((previous) => ({ ...previous, ...variables })), + [] + ); + const nestedThemeContext = useMemo(() => { return { isGlobalTheme: false, // The theme that determines the global body styles @@ -148,8 +163,25 @@ export const EuiThemeProvider = ({ label: euiColorMode-${_colorMode}; color: ${theme.colors.text}; `, + setGlobalCSSVariables: isGlobalTheme + ? setThemeCSSVariables + : setGlobalCSSVariables, + globalCSSVariables: isGlobalTheme + ? themeCSSVariables + : globalCSSVariables, + setNearestThemeCSSVariables: setThemeCSSVariables, + themeCSSVariables: themeCSSVariables, }; - }, [theme, isGlobalTheme, bodyColor, _colorMode]); + }, [ + theme, + isGlobalTheme, + bodyColor, + _colorMode, + setGlobalCSSVariables, + globalCSSVariables, + setThemeCSSVariables, + themeCSSVariables, + ]); const renderedChildren = useMemo(() => { if (isGlobalTheme) { @@ -161,9 +193,14 @@ export const EuiThemeProvider = ({ ...rest, className: classNames(className, nestedThemeContext.colorClassName), }; + // Condition avoids rendering an empty Emotion selector if no + // theme-specific CSS variables have been set by child components + if (themeCSSVariables) { + props.css = { label: 'euiCSSVariables', ...themeCSSVariables }; + } if (cloneElement) { - return React.cloneElement(children, { + return cloneElementWithCss(children, { ...props, className: classNames(children.props.className, props.className), }); @@ -177,21 +214,32 @@ export const EuiThemeProvider = ({ ); } - }, [isGlobalTheme, nestedThemeContext, wrapperProps, children]); + }, [ + isGlobalTheme, + themeCSSVariables, + nestedThemeContext, + wrapperProps, + children, + ]); return ( - - - - - - - {renderedChildren} - - - - - - + <> + {isGlobalTheme && themeCSSVariables && ( + + )} + + + + + + + {renderedChildren} + + + + + + + ); }; diff --git a/src/services/theme/types.ts b/src/services/theme/types.ts index b719e135889..04b93b40f20 100644 --- a/src/services/theme/types.ts +++ b/src/services/theme/types.ts @@ -6,6 +6,8 @@ * Side Public License, v 1. */ +import type { CSSObject } from '@emotion/react'; + import { RecursivePartial, ValueOf } from '../../components/common'; import { _EuiThemeAnimation } from '../../global_styling/variables/animations'; import { _EuiThemeBreakpoints } from '../../global_styling/variables/breakpoint'; @@ -99,4 +101,8 @@ export type EuiThemeNested = { hasDifferentColorFromGlobalTheme: boolean; bodyColor: string; colorClassName: string; + setGlobalCSSVariables: Function; + globalCSSVariables?: CSSObject; + setNearestThemeCSSVariables: Function; + themeCSSVariables?: CSSObject; }; From f04779589830318186a9454c6044a5fd941681e4 Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Fri, 1 Sep 2023 09:25:21 -0700 Subject: [PATCH 2/5] [EuiHeader] Update to set a new global `--euiFixedHeadersOffset` CSS variable (#7144) Co-authored-by: Trevor Pierce <1Copenut@users.noreply.github.com> --- .../components/guide_page/_guide_page.scss | 17 --- .../guide_page/guide_page_chrome.js | 2 +- .../views/header/header_elastic_pattern.tsx | 10 +- src-docs/src/views/header/header_example.js | 40 ++++--- src-docs/src/views/header/header_stacked.tsx | 10 +- .../collapsible_nav_beta.stories.tsx | 4 +- .../collapsible_nav_beta.styles.ts | 16 ++- .../collapsible_nav_beta.test.tsx | 16 --- .../collapsible_nav_beta.tsx | 32 +----- src/components/datagrid/_data_grid.scss | 15 ++- .../flyout/__snapshots__/flyout.test.tsx.snap | 2 +- src/components/flyout/flyout.styles.ts | 4 +- .../header/__snapshots__/header.test.tsx.snap | 1 + src/components/header/header.stories.tsx | 77 ++++++++++++- src/components/header/header.styles.ts | 4 - src/components/header/header.test.tsx | 44 +++++++- src/components/header/header.tsx | 104 +++++++++++++----- .../overlay_mask/overlay_mask.styles.ts | 3 +- .../overlay_mask/overlay_mask.test.tsx | 2 +- .../page/page_sidebar/page_sidebar.test.tsx | 15 ++- .../page/page_sidebar/page_sidebar.tsx | 10 +- .../__snapshots__/page_template.test.tsx.snap | 36 +++--- .../page_template/page_template.tsx | 26 +---- src/global_styling/mixins/_header.scss | 2 + src/global_styling/mixins/_helpers.ts | 4 +- upcoming_changelogs/7144.md | 8 ++ 26 files changed, 300 insertions(+), 204 deletions(-) create mode 100644 upcoming_changelogs/7144.md diff --git a/src-docs/src/components/guide_page/_guide_page.scss b/src-docs/src/components/guide_page/_guide_page.scss index 27e8150a5a1..af407b0b542 100644 --- a/src-docs/src/components/guide_page/_guide_page.scss +++ b/src-docs/src/components/guide_page/_guide_page.scss @@ -1,22 +1,5 @@ @import '../../../../src/global_styling/mixins/helpers'; -@include euiHeaderAffordForFixed; - -.guideBody { - // Override euiHeaderAffordForFixed mixin since the page template handles this now - padding-top: 0 !important; // stylelint-disable-line declaration-no-important -} - -.euiBody--headerIsFixed--double { - @include euiHeaderAffordForFixed($euiHeaderHeightCompensation * 2); - - .euiHeader:not([data-fixed-header]) { - // Force headers below the fullscreen. - // This shouldn't be necessary in consuming applications because headers should always be at the top of the page - z-index: 0; - } -} - .guideSideNav { @include euiSideNavEmbellish; } diff --git a/src-docs/src/components/guide_page/guide_page_chrome.js b/src-docs/src/components/guide_page/guide_page_chrome.js index a935a04b03b..7cb7d732f2e 100644 --- a/src-docs/src/components/guide_page/guide_page_chrome.js +++ b/src-docs/src/components/guide_page/guide_page_chrome.js @@ -69,7 +69,7 @@ export class GuidePageChrome extends Component { scrollNavSectionIntoView = () => { // wait a bit for react to blow away and re-create the DOM // then scroll the selected nav section into view - requestAnimationFrame(() => { + setTimeout(() => { const sideNav = document.querySelector('.guideSideNav__content'); const isMobile = sideNav?.querySelector('.euiSideNav__mobileToggle'); diff --git a/src-docs/src/views/header/header_elastic_pattern.tsx b/src-docs/src/views/header/header_elastic_pattern.tsx index 5527ff4f629..0db0abdd832 100644 --- a/src-docs/src/views/header/header_elastic_pattern.tsx +++ b/src-docs/src/views/header/header_elastic_pattern.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState } from 'react'; import { Link } from 'react-router-dom'; import { @@ -53,14 +53,6 @@ export default () => { prefix: 'guideHeaderDeploymentPopover', }); - useEffect(() => { - document.body.classList.add('euiBody--headerIsFixed--double'); - - return () => { - document.body.classList.remove('euiBody--headerIsFixed--double'); - }; - }, []); - /** * Collapsible Nav */ diff --git a/src-docs/src/views/header/header_example.js b/src-docs/src/views/header/header_example.js index 89fe62c334d..45df96182d0 100644 --- a/src-docs/src/views/header/header_example.js +++ b/src-docs/src/views/header/header_example.js @@ -298,24 +298,35 @@ export const HeaderExample = { <>

Most consumers need a header that does not scroll away with the page - contents. You can apply this display by applying the property{' '} - {'position="fixed"'}. This will - also add a class of .euiBody--headerIsFixed to - the window body. + contents. You can set this display by applying the property{' '} + {'position="fixed"'}. Multiple + fixed headers will automatically stack underneath one another. No + manual positioning is required.

- You will then need to apply your own padding to this body class to - afford for the header height. EUI supplies a helper mixin that also - accounts for this height in flyouts and the collapsible nav. Simply - add{' '} - @include euiHeaderAffordForFixed;{' '} - anywhere in your SASS. + If you're using{' '} + + EuiPageTemplate + + , a padding top will be automatically set based on the number of + fixed headers on the page.{' '} + + EuiFlyouts + {' '} + will also automatically account for and sit below fixed headers. +

+

+ If you're using your own custom layout, or have custom UI that needs + to sit below your fixed header(s), EUI provides a global CSS{' '} + var(--euiFixedHeadersOffset){' '} + variable. You can use this variable anywhere, or even override it, + to correctly offset any and all fixed header heights.

), snippet: [ '', - '@include euiHeaderAffordForFixed;', + 'body { padding-block-start: var(--euiFixedHeadersOffset, 0) }', ], demo: , demoPanelProps: { @@ -446,17 +457,12 @@ export const HeaderExample = { text: (

Stacking multiple headers provides a great way to separate global - navigation concerns. However, the{' '} - {'position="fixed"'} option will not - be aware of the number of headers. If you do need fixed{' '} - and stacked headers, you will need to apply the SASS - helper mixin and pass in the correct height to afford for. + navigation concerns.

), snippet: [ ` `, - '@include euiHeaderAffordForFixed($euiHeaderHeightCompensation * 2);', ], demo: , demoPanelProps: { diff --git a/src-docs/src/views/header/header_stacked.tsx b/src-docs/src/views/header/header_stacked.tsx index 24033bc1162..e1714b97d2c 100644 --- a/src-docs/src/views/header/header_stacked.tsx +++ b/src-docs/src/views/header/header_stacked.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState } from 'react'; import { EuiBreadcrumb, @@ -27,14 +27,6 @@ export default () => { }, ]; - useEffect(() => { - if (isFixed) document.body.classList.add('euiBody--headerIsFixed--double'); - - return () => { - document.body.classList.remove('euiBody--headerIsFixed--double'); - }; - }, [isFixed]); - const headers = ( <> - This story tests that EuiCollapsibleNav's fixed header detection & - offsetting works as expected + This story tests that EuiCollapsibleNav automatically adjusts its + position & height for multiple fixed headers Second header diff --git a/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts b/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts index 531b03cef06..d49cae7c31d 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts +++ b/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts @@ -10,18 +10,30 @@ import { css } from '@emotion/react'; import { UseEuiTheme } from '../../services'; import { logicalCSS, euiYScroll } from '../../global_styling'; import { euiShadowFlat } from '../../themes'; +import { euiHeaderVariables } from '../header/header.styles'; export const euiCollapsibleNavBetaStyles = (euiThemeContext: UseEuiTheme) => { const { euiTheme } = euiThemeContext; + // At least for serverless, EuiCollapsibleNav is only going to be used with 1 + // fixed header. For those scenarios, we can prevent a minor layout jump on + // page load by setting the CSS var fallback to the height of a single header + const defaultHeaderHeight = euiHeaderVariables(euiThemeContext).height; + const fixedHeaderOffset = `var(--euiFixedHeadersOffset, ${defaultHeaderHeight})`; + return { euiCollapsibleNavBeta: css` + /* Fixed header affordance */ + ${logicalCSS('top', fixedHeaderOffset)} + ${logicalCSS('height', `calc(100% - ${fixedHeaderOffset})`)} + /* This extra padding is needed for EuiPopovers to have enough space to render with the right anchorPosition */ ${logicalCSS('padding-bottom', euiTheme.size.xs)} - /* Allow the nav to scroll, in case consumers don't use EuiFlyoutBody/EuiFyoutFooter */ - ${euiYScroll(euiThemeContext)} + /* Allow the nav to scroll, in case consumers don't use EuiFlyoutBody/EuiFyoutFooter + Height is already set by header affordance above */ + ${euiYScroll(euiThemeContext, { height: false })} /* In case things get really dire responsively, ensure the footer doesn't overtake the body */ .euiFlyoutBody { diff --git a/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx b/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx index 2433909559c..4d594b35871 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx @@ -12,8 +12,6 @@ import { render } from '../../test/rtl'; import { shouldRenderCustomStyles } from '../../test/internal'; import { requiredProps } from '../../test'; -import { EuiHeader } from '../header'; - import { EuiCollapsibleNavBeta } from './collapsible_nav_beta'; describe('EuiCollapsibleNavBeta', () => { @@ -66,20 +64,6 @@ describe('EuiCollapsibleNavBeta', () => { expect(onCollapseToggle).toHaveBeenLastCalledWith(true); }); - it('automatically accounts for fixed EuiHeaders in its positioning', () => { - const { getByTestSubject } = render( - - - Nav content - - - ); - expect(getByTestSubject('nav')).toHaveStyle({ - 'inset-block-start': '48px', - 'block-size': 'calc(100% - 48px)', - }); - }); - describe('responsive behavior', () => { const mockWindowResize = (width: number) => { window.innerWidth = width; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_beta.tsx b/src/components/collapsible_nav_beta/collapsible_nav_beta.tsx index a549a4dbb95..94eed977152 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_beta.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_beta.tsx @@ -19,7 +19,6 @@ import React, { import classNames from 'classnames'; import { useEuiTheme, useGeneratedHtmlId, throttle } from '../../services'; -import { mathWithUnits, logicalStyle } from '../../global_styling'; import { CommonProps } from '../common'; import { EuiFlyout, EuiFlyoutProps } from '../flyout'; @@ -83,7 +82,6 @@ export const EuiCollapsibleNavBeta: FunctionComponent< id, children, className, - style, initialIsCollapsed = false, onCollapseToggle, width: _width = 248, @@ -144,32 +142,6 @@ export const EuiCollapsibleNavBeta: FunctionComponent< return _width; }, [_width, isOverlayFullWidth, isPush, isCollapsed, headerHeight]); - /** - * Header affordance - */ - const [fixedHeadersCount, setFixedHeadersCount] = useState( - false - ); - useEffect(() => { - setFixedHeadersCount( - document.querySelectorAll('.euiHeader[data-fixed-header]').length - ); - }, []); - - const stylesWithHeaderOffset = useMemo(() => { - if (!fixedHeadersCount) return style; - - const headersOffset = mathWithUnits( - headerHeight, - (x) => x * fixedHeadersCount - ); - return { - ...style, - ...logicalStyle('top', headersOffset), - ...logicalStyle('height', `calc(100% - ${headersOffset})`), - }; - }, [fixedHeadersCount, style, headerHeight]); - /** * Prop setup */ @@ -205,15 +177,13 @@ export const EuiCollapsibleNavBeta: FunctionComponent< isOverlayFullWidth && styles.isOverlayFullWidth, ]; - // Wait for any fixed headers to be queried before rendering (prevents position jumping) - const flyout = fixedHeadersCount !== false && ( + const flyout = (
{ return { euiFlyout: css` position: fixed; - ${logicalCSS('top', 0)} ${logicalCSS('bottom', 0)} - ${logicalCSS('height', '100%')} + ${logicalCSS('top', 'var(--euiFixedHeadersOffset, 0)')} + ${logicalCSS('height', 'calc(100% - var(--euiFixedHeadersOffset, 0))')} z-index: ${euiTheme.levels.flyout}; background: ${euiTheme.colors.emptyShade}; display: flex; diff --git a/src/components/header/__snapshots__/header.test.tsx.snap b/src/components/header/__snapshots__/header.test.tsx.snap index ba5bb7e1070..222aeff6d1c 100644 --- a/src/components/header/__snapshots__/header.test.tsx.snap +++ b/src/components/header/__snapshots__/header.test.tsx.snap @@ -28,6 +28,7 @@ exports[`EuiHeader renders in fixed position 1`] = `
Hello! diff --git a/src/components/header/header.stories.tsx b/src/components/header/header.stories.tsx index 7d4ca7159df..0e458880861 100644 --- a/src/components/header/header.stories.tsx +++ b/src/components/header/header.stories.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import React from 'react'; +import React, { useState } from 'react'; import type { Meta, StoryObj } from '@storybook/react'; import { @@ -16,6 +16,9 @@ import { EuiHeaderLink, EuiIcon, EuiAvatar, + EuiButton, + EuiFlyout, + EuiPageTemplate, } from '../../components'; import { EuiHeader, EuiHeaderProps } from './header'; @@ -80,3 +83,75 @@ export const Sections: Story = { ], }, }; + +export const MultipleFixedHeaders: Story = { + parameters: { + layout: 'fullscreen', + }, + render: () => { + const [fixedHeadersCount, setFixedHeadersCount] = useState(3); // eslint-disable-line react-hooks/rules-of-hooks + const [isFlyoutOpen, setIsFlyoutOpen] = useState(false); // eslint-disable-line react-hooks/rules-of-hooks + + const sections = [ + { + items: [ + , + ], + }, + { + items: [ + setIsFlyoutOpen(!isFlyoutOpen)}> + Toggle flyout + , + ], + }, + ]; + + return ( + + + The page template and flyout should automatically adjust dynamically + to the number of fixed headers on the page. + {isFlyoutOpen && ( + setIsFlyoutOpen(false)}> + The flyout position and mask should automatically adjust + dynamically to the number of fixed headers on the page. + + )} +
+
+ setFixedHeadersCount((count) => count - 1)} + > + Remove a fixed header + +   + setFixedHeadersCount((count) => count + 1)} + > + Add a fixed header + +
+
+ {/* Always render at least one static header so we can toggle/test the flyout */} + + {/* Conditionally render additional fixed headers */} + {Array.from({ length: fixedHeadersCount - 1 }).map((_, i) => ( + + ))} +
+
+ ); + }, +}; diff --git a/src/components/header/header.styles.ts b/src/components/header/header.styles.ts index 8bddba5f60f..0bf9ebf823b 100644 --- a/src/components/header/header.styles.ts +++ b/src/components/header/header.styles.ts @@ -57,10 +57,6 @@ export const euiHeaderStyles = (euiThemeContext: UseEuiTheme) => { position: fixed; ${logicalCSS('top', 0)} ${logicalCSS('horizontal', 0)} - - & + [data-fixed-header] { - ${logicalCSS('top', height)} - } `, // Theme default: css` diff --git a/src/components/header/header.test.tsx b/src/components/header/header.test.tsx index e709c36a1c1..8657dfb9341 100644 --- a/src/components/header/header.test.tsx +++ b/src/components/header/header.test.tsx @@ -11,7 +11,7 @@ import { render } from '../../test/rtl'; import { requiredProps } from '../../test/required_props'; import { shouldRenderCustomStyles } from '../../test/internal'; -import { EuiHeader } from './header'; +import { euiFixedHeadersCount, EuiFixedHeader, EuiHeader } from './header'; describe('EuiHeader', () => { shouldRenderCustomStyles(); @@ -125,3 +125,45 @@ describe('EuiHeader', () => { }); }); }); + +describe('EuiFixedHeader', () => { + describe('on mount/unmount', () => { + it('updates the fixed headers count and body className', () => { + const { unmount } = render( + <> + + + + + ); + expect(euiFixedHeadersCount).toEqual(3); + // TODO: we're not yet on a jsdom version that supports inspecting :root + expect(document.body.className).toContain('euiBody--headerIsFixed'); + + unmount(); + expect(euiFixedHeadersCount).toEqual(0); + // TODO: we're not yet on a jsdom version that supports inspecting :root + expect(document.body.className).not.toContain('euiBody--headerIsFixed'); + }); + }); + + it('sets the inline position styles of headers', () => { + const { getByTestSubject } = render( + <> + + + + + ); + expect(getByTestSubject('first')).toHaveStyle('inset-block-start: 0px'); + expect(getByTestSubject('second')).toHaveStyle('inset-block-start: 48px'); + expect(getByTestSubject('last')).toHaveStyle('inset-block-start: 96px'); + }); + + it('allows consumers to override inline styles as needed', () => { + const { container } = render( + + ); + expect(container.firstElementChild).toHaveStyle('inset-block-start: 20px'); + }); +}); diff --git a/src/components/header/header.tsx b/src/components/header/header.tsx index bc5173f1782..985fb2bc294 100644 --- a/src/components/header/header.tsx +++ b/src/components/header/header.tsx @@ -6,10 +6,18 @@ * Side Public License, v 1. */ -import React, { FunctionComponent, HTMLAttributes, useEffect } from 'react'; +import React, { + FunctionComponent, + HTMLAttributes, + useEffect, + useState, + useMemo, + useCallback, +} from 'react'; import classNames from 'classnames'; -import { useEuiTheme } from '../../services'; +import { useEuiTheme, useEuiThemeCSSVariables } from '../../services'; +import { mathWithUnits, logicalStyles } from '../../global_styling'; import { CommonProps } from '../common'; import { EuiBreadcrumb, EuiBreadcrumbsProps } from '../breadcrumbs'; @@ -19,7 +27,7 @@ import { EuiHeaderSectionItemProps, EuiHeaderSection, } from './header_section'; -import { euiHeaderStyles } from './header.styles'; +import { euiHeaderStyles, euiHeaderVariables } from './header.styles'; type EuiHeaderSectionItemType = EuiHeaderSectionItemProps['children']; @@ -66,9 +74,6 @@ export type EuiHeaderProps = CommonProps & theme?: 'default' | 'dark'; }; -// Start a counter to manage the total number of fixed headers that need the body class -let euiHeaderFixedCounter = 0; - export const EuiHeader: FunctionComponent = ({ children, className, @@ -83,24 +88,6 @@ export const EuiHeader: FunctionComponent = ({ const styles = euiHeaderStyles(euiTheme); const cssStyles = [styles.euiHeader, styles[position], styles[theme]]; - useEffect(() => { - if (position === 'fixed') { - // Increment fixed header counter for each fixed header - euiHeaderFixedCounter++; - document.body.classList.add('euiBody--headerIsFixed'); - document.body.dataset.fixedHeaders = String(euiHeaderFixedCounter); - - return () => { - // Both decrement the fixed counter AND then check if there are none - if (--euiHeaderFixedCounter === 0) { - // If there are none, THEN remove class - document.body.classList.remove('euiBody--headerIsFixed'); - delete document.body.dataset.fixedHeaders; - } - }; - } - }, [position]); - let contents; if (sections) { if (children) { @@ -137,14 +124,75 @@ export const EuiHeader: FunctionComponent = ({ contents = children; } + return position === 'fixed' ? ( + + {contents} + + ) : ( +
+ {contents} +
+ ); +}; + +/** + * Fixed headers - logic around dynamically calculating the total + * page offset and setting the `top` position of subsequent headers + */ + +// Start a counter to manage the total number of fixed headers +// Exported for unit testing only +export let euiFixedHeadersCount = 0; + +// Exported for unit testing only +export const EuiFixedHeader: FunctionComponent = ({ + children, + style, + ...rest +}) => { + const { setGlobalCSSVariables } = useEuiThemeCSSVariables(); + const euiTheme = useEuiTheme(); + const headerHeight = euiHeaderVariables(euiTheme).height; + const getHeaderOffset = useCallback( + () => mathWithUnits(headerHeight, (x) => x * euiFixedHeadersCount), + [headerHeight] + ); + const [topPosition, setTopPosition] = useState(); + + useEffect(() => { + // Get the top position from the offset of previous header(s) + setTopPosition(getHeaderOffset()); + + // Increment fixed header counter for each fixed header + euiFixedHeadersCount++; + setGlobalCSSVariables({ + '--euiFixedHeadersOffset': getHeaderOffset(), + }); + document.body.classList.add('euiBody--headerIsFixed'); // TODO: Consider deleting this legacy className + + return () => { + euiFixedHeadersCount--; + setGlobalCSSVariables({ + '--euiFixedHeadersOffset': getHeaderOffset(), + }); + if (euiFixedHeadersCount === 0) { + document.body.classList.remove('euiBody--headerIsFixed'); // TODO: Consider deleting this legacy className + } + }; + }, [getHeaderOffset, setGlobalCSSVariables]); + + const inlineStyles = useMemo( + () => logicalStyles({ top: topPosition, ...style }), + [topPosition, style] + ); + return (
- {contents} + {children}
); }; diff --git a/src/components/overlay_mask/overlay_mask.styles.ts b/src/components/overlay_mask/overlay_mask.styles.ts index 1f67f9a2a81..54baf0490a7 100644 --- a/src/components/overlay_mask/overlay_mask.styles.ts +++ b/src/components/overlay_mask/overlay_mask.styles.ts @@ -29,7 +29,6 @@ export const euiOverlayMaskStyles = ({ euiTheme }: UseEuiTheme) => ({ `, belowHeader: css` z-index: ${euiTheme.levels.maskBelowHeader}; - /* TODO: use size variable when EuiHeader is converted */ - ${logicalCSS('top', `${euiTheme.base * 3}px`)} + ${logicalCSS('top', 'var(--euiFixedHeadersOffset, 0)')} `, }); diff --git a/src/components/overlay_mask/overlay_mask.test.tsx b/src/components/overlay_mask/overlay_mask.test.tsx index f99264b8393..1fecde19718 100644 --- a/src/components/overlay_mask/overlay_mask.test.tsx +++ b/src/components/overlay_mask/overlay_mask.test.tsx @@ -54,7 +54,7 @@ describe('EuiOverlayMask', () => { ); expect(getClassName()).toMatchInlineSnapshot( - `"euiOverlayMask css-1sm7f9p-euiOverlayMask-belowHeader world"` + `"euiOverlayMask css-1j0pa91-euiOverlayMask-belowHeader world"` ); }); diff --git a/src/components/page/page_sidebar/page_sidebar.test.tsx b/src/components/page/page_sidebar/page_sidebar.test.tsx index 849933a3f1d..cbbdbe86e4b 100644 --- a/src/components/page/page_sidebar/page_sidebar.test.tsx +++ b/src/components/page/page_sidebar/page_sidebar.test.tsx @@ -66,14 +66,15 @@ describe('EuiPageSidebar', () => { const component = mount( ); + const expectedStyles = { + insetBlockStart: 'var(--euiFixedHeadersOffset, 0)', + maxBlockSize: 'calc(100vh - var(--euiFixedHeadersOffset, 0))', + minInlineSize: 248, + }; expect( component.find('[data-test-subj="sidebar"]').last().prop('style') - ).toEqual({ - insetBlockStart: 0, - maxBlockSize: 'calc(100vh - 0px)', - minInlineSize: 248, - }); + ).toEqual(expectedStyles); component.setProps({ style: { color: 'red' } }); component.update(); @@ -82,9 +83,7 @@ describe('EuiPageSidebar', () => { component.find('[data-test-subj="sidebar"]').last().prop('style') ).toEqual({ color: 'red', - insetBlockStart: 0, - maxBlockSize: 'calc(100vh - 0px)', - minInlineSize: 248, + ...expectedStyles, }); }); }); diff --git a/src/components/page/page_sidebar/page_sidebar.tsx b/src/components/page/page_sidebar/page_sidebar.tsx index 8bcce41df84..f4233e51884 100644 --- a/src/components/page/page_sidebar/page_sidebar.tsx +++ b/src/components/page/page_sidebar/page_sidebar.tsx @@ -87,19 +87,15 @@ export const EuiPageSidebar: FunctionComponent = ({ }; if (sticky) { - const euiHeaderFixedCounter = Number( - document.body.dataset.fixedHeaders ?? 0 - ); - const offset = typeof sticky === 'object' - ? sticky?.offset - : themeContext.euiTheme.base * 3 * euiHeaderFixedCounter; + ? `${sticky?.offset}px` + : 'var(--euiFixedHeadersOffset, 0)'; updatedStyles = { ...updatedStyles, ...logicalStyle('top', offset), - ...logicalStyle('max-height', `calc(100vh - ${offset}px)`), + ...logicalStyle('max-height', `calc(100vh - ${offset})`), }; } diff --git a/src/components/page_template/__snapshots__/page_template.test.tsx.snap b/src/components/page_template/__snapshots__/page_template.test.tsx.snap index 028a8b2b3e0..95a1ba11907 100644 --- a/src/components/page_template/__snapshots__/page_template.test.tsx.snap +++ b/src/components/page_template/__snapshots__/page_template.test.tsx.snap @@ -3,7 +3,7 @@ exports[`EuiPageTemplate _EuiPageInnerProps is rendered 1`] = `
{ - if (typeof document === 'undefined') return 0; // SSR catch - - const euiHeaderFixedCounter = Number(document.body.dataset.fixedHeaders ?? 0); - return base * 3 * euiHeaderFixedCounter; -}; - /** * Consumed via `EuiPageTemplate`, * it controls and propogates most of the shared props per direct child @@ -98,7 +89,7 @@ export const _EuiPageTemplate: FunctionComponent = ({ paddingSize = 'l', grow = true, bottomBorder, - offset: _offset, + offset, panelled, // Inner props contentBorder, @@ -109,11 +100,6 @@ export const _EuiPageTemplate: FunctionComponent = ({ minHeight = '460px', ...rest }) => { - const { euiTheme } = useEuiTheme(); - - const [offset, setOffset] = useState( - () => _offset ?? calculateOffset(euiTheme.base) - ); const templateContext = useContext(TemplateContext); // Used as a target to insert the bottom bar component @@ -122,12 +108,6 @@ export const _EuiPageTemplate: FunctionComponent = ({ conditionalId: mainProps?.id, }); - useEffect(() => { - if (_offset === undefined) { - setOffset(calculateOffset(euiTheme.base)); - } - }, [_offset, euiTheme.base]); - // Sections include page header const sections: React.ReactElement[] = []; const sidebar: React.ReactElement[] = []; @@ -194,7 +174,7 @@ export const _EuiPageTemplate: FunctionComponent = ({ const classes = classNames('euiPageTemplate', className); const pageStyle = { ...logicalStyle('min-height', _minHeight), - ...logicalStyle('padding-top', offset), + ...logicalStyle('padding-top', offset ?? 'var(--euiFixedHeadersOffset, 0)'), ...rest.style, }; diff --git a/src/global_styling/mixins/_header.scss b/src/global_styling/mixins/_header.scss index 06109c5d6c5..aae8c7eb1fc 100644 --- a/src/global_styling/mixins/_header.scss +++ b/src/global_styling/mixins/_header.scss @@ -1,6 +1,8 @@ @import '../variables/header'; @mixin euiHeaderAffordForFixed($headerHeight: $euiHeaderHeightCompensation) { + @warn 'This mixin will shortly be deprecated. Use the CSS variable var(--euiFixedHeadersOffset) instead, which updates dynamically based on the number of fixed headers on the page.'; + // The `@at-root #{&}` allows for grouping alongside another specific body class. // When not applied inside of another selector, it simply renders with the single class @at-root #{&}.euiBody--headerIsFixed { diff --git a/src/global_styling/mixins/_helpers.ts b/src/global_styling/mixins/_helpers.ts index 72ffdb6f472..6c2b79804d3 100644 --- a/src/global_styling/mixins/_helpers.ts +++ b/src/global_styling/mixins/_helpers.ts @@ -126,14 +126,14 @@ const euiOverflowShadowStyles = ( * Others like Safari, won't show anything at all. */ interface _EuiYScroll { - height?: CSSProperties['height']; + height?: CSSProperties['height'] | false; } export const euiYScroll = ( euiTheme: UseEuiTheme, { height }: _EuiYScroll = {} ) => ` ${euiScrollBarStyles(euiTheme)} - ${logicalCSS('height', height || '100%')} + ${height !== false ? logicalCSS('height', height || '100%') : ''} ${logicalCSSWithFallback('overflow-y', 'auto')} ${logicalCSSWithFallback('overflow-x', 'hidden')} &:focus { diff --git a/upcoming_changelogs/7144.md b/upcoming_changelogs/7144.md new file mode 100644 index 00000000000..25604d3fb51 --- /dev/null +++ b/upcoming_changelogs/7144.md @@ -0,0 +1,8 @@ +- Fixed-positioned `EuiHeader`s now set a global CSS `--euiFixedHeadersOffset` variable, which updates dynamically based on the number of fixed headers on the page. +- `EuiFlyout`s now dynamically set their position, height, and mask based on the number of fixed headers on the page. +- Sticky-positioned `EuiPageSidebar`s now dynamically set their position and height based on the number of fixed headers on the page. This can still be overridden via the `sticky.offset` prop if needed. +- `EuiPageTemplate` now dynamically offsets content from any fixed headers on the page. This can still be overridden via the `offset` prop if needed. + +**Deprecations** + +- Deprecated the Sass `euiHeaderAffordForFixed` mixin. Use the new global CSS `var(--euiFixedHeadersOffset)` variable instead. From 2bd110f43d2a1f574bac9961976586ccca57156a Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Fri, 1 Sep 2023 09:25:40 -0700 Subject: [PATCH 3/5] [wiki] Add CSS variables docs to our Emotion wiki (#7145) Co-authored-by: Trevor Pierce <1Copenut@users.noreply.github.com> --- .../developing/writing-styles-with-emotion.md | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/wiki/contributing-to-eui/developing/writing-styles-with-emotion.md b/wiki/contributing-to-eui/developing/writing-styles-with-emotion.md index 5f6f58abac9..7faf221c044 100644 --- a/wiki/contributing-to-eui/developing/writing-styles-with-emotion.md +++ b/wiki/contributing-to-eui/developing/writing-styles-with-emotion.md @@ -518,6 +518,58 @@ When creating mixins & utilities for reuse within Emotion CSS, consider the foll - If you anticipate your mixin being used in the `style` prop instead of `css` (since React will want an object and camelCased CSS properties) - If you want your mixin to be partially composable, so if you think developers will want to obtain a single line/property from your mixin instead of the entire thing (e.g. `euiFontSize.lineHeight`) +## JS vs. CSS component variables + +In general, most component-specific style variables can remain JS-only (e.g., [euiStepVariables](https://github.com/elastic/eui/blob/068f0000532e6433383093d3488d7b1c4979c022/src/components/steps/step.styles.ts#L13-L19), [euiFormVariables](https://github.com/elastic/eui/blob/d39c0e988409f90f62af57174590044664b2bfce/src/components/form/form.styles.ts#L19)). These JS variable examples are generally used internally by EUI, and are not public top-level exports. + +There are some scenarios, however, where certain component style variables are important enough to be made globally available via a [CSS variable](https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties). + +An example of this is **EuiHeader**: Fixed header height(s) and the page offset they cause need to be accounted for by multiple other EUI components (e.g. **EuiFlyout**, **EuiPageTemplate**), and potentially by custom consumer layouts. Using a global CSS variable allows **EuiHeader** to dynamically track the number of fixed headers and calculate total height in a single place. Other components can reuse that CSS variable without extra JS logic needed ([#7144](https://github.com/elastic/eui/pull/7144)). + +EUI components can set CSS variables in two places: globally, or at the nearest **EuiThemeProvider** wrapper level: + +```tsx +import React, { useEffect } from 'react'; +import { useEuiTheme, useEuiThemeCSSVariables } from '../../services'; + +const EuiComponent = ({ ...props }) => { + const { euiTheme } = useEuiTheme(); + const { + setGlobalCSSVariables, + setNearestThemeCSSvariables, + } = useEuiThemeCSSVariables(); + + useEffect(() => { + // Sets the CSS variable at `:root` + setGlobalCSSVariables({ '--euiSomeGlobalVariable': euiTheme.color.success }); + + // Sets the CSS variable on the nearest parent theme provider wrapper + // If the nearest provider is EuiProvider, the variable is set globally on `:root` in any case + setNearestThemeCSSVariables({ '--euiSomeThemeVariable': euiTheme.size.m }); + }, []); + + return <>; +} +``` + +While a global CSS variable makes sense for **EuiHeader**, for most components, nearest theme variables would likely make more sense. For example, **EuiForm** should respect any custom theme modifications and pass its modified form variables to any children, but not siblings or parent forms that do not have modifications. + +```tsx +// Normal form + + {/* ... Form controls that inherit global form variables */} + + +// Form with a custom size scale + + + {/* ... Form controls that inherit from the nearest theme variables */} + + +``` + +[See our EuiThemeProvider stories](http://localhost:6006/?path=/story/euithemeprovider--css-variables-nearest) to view an example of this behavior in the browser. + ### Naming When naming your mixins & utilities, consider the following statements: From 4f08b9a1d68f6f4fd840db16ae48157746e2504c Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 5 Sep 2023 13:23:01 -0700 Subject: [PATCH 4/5] [PR feedback] Revert `euiYScroll` change --- .../collapsible_nav_beta/collapsible_nav_beta.styles.ts | 8 +++----- src/global_styling/mixins/_helpers.ts | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts b/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts index d49cae7c31d..d60e66fb5f1 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts +++ b/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts @@ -20,21 +20,19 @@ export const euiCollapsibleNavBetaStyles = (euiThemeContext: UseEuiTheme) => { // page load by setting the CSS var fallback to the height of a single header const defaultHeaderHeight = euiHeaderVariables(euiThemeContext).height; const fixedHeaderOffset = `var(--euiFixedHeadersOffset, ${defaultHeaderHeight})`; + const height = `calc(100% - ${fixedHeaderOffset})`; return { euiCollapsibleNavBeta: css` /* Fixed header affordance */ ${logicalCSS('top', fixedHeaderOffset)} - ${logicalCSS('height', `calc(100% - ${fixedHeaderOffset})`)} + /* Set the height & allow the nav to scroll, in case consumers don't use EuiFlyoutBody/EuiFyoutFooter */ + ${euiYScroll(euiThemeContext, { height })} /* This extra padding is needed for EuiPopovers to have enough space to render with the right anchorPosition */ ${logicalCSS('padding-bottom', euiTheme.size.xs)} - /* Allow the nav to scroll, in case consumers don't use EuiFlyoutBody/EuiFyoutFooter - Height is already set by header affordance above */ - ${euiYScroll(euiThemeContext, { height: false })} - /* In case things get really dire responsively, ensure the footer doesn't overtake the body */ .euiFlyoutBody { ${logicalCSS('min-height', '50%')} diff --git a/src/global_styling/mixins/_helpers.ts b/src/global_styling/mixins/_helpers.ts index 6c2b79804d3..72ffdb6f472 100644 --- a/src/global_styling/mixins/_helpers.ts +++ b/src/global_styling/mixins/_helpers.ts @@ -126,14 +126,14 @@ const euiOverflowShadowStyles = ( * Others like Safari, won't show anything at all. */ interface _EuiYScroll { - height?: CSSProperties['height'] | false; + height?: CSSProperties['height']; } export const euiYScroll = ( euiTheme: UseEuiTheme, { height }: _EuiYScroll = {} ) => ` ${euiScrollBarStyles(euiTheme)} - ${height !== false ? logicalCSS('height', height || '100%') : ''} + ${logicalCSS('height', height || '100%')} ${logicalCSSWithFallback('overflow-y', 'auto')} ${logicalCSSWithFallback('overflow-x', 'hidden')} &:focus { From d2e834c55b2bc5e954e732f9141ab8709e90c0df Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 5 Sep 2023 13:40:12 -0700 Subject: [PATCH 5/5] [PR feedback] height: inherit --- .../collapsible_nav_beta/collapsible_nav_beta.styles.ts | 6 +++--- src/components/flyout/flyout.styles.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts b/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts index d60e66fb5f1..5e93c634fa5 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts +++ b/src/components/collapsible_nav_beta/collapsible_nav_beta.styles.ts @@ -20,14 +20,14 @@ export const euiCollapsibleNavBetaStyles = (euiThemeContext: UseEuiTheme) => { // page load by setting the CSS var fallback to the height of a single header const defaultHeaderHeight = euiHeaderVariables(euiThemeContext).height; const fixedHeaderOffset = `var(--euiFixedHeadersOffset, ${defaultHeaderHeight})`; - const height = `calc(100% - ${fixedHeaderOffset})`; return { euiCollapsibleNavBeta: css` /* Fixed header affordance */ ${logicalCSS('top', fixedHeaderOffset)} - /* Set the height & allow the nav to scroll, in case consumers don't use EuiFlyoutBody/EuiFyoutFooter */ - ${euiYScroll(euiThemeContext, { height })} + + /* Allow the nav to scroll, in case consumers don't use EuiFlyoutBody/EuiFyoutFooter */ + ${euiYScroll(euiThemeContext, { height: 'inherit' })} /* This extra padding is needed for EuiPopovers to have enough space to render with the right anchorPosition */ diff --git a/src/components/flyout/flyout.styles.ts b/src/components/flyout/flyout.styles.ts index b325d3be216..d24faac1f0d 100644 --- a/src/components/flyout/flyout.styles.ts +++ b/src/components/flyout/flyout.styles.ts @@ -100,7 +100,7 @@ export const euiFlyoutStyles = (euiThemeContext: UseEuiTheme) => { position: fixed; ${logicalCSS('bottom', 0)} ${logicalCSS('top', 'var(--euiFixedHeadersOffset, 0)')} - ${logicalCSS('height', 'calc(100% - var(--euiFixedHeadersOffset, 0))')} + ${logicalCSS('height', 'inherit')} z-index: ${euiTheme.levels.flyout}; background: ${euiTheme.colors.emptyShade}; display: flex;