From 70638083a44e66527168065f25a069f039bce271 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 09:38:35 -0800 Subject: [PATCH 1/9] [EuiText][EuiTextColor] Memoize styles --- src/components/text/text.tsx | 5 ++--- src/components/text/text_color.tsx | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/components/text/text.tsx b/src/components/text/text.tsx index 3d7124b60b1..e264e6c9ca3 100644 --- a/src/components/text/text.tsx +++ b/src/components/text/text.tsx @@ -10,7 +10,7 @@ import React, { FunctionComponent, HTMLAttributes, CSSProperties } from 'react'; import classNames from 'classnames'; import { CommonProps } from '../common'; -import { useEuiTheme } from '../../services'; +import { useEuiMemoizedStyles } from '../../services'; import { euiTextStyles } from './text.styles'; import { TextColor, EuiTextColor } from './text_color'; @@ -44,8 +44,7 @@ export const EuiText: FunctionComponent = ({ className, ...rest }) => { - const euiTheme = useEuiTheme(); - const styles = euiTextStyles(euiTheme); + const styles = useEuiMemoizedStyles(euiTextStyles); const cssStyles = [ styles.euiText, !grow ? styles.constrainedWidth : undefined, diff --git a/src/components/text/text_color.tsx b/src/components/text/text_color.tsx index dff5f4622b6..88245417381 100644 --- a/src/components/text/text_color.tsx +++ b/src/components/text/text_color.tsx @@ -14,7 +14,7 @@ import React, { } from 'react'; import { CommonProps } from '../common'; -import { useEuiTheme, cloneElementWithCss } from '../../services'; +import { useEuiMemoizedStyles, cloneElementWithCss } from '../../services'; import { euiTextColorStyles } from './text_color.styles'; @@ -60,8 +60,7 @@ export const EuiTextColor: FunctionComponent = ({ }) => { const isNamedColor = COLORS.includes(color as TextColor); - const euiTheme = useEuiTheme(); - const styles = euiTextColorStyles(euiTheme); + const styles = useEuiMemoizedStyles(euiTextColorStyles); const cssStyles = [ styles.euiTextColor, isNamedColor ? styles[color as TextColor] : styles.customColor, From bb3e7d96e7ed6deef84c7fbbd3b7a48861a88068 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 09:38:51 -0800 Subject: [PATCH 2/9] [EuiTextAlign] Remove unnecessary styles function --- src/components/text/text_align.styles.ts | 4 ++-- src/components/text/text_align.tsx | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/text/text_align.styles.ts b/src/components/text/text_align.styles.ts index c6a7fa403b3..151b66c4832 100644 --- a/src/components/text/text_align.styles.ts +++ b/src/components/text/text_align.styles.ts @@ -9,7 +9,7 @@ import { css } from '@emotion/react'; import { logicalTextAlignCSS } from '../../global_styling'; -export const euiTextAlignStyles = () => ({ +export const euiTextAlignStyles = { euiTextAlign: css``, left: css` ${logicalTextAlignCSS('left')} @@ -20,4 +20,4 @@ export const euiTextAlignStyles = () => ({ center: css` ${logicalTextAlignCSS('center')} `, -}); +}; diff --git a/src/components/text/text_align.tsx b/src/components/text/text_align.tsx index 0b42a77556c..5b3c44736c4 100644 --- a/src/components/text/text_align.tsx +++ b/src/components/text/text_align.tsx @@ -14,7 +14,7 @@ import React, { import { CommonProps } from '../common'; import { cloneElementWithCss } from '../../services'; -import { euiTextAlignStyles } from './text_align.styles'; +import { euiTextAlignStyles as styles } from './text_align.styles'; export const ALIGNMENTS = ['left', 'right', 'center'] as const; export type TextAlignment = (typeof ALIGNMENTS)[number]; @@ -35,7 +35,6 @@ export const EuiTextAlign: FunctionComponent = ({ cloneElement = false, ...rest }) => { - const styles = euiTextAlignStyles(); const cssStyles = [styles.euiTextAlign, styles[textAlign]]; const props = { css: cssStyles, ...rest }; From 2da03245d752f4443478c80d2ae5ac442630a439 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 09:39:45 -0800 Subject: [PATCH 3/9] [EuiTitle] Memoize styles --- src/components/title/title.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/title/title.tsx b/src/components/title/title.tsx index 0a0a819a45d..b8d5243997a 100644 --- a/src/components/title/title.tsx +++ b/src/components/title/title.tsx @@ -8,10 +8,12 @@ import { FunctionComponent, ReactElement } from 'react'; import classNames from 'classnames'; -import { useEuiTheme, cloneElementWithCss } from '../../services'; -import { euiTitleStyles } from './title.styles'; + +import { useEuiMemoizedStyles, cloneElementWithCss } from '../../services'; import { CommonProps } from '../common'; +import { euiTitleStyles } from './title.styles'; + export const TITLE_SIZES = ['xxxs', 'xxs', 'xs', 's', 'm', 'l'] as const; export type EuiTitleSize = (typeof TITLE_SIZES)[number]; @@ -35,8 +37,7 @@ export const EuiTitle: FunctionComponent = ({ textTransform, ...rest }) => { - const euiTheme = useEuiTheme(); - const styles = euiTitleStyles(euiTheme); + const styles = useEuiMemoizedStyles(euiTitleStyles); const cssStyles = [ styles.euiTitle, textTransform ? styles[textTransform] : undefined, From 4348b1dad91fc321db56fdbdcdf6b2cdd9eac43a Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 09:40:45 -0800 Subject: [PATCH 4/9] [EuiTitle] Remove unused `useEuiTitle` hook/export - no one's using it - it isn't documented anywhere - we should likely try to remove unnecessary style exports where we can, and encourage using components directly instead --- src/components/title/title.styles.test.ts | 6 ++++-- src/components/title/title.styles.ts | 11 +---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/components/title/title.styles.test.ts b/src/components/title/title.styles.test.ts index d270a0ed775..ccef482e419 100644 --- a/src/components/title/title.styles.test.ts +++ b/src/components/title/title.styles.test.ts @@ -8,15 +8,17 @@ import { renderHook } from '@testing-library/react'; +import { useEuiTheme } from '../../services'; + import { TITLE_SIZES } from './title'; -import { useEuiTitle } from './title.styles'; +import { euiTitle } from './title.styles'; describe('euiTitle mixin', () => { describe('returns a static object of title font properties for each title size', () => { TITLE_SIZES.forEach((size) => { it(size, () => { expect( - renderHook(() => useEuiTitle(size)).result.current + renderHook(() => euiTitle(useEuiTheme(), size)).result.current ).toMatchSnapshot(); }); }); diff --git a/src/components/title/title.styles.ts b/src/components/title/title.styles.ts index da206ef83a3..e4c7567efa6 100644 --- a/src/components/title/title.styles.ts +++ b/src/components/title/title.styles.ts @@ -8,7 +8,7 @@ import { CSSProperties } from 'react'; import { css } from '@emotion/react'; -import { UseEuiTheme, useEuiTheme } from '../../services'; +import { UseEuiTheme } from '../../services'; import { euiTextBreakWord, euiFontSize, @@ -56,15 +56,6 @@ export const euiTitle = ( }; }; -// Hook version -export const useEuiTitle = ( - scale: EuiTitleSize, - options?: _FontScaleOptions -): EuiThemeTitle => { - const euiTheme = useEuiTheme(); - return euiTitle(euiTheme, scale, options); -}; - /** * Styles */ From 21e68b0bd769f26e895a0cab4637a4728bbd74bd Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 09:43:38 -0800 Subject: [PATCH 5/9] [EuiLink] Memoize styles --- src/components/link/external_link_icon.tsx | 9 ++++++--- src/components/link/link.tsx | 5 ++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/components/link/external_link_icon.tsx b/src/components/link/external_link_icon.tsx index f74093c46c2..e3cb3d1c230 100644 --- a/src/components/link/external_link_icon.tsx +++ b/src/components/link/external_link_icon.tsx @@ -8,7 +8,7 @@ import React, { FunctionComponent, AnchorHTMLAttributes } from 'react'; -import { useEuiTheme } from '../../services'; +import { useEuiMemoizedStyles, UseEuiTheme } from '../../services'; import { logicalStyle } from '../../global_styling'; import { EuiIcon, EuiIconProps } from '../icon'; import { EuiI18n, useEuiI18n } from '../i18n'; @@ -28,10 +28,13 @@ export type EuiExternalLinkIconProps = { external?: boolean; }; +const iconStyle = ({ euiTheme }: UseEuiTheme) => + logicalStyle('margin-left', euiTheme.size.xs); + export const EuiExternalLinkIcon: FunctionComponent< EuiExternalLinkIconProps & Partial > = ({ target, external, ...rest }) => { - const { euiTheme } = useEuiTheme(); + const iconCssStyle = useEuiMemoizedStyles(iconStyle); const showExternalLinkIcon = (target === '_blank' && external !== false) || external === true; @@ -45,7 +48,7 @@ export const EuiExternalLinkIcon: FunctionComponent< <> {showExternalLinkIcon && ( ( }, ref ) => { - const euiTheme = useEuiTheme(); - const styles = euiLinkStyles(euiTheme); + const styles = useEuiMemoizedStyles(euiLinkStyles); const cssStyles = [styles.euiLink]; const isHrefValid = !href || validateHref(href); From 8753ca18f1b2b6db3e21304e951529eea244a71a Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 09:44:36 -0800 Subject: [PATCH 6/9] [EuiLink] Remove unnecessary style exports - not being used anywhere in EUI or KIbana, just inline them + move internal `_colorCSS` util to bottom of file --- src/components/link/link.styles.ts | 38 ++++++++++-------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/src/components/link/link.styles.ts b/src/components/link/link.styles.ts index 7dd64046b26..96530e33c8f 100644 --- a/src/components/link/link.styles.ts +++ b/src/components/link/link.styles.ts @@ -10,29 +10,6 @@ import { css } from '@emotion/react'; import { UseEuiTheme } from '../../services'; import { euiFocusRing, logicalTextAlignCSS } from '../../global_styling'; -const _colorCSS = (color: string) => { - return ` - color: ${color}; - - &:target { - color: darken(${color}, 10%); - } - `; -}; - -export const euiLinkHoverCSS = () => { - return ` - text-decoration: underline; - `; -}; - -export const euiLinkFocusCSS = (euiTheme: UseEuiTheme['euiTheme']) => { - return ` - text-decoration: underline; - text-decoration-thickness: ${euiTheme.border.width.thick}; - `; -}; - export const euiLinkCSS = (euiThemeContext: UseEuiTheme) => { const { euiTheme } = euiThemeContext; return ` @@ -40,12 +17,13 @@ export const euiLinkCSS = (euiThemeContext: UseEuiTheme) => { ${logicalTextAlignCSS('left')} &:hover { - ${euiLinkHoverCSS()} + text-decoration: underline; } &:focus { ${euiFocusRing(euiThemeContext, 'outset')} - ${euiLinkFocusCSS(euiTheme)} + text-decoration: underline; + text-decoration-thickness: ${euiTheme.border.width.thick}; } `; }; @@ -85,3 +63,13 @@ export const euiLinkStyles = (euiThemeContext: UseEuiTheme) => { text: css(_colorCSS(euiTheme.colors.text)), }; }; + +const _colorCSS = (color: string) => { + return ` + color: ${color}; + + &:target { + color: darken(${color}, 10%); + } + `; +}; From 33e844885940614471e25d2ae082060c26a29ed1 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 10:01:28 -0800 Subject: [PATCH 7/9] [EuiFacetButton] Remove `euiLinkFocusCSS` usage - just write out the style manually :shrug: honestly, it might need a redesign at some point anyway + clean up styles and how they're written --- src/components/facet/facet_button.styles.ts | 27 +++++++++------------ 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/components/facet/facet_button.styles.ts b/src/components/facet/facet_button.styles.ts index d057a31fd64..b35e67dd78c 100644 --- a/src/components/facet/facet_button.styles.ts +++ b/src/components/facet/facet_button.styles.ts @@ -14,33 +14,30 @@ import { logicalTextAlignCSS, } from '../../global_styling'; -import { euiLinkFocusCSS } from '../link/link.styles'; - export const euiFacetButtonStyles = ({ euiTheme }: UseEuiTheme) => ({ // Base euiFacetButton: css` ${logicalTextAlignCSS('left')} - &:hover, - &:focus { - /* Make sure the quantity doesn't get an underline on hover */ - &:not(:disabled) [class*='euiFacetButton__text'] { - text-decoration: underline; + &:not(:disabled) { + &:hover, + &:focus { + /* Make sure the quantity doesn't get an underline on hover */ + text-decoration: none; + + .euiFacetButton__text { + text-decoration: underline; + } } - } - &:focus:not(:disabled) [class*='euiFacetButton__text'] { - ${euiLinkFocusCSS(euiTheme)} + &:focus .euiFacetButton__text { + text-decoration-thickness: ${euiTheme.border.width.thick}; + } } &:disabled { color: ${euiTheme.colors.disabledText}; pointer-events: none; - - &:hover, - &:focus { - text-decoration: none; - } } `, }); From 04da2e68fd5263dd6795e4ba6a6dba5046fd0926 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 23 Feb 2024 09:44:52 -0800 Subject: [PATCH 8/9] [mixins] Memoize typography hooks --- src/global_styling/mixins/_typography.ts | 34 +++++++++++++++++++----- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/global_styling/mixins/_typography.ts b/src/global_styling/mixins/_typography.ts index 668f3593ff4..e7268f607e2 100644 --- a/src/global_styling/mixins/_typography.ts +++ b/src/global_styling/mixins/_typography.ts @@ -12,8 +12,15 @@ import { euiFontSizeFromScale, _FontScaleOptions, } from '../functions/typography'; -import { useEuiTheme, UseEuiTheme } from '../../services/theme/hooks'; -import { _EuiThemeFontScale } from '../variables/typography'; +import { + useEuiMemoizedStyles, + useEuiTheme, + UseEuiTheme, +} from '../../services/theme'; +import { + _EuiThemeFontScale, + EuiThemeFontScales, +} from '../variables/typography'; import { logicalCSS } from '../functions'; export type EuiThemeFontSize = { @@ -39,7 +46,22 @@ export const useEuiFontSize = ( options?: _FontScaleOptions ): EuiThemeFontSize => { const euiTheme = useEuiTheme(); - return euiFontSize(euiTheme, scale, options); + const memoizedFontSizes = useEuiMemoizedStyles(euiFontSizes); + + return !options + ? memoizedFontSizes[scale] + : euiFontSize(euiTheme, scale, options); +}; +// Memomize a basic set of font sizes. We unfortunately can't +// memoize all possible options, there's too many permutations +const euiFontSizes = (euiThemeContext: UseEuiTheme) => { + return EuiThemeFontScales.reduce( + (map, scale) => ({ + ...map, + [scale]: euiFontSize(euiThemeContext, scale), + }), + {} as Record<_EuiThemeFontScale, EuiThemeFontSize> + ); }; /** @@ -71,7 +93,5 @@ export const euiTextTruncate = ( export const euiNumberFormat = ({ euiTheme }: UseEuiTheme) => ` font-feature-settings: ${euiTheme.font.featureSettings}, 'tnum' 1; `; -export const useEuiNumberFormat = (): string => { - const euiTheme = useEuiTheme(); - return euiNumberFormat(euiTheme); -}; +export const useEuiNumberFormat = (): string => + useEuiMemoizedStyles(euiNumberFormat); From f9242a81fa12923fc68f57319c2c1cd8e5b70595 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 6 Mar 2024 10:00:30 -0800 Subject: [PATCH 9/9] [PR discussion] Remove `useEuiNumberFormat` --- .../src/views/theme/typography/_text_numbers.tsx | 14 ++++++++------ src/global_styling/mixins/_typography.test.ts | 5 +++-- src/global_styling/mixins/_typography.ts | 2 -- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src-docs/src/views/theme/typography/_text_numbers.tsx b/src-docs/src/views/theme/typography/_text_numbers.tsx index 0714212b8b3..a5f85d910fa 100644 --- a/src-docs/src/views/theme/typography/_text_numbers.tsx +++ b/src-docs/src/views/theme/typography/_text_numbers.tsx @@ -7,12 +7,14 @@ import { EuiTextAlign, EuiFlexGrid, EuiFlexItem, - useEuiNumberFormat, + useEuiTheme, + euiNumberFormat, } from '../../../../../src'; import { css } from '@emotion/react'; import { ThemeExample } from '../_components/_theme_example'; export default () => { + const euiTheme = useEuiTheme(); const themeContext = useContext(ThemeContext); const currentLanguage = themeContext.themeLanguage; const showSass = currentLanguage.includes('sass'); @@ -22,8 +24,8 @@ export default () => { {/* Mixin */} {!showSass ? ( useEuiNumberFormat()} - type="hook" + title={euiNumberFormat(euiTheme)} + type="function" description={

Applies{' '} @@ -49,7 +51,7 @@ export default () => {

With function @@ -62,7 +64,7 @@ export default () => { } - snippet={'${useEuiNumberFormat()}'} + snippet={'${euiNumberFormat(useEuiTheme())}'} snippetLanguage="emotion" /> ) : ( @@ -84,7 +86,7 @@ export default () => { type="className" description={

- Applies the useEuiNumberFormat() styles as an + Applies the euiNumberFormat() styles as an overriding CSS utility class.

} diff --git a/src/global_styling/mixins/_typography.test.ts b/src/global_styling/mixins/_typography.test.ts index a76dcb43a89..90310db61d4 100644 --- a/src/global_styling/mixins/_typography.test.ts +++ b/src/global_styling/mixins/_typography.test.ts @@ -8,12 +8,13 @@ import { renderHook } from '@testing-library/react'; +import { useEuiTheme } from '../../services'; import { EuiThemeFontScales, EuiThemeFontUnits } from '../variables/typography'; import { useEuiFontSize, euiTextBreakWord, euiTextTruncate, - useEuiNumberFormat, + euiNumberFormat, } from './_typography'; describe('euiFontSize', () => { @@ -66,7 +67,7 @@ describe('euiTextTruncate', () => { describe('euiNumberFormat', () => { it('returns a string of CSS text', () => { expect( - renderHook(() => useEuiNumberFormat()).result.current + renderHook(() => euiNumberFormat(useEuiTheme())).result.current ).toMatchSnapshot(); }); }); diff --git a/src/global_styling/mixins/_typography.ts b/src/global_styling/mixins/_typography.ts index e7268f607e2..3b031c2d3c8 100644 --- a/src/global_styling/mixins/_typography.ts +++ b/src/global_styling/mixins/_typography.ts @@ -93,5 +93,3 @@ export const euiTextTruncate = ( export const euiNumberFormat = ({ euiTheme }: UseEuiTheme) => ` font-feature-settings: ${euiTheme.font.featureSettings}, 'tnum' 1; `; -export const useEuiNumberFormat = (): string => - useEuiMemoizedStyles(euiNumberFormat);