From 3c0e0ae3fac88d3fa054cb388f857ca0b2230c9c Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Mon, 1 Jul 2024 05:01:47 -0700 Subject: [PATCH] [EuiThemeProvider] Allow nested `modify.breakpoint` overrides (#7862) --- packages/eui/changelogs/upcoming/7862.md | 1 + .../theme/breakpoints/_breakpoints_js.tsx | 7 +- .../views/theme/breakpoints/breakpoints.tsx | 6 +- .../eui/src/components/provider/provider.tsx | 5 +- .../breakpoint/current_breakpoint.tsx | 8 ++- .../eui/src/services/theme/provider.test.tsx | 72 +++++++++++++++++++ packages/eui/src/services/theme/provider.tsx | 15 +++- 7 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 packages/eui/changelogs/upcoming/7862.md diff --git a/packages/eui/changelogs/upcoming/7862.md b/packages/eui/changelogs/upcoming/7862.md new file mode 100644 index 00000000000..9dfbb90ddd7 --- /dev/null +++ b/packages/eui/changelogs/upcoming/7862.md @@ -0,0 +1 @@ +- Updated `EuiThemeProvider`s to allow modifying/setting custom `breakpoint`s in nested usage (as opposed to only at the top `EuiProvider` level) diff --git a/packages/eui/src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx b/packages/eui/src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx index 870676528a9..23c749f41b8 100644 --- a/packages/eui/src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx +++ b/packages/eui/src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx @@ -217,7 +217,8 @@ export const CustomBreakpointsJS = () => { <>

Theme breakpoints can be overriden or added via{' '} - EuiProvider's modify{' '} + EuiProvider's or{' '} + EuiThemeProvider's modify{' '} prop.

@@ -232,7 +233,7 @@ export const CustomBreakpointsJS = () => { Current custom breakpoint: {currentBreakpoint}

} - snippet={` { }} > - + `} snippetLanguage="js" /> diff --git a/packages/eui/src-docs/src/views/theme/breakpoints/breakpoints.tsx b/packages/eui/src-docs/src/views/theme/breakpoints/breakpoints.tsx index 6183b3d3a8e..d383b2b4b41 100644 --- a/packages/eui/src-docs/src/views/theme/breakpoints/breakpoints.tsx +++ b/packages/eui/src-docs/src/views/theme/breakpoints/breakpoints.tsx @@ -1,5 +1,5 @@ import React, { useContext, useMemo } from 'react'; -import { EuiSpacer, EuiText, EuiProvider } from '../../../../../src'; +import { EuiSpacer, EuiText, EuiThemeProvider } from '../../../../../src'; import { GuideSection } from '../../../components/guide_section/guide_section'; import { ThemeContext } from '../../../components/with_theme'; @@ -51,7 +51,7 @@ export default () => { {valuesContent} {currentLanguage.includes('js') && ( - +

@@ -70,7 +70,7 @@ export default () => { {valuesContent} - + )} ); diff --git a/packages/eui/src/components/provider/provider.tsx b/packages/eui/src/components/provider/provider.tsx index 69403787511..2275009bd7c 100644 --- a/packages/eui/src/components/provider/provider.tsx +++ b/packages/eui/src/components/provider/provider.tsx @@ -13,7 +13,6 @@ import { EuiThemeProvider, EuiThemeProviderProps, EuiThemeSystem, - CurrentEuiBreakpointProvider, } from '../../services'; import { emitEuiProviderWarning } from '../../services/theme/warning'; import { cache as fallbackCache } from '../../services/emotion/css'; @@ -146,9 +145,7 @@ export const EuiProvider = ({ )} - - {children} - + {children} diff --git a/packages/eui/src/services/breakpoint/current_breakpoint.tsx b/packages/eui/src/services/breakpoint/current_breakpoint.tsx index 71c04ee4e05..f41a704e7c9 100644 --- a/packages/eui/src/services/breakpoint/current_breakpoint.tsx +++ b/packages/eui/src/services/breakpoint/current_breakpoint.tsx @@ -21,7 +21,7 @@ import { _EuiThemeBreakpoint, _EuiThemeBreakpoints, } from '../../global_styling/variables/breakpoint'; -import { useEuiTheme } from '../theme'; +import { useEuiTheme } from '../theme/hooks'; import { throttle } from '../throttle'; import { sortMapByLargeToSmallValues } from './_sorting'; @@ -31,8 +31,10 @@ export const CurrentEuiBreakpointContext = createContext(undefined); /** - * Top level provider (nested within EuiProvider) which provides a single - * resize listener that returns the current breakpoint based on window width + * Returns the current breakpoint based on window width. + * Typically only called by the top-level `EuiProvider` (to reduce the number + * of window resize listeners on the page). Also conditionally called if a + * nested `EuiThemeProvider` defines a `modify.breakpoint` override */ export const CurrentEuiBreakpointProvider: FunctionComponent< PropsWithChildren diff --git a/packages/eui/src/services/theme/provider.test.tsx b/packages/eui/src/services/theme/provider.test.tsx index 3b0368723c5..539016bca0a 100644 --- a/packages/eui/src/services/theme/provider.test.tsx +++ b/packages/eui/src/services/theme/provider.test.tsx @@ -11,6 +11,7 @@ import { render } from '@testing-library/react'; // Note - don't use the EUI cus import { css } from '@emotion/react'; import { EuiProvider } from '../../components/provider'; +import { useCurrentEuiBreakpoint } from '../breakpoint'; import { EuiNestedThemeContext } from './context'; import { EuiThemeProvider } from './provider'; @@ -75,6 +76,77 @@ describe('EuiThemeProvider', () => { }); }); + describe('conditional CurrentEuiBreakpointProvider', () => { + const PrintCurrentBreakpoint = () => <>{useCurrentEuiBreakpoint()}; + let resizeListenerCount = 0; + + beforeAll(() => { + // React 16 and 17 register a bunch of error listeners which we don't need to capture + window.addEventListener = jest.fn((event: string) => { + if (event === 'resize') resizeListenerCount++; + }); + }); + + beforeEach(() => { + // Reset counts + resizeListenerCount = 0; + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + describe('in the top-level global theme provider', () => { + it('is always rendered regardless of modified breakpoints', () => { + const { container } = render( + + + + ); + expect(container.textContent).toEqual('l'); + expect(resizeListenerCount).toEqual(1); + }); + }); + + describe('in nested child theme providers', () => { + beforeAll(() => { + window.innerWidth = 2500; + }); + + afterAll(() => { + // Reset window width to jsdom's default + window.innerWidth = 1024; + }); + + it('is rendered if modify.breakpoint is passed', () => { + const customBreakpoints = { xxl: 2000 }; + + const { container } = render( + + + + + + ); + + expect(container.textContent).toEqual('xxl'); + expect(resizeListenerCount).toBeGreaterThanOrEqual(2); // Technically 3 due to EuiThemeProvider rerendering + }); + + it('is not rendered if modify.breakpoint is not passed', () => { + const { container } = render( + + + + + + ); + expect(container.textContent).toEqual('xl'); + expect(resizeListenerCount).toEqual(1); + }); + }); + }); + describe('nested EuiThemeProviders', () => { it('renders with a span wrapper that sets the inherited text color', () => { const { container } = render( diff --git a/packages/eui/src/services/theme/provider.tsx b/packages/eui/src/services/theme/provider.tsx index aa6a8bf2c26..c5a6957e8c3 100644 --- a/packages/eui/src/services/theme/provider.tsx +++ b/packages/eui/src/services/theme/provider.tsx @@ -15,6 +15,7 @@ import React, { useCallback, PropsWithChildren, HTMLAttributes, + Fragment, } from 'react'; import { Global, type CSSObject } from '@emotion/react'; import isEqual from 'lodash/isEqual'; @@ -22,6 +23,7 @@ import isEqual from 'lodash/isEqual'; import type { CommonProps } from '../../components/common'; import { cloneElementWithCss } from '../emotion'; import { css, cx } from '../emotion/css'; +import { CurrentEuiBreakpointProvider } from '../breakpoint/current_breakpoint'; import { EuiSystemContext, @@ -80,6 +82,15 @@ export const EuiThemeProvider = ({ const [system, setSystem] = useState(_system || parentSystem); const prevSystemKey = useRef(system.key); + // To reduce the number of window resize listeners, only render a + // CurrentEuiBreakpointProvider for the top level parent theme, or for + // nested themes only if modified breakpoint overrides are passed + const EuiConditionalBreakpointProvider = useMemo(() => { + return isGlobalTheme || _modifications?.breakpoint + ? CurrentEuiBreakpointProvider + : Fragment; + }, [isGlobalTheme, _modifications]); + const [modifications, setModifications] = useState( mergeDeep(parentModifications, _modifications) ); @@ -232,7 +243,9 @@ export const EuiThemeProvider = ({ - {renderedChildren} + + {renderedChildren} +