Skip to content
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

[Emotion][perf] Memoize low-impact components (A-C) #7635

Merged
merged 11 commits into from
Apr 2, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ exports[`EuiScreenReaderOnly adds an accessibility class to a child element when

exports[`EuiScreenReaderOnly will show on focus 1`] = `
<a
class="emotion-euiScreenReaderOnly"
class="emotion-euiScreenReaderOnly-showOnFocus"
href="#"
>
Link
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ export const euiScreenReaderOnly = () => `
/*
* Styles
*/
export const euiScreenReaderOnlyStyles = (showOnFocus?: boolean) => ({
euiScreenReaderOnly: showOnFocus
? css`
/* The :active selector is necessary for Safari which removes :focus when a button is pressed */
&:not(:focus, :active, :focus-within) {
${euiScreenReaderOnly()}
}
`
: css(euiScreenReaderOnly()),
});
export const euiScreenReaderOnlyStyles = {
euiScreenReaderOnly: css(euiScreenReaderOnly()),

'euiScreenReaderOnly-showOnFocus': css`
/* The :active selector is necessary for Safari which removes :focus when a button is pressed */
&:not(:focus, :active, :focus-within) {
${euiScreenReaderOnly()}
}
`,
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
* Side Public License, v 1.
*/

import { ReactElement, FunctionComponent } from 'react';
import { ReactElement, FunctionComponent, useMemo } from 'react';
import classNames from 'classnames';

import { cloneElementWithCss } from '../../../services';
import { euiScreenReaderOnlyStyles } from './screen_reader_only.styles';
import { euiScreenReaderOnlyStyles as styles } from './screen_reader_only.styles';

export interface EuiScreenReaderOnlyProps {
/**
Expand All @@ -30,13 +30,15 @@ export const EuiScreenReaderOnly: FunctionComponent<
> = ({ children, className, showOnFocus }) => {
const classes = classNames(className, children.props.className);

const styles = euiScreenReaderOnlyStyles(showOnFocus);
const cssStyles = [styles.euiScreenReaderOnly];

const props = {
className: classes.length ? classes : undefined,
css: cssStyles,
};
const props = useMemo(
() => ({
className: classes.length ? classes : undefined,
css: showOnFocus
? styles['euiScreenReaderOnly-showOnFocus']
: styles.euiScreenReaderOnly,
}),
[classes, showOnFocus]
);

return cloneElementWithCss(children, props);
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`EuiSkipLink is rendered 1`] = `
<a
aria-label="aria-label"
class="euiButton euiSkipLink testClass1 testClass2 emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-euiScreenReaderOnly-euiTestCss"
class="euiButton euiSkipLink testClass1 testClass2 emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-euiScreenReaderOnly-showOnFocus-euiTestCss"
data-test-subj="test subject string"
href="#somewhere"
rel="noreferrer"
Expand All @@ -22,7 +22,7 @@ exports[`EuiSkipLink is rendered 1`] = `

exports[`EuiSkipLink props position absolute is rendered 1`] = `
<a
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-absolute-euiScreenReaderOnly"
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-absolute-euiScreenReaderOnly-showOnFocus"
href="#somewhere"
rel="noreferrer"
>
Expand All @@ -34,7 +34,7 @@ exports[`EuiSkipLink props position absolute is rendered 1`] = `

exports[`EuiSkipLink props position fixed is rendered 1`] = `
<a
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-fixed-euiScreenReaderOnly"
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-fixed-euiScreenReaderOnly-showOnFocus"
href="#somewhere"
rel="noreferrer"
tabindex="0"
Expand All @@ -47,7 +47,7 @@ exports[`EuiSkipLink props position fixed is rendered 1`] = `

exports[`EuiSkipLink props position static is rendered 1`] = `
<a
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-euiScreenReaderOnly"
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-euiScreenReaderOnly-showOnFocus"
href="#somewhere"
rel="noreferrer"
>
Expand All @@ -59,7 +59,7 @@ exports[`EuiSkipLink props position static is rendered 1`] = `

exports[`EuiSkipLink props tabIndex is rendered 1`] = `
<a
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-euiScreenReaderOnly"
class="euiButton euiSkipLink emotion-euiButtonDisplay-s-defaultMinWidth-fill-primary-euiSkipLink-euiScreenReaderOnly-showOnFocus"
href="#somewhere"
rel="noreferrer"
tabindex="-1"
Expand Down
6 changes: 2 additions & 4 deletions src/components/accessibility/skip_link/skip_link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import React, { FunctionComponent, Ref, useCallback } from 'react';
import classNames from 'classnames';
import { isTabbable } from 'tabbable';
import { useEuiTheme } from '../../../services';
import { useEuiMemoizedStyles } from '../../../services';
import { EuiButton, EuiButtonProps } from '../../button/button';
import { PropsForAnchor } from '../../common';
import { EuiScreenReaderOnly } from '../screen_reader_only';
Expand Down Expand Up @@ -69,11 +69,9 @@ export const EuiSkipLink: FunctionComponent<EuiSkipLinkProps> = ({
onClick: _onClick,
...rest
}) => {
const euiTheme = useEuiTheme();
const styles = euiSkipLinkStyles(euiTheme);

const classes = classNames('euiSkipLink', className);

const styles = useEuiMemoizedStyles(euiSkipLinkStyles);
const cssStyles = [
styles.euiSkipLink,
position !== 'static' ? styles[position] : undefined,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,46 +1,15 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiAspectRatio allows overriding with custom styles 1`] = `
<iframe
allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture"
allowfullscreen=""
class="euiAspectRatio"
frameborder="0"
height="315"
src="https://www.youtube.com/embed/yJarWSLRM24"
style="height: 300px; width: 100%; margin: 2em;"
title="Elastic is a search company"
width="560"
/>
`;

exports[`EuiAspectRatio is rendered 1`] = `
<iframe
allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture"
allowfullscreen=""
aria-label="aria-label"
class="euiAspectRatio testClass1 testClass2 emotion-euiTestCss"
data-test-subj="test subject string"
frameborder="0"
height="315"
src="https://www.youtube.com/embed/yJarWSLRM24"
style="height: auto; width: 100%;"
title="Elastic is a search company"
width="560"
/>
`;

exports[`EuiAspectRatio props maxWidth is rendered 1`] = `
exports[`EuiAspectRatio renders 1`] = `
<iframe
allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture"
allowfullscreen=""
aria-label="aria-label"
class="euiAspectRatio testClass1 testClass2 emotion-euiTestCss"
data-test-subj="test subject string"
frameborder="0"
height="315"
src="https://www.youtube.com/embed/yJarWSLRM24"
style="height: auto; width: 100%; max-width: 500px;"
style="block-size: auto; inline-size: 100%;"
title="Elastic is a search company"
width="560"
/>
Expand Down
64 changes: 13 additions & 51 deletions src/components/aspect_ratio/aspect_ratio.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,83 +13,45 @@ import { requiredProps } from '../../test/required_props';
import { EuiAspectRatio } from './aspect_ratio';

describe('EuiAspectRatio', () => {
test('is rendered', () => {
it('renders', () => {
const { container } = render(
<EuiAspectRatio height={4} width={9} {...requiredProps}>
<EuiAspectRatio height={4} width={9}>
<iframe
title="Elastic is a search company"
width="560"
height="315"
src="https://www.youtube.com/embed/yJarWSLRM24"
frameBorder="0"
allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture"
allowFullScreen
{...requiredProps}
/>
</EuiAspectRatio>
);

expect(container.firstChild).toMatchSnapshot();
});

test('allows overriding with custom styles', () => {
const { container } = render(
<EuiAspectRatio
height={4}
width={9}
style={{ margin: '2em', height: '300px' }}
>
<iframe
title="Elastic is a search company"
width="560"
height="315"
src="https://www.youtube.com/embed/yJarWSLRM24"
frameBorder="0"
allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture"
allowFullScreen
/>
</EuiAspectRatio>
);

expect(container.firstChild).toMatchSnapshot();
});

test('inherits child styles', () => {
it('merges child styles', () => {
const { getByTestSubject } = render(
<EuiAspectRatio height={4} width={9} style={{ color: 'gray' }}>
<EuiAspectRatio height={4} width={9}>
<div data-test-subj="child" style={{ backgroundColor: 'salmon' }} />
</EuiAspectRatio>
);

expect(getByTestSubject('child')).toHaveStyle({
'background-color': 'salmon',
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
color: 'gray',
});
});

describe('props', () => {
describe('maxWidth', () => {
test('is rendered', () => {
const { container } = render(
<EuiAspectRatio
height={16}
width={9}
maxWidth={500}
{...requiredProps}
>
<iframe
title="Elastic is a search company"
width="560"
height="315"
src="https://www.youtube.com/embed/yJarWSLRM24"
frameBorder="0"
allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture"
allowFullScreen
/>
</EuiAspectRatio>
);
test('maxWidth', () => {
const { getByTestSubject } = render(
<EuiAspectRatio height={16} width={9} maxWidth={500} {...requiredProps}>
<div data-test-subj="child" />
</EuiAspectRatio>
);

expect(container.firstChild).toMatchSnapshot();
});
expect(getByTestSubject('child')).toHaveStyle({
maxInlineSize: '500px',
});
});
});
68 changes: 32 additions & 36 deletions src/components/aspect_ratio/aspect_ratio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,55 +8,51 @@

import React, {
FunctionComponent,
HTMLAttributes,
ReactElement,
CSSProperties,
useMemo,
} from 'react';
import { CommonProps } from '../common';
import classNames from 'classnames';

export type EuiAspectRatioProps = HTMLAttributes<HTMLDivElement> &
CommonProps & {
/**
* Aspect ratio height. For example 9 would be widescreen video.
*/
height: number;
/**
* Aspect ratio width. For example 16 would be widescreen video.
*/
width: number;
/**
* The maximum width you want the child to stretch to.
*/
maxWidth?: CSSProperties['width'];
children: ReactElement<any>;
};
import { logicalStyles } from '../../global_styling';

export type EuiAspectRatioProps = {
/**
* Aspect ratio height. For example 9 would be widescreen video.
*/
height: number;
/**
* Aspect ratio width. For example 16 would be widescreen video.
*/
width: number;
/**
* The maximum width you want the child to stretch to.
*/
maxWidth?: CSSProperties['width'];
children: ReactElement<any>;
};

export const EuiAspectRatio: FunctionComponent<EuiAspectRatioProps> = ({
children,
className,
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
height,
width,
maxWidth,
style,
...rest
}) => {
const classes = classNames('euiAspectRatio', className);
const classes = classNames('euiAspectRatio', children.props.className);

const euiAspectRatioStyle = {
...children.props.style,
aspectRatio: `${width} / ${height}`,
height: 'auto',
width: '100%',
maxWidth,
...style,
};
const euiAspectRatioStyle = useMemo(
() =>
logicalStyles({
aspectRatio: `${width} / ${height}`,
height: 'auto',
width: '100%',
maxWidth,
}),
[height, width, maxWidth]
);

const props = {
return React.cloneElement(children, {
className: classes,
style: euiAspectRatioStyle,
...rest,
};

return React.cloneElement(children, props);
style: { ...children.props.style, ...euiAspectRatioStyle },
});
};
2 changes: 1 addition & 1 deletion src/components/avatar/__snapshots__/avatar.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ exports[`EuiAvatar props imageUrl is rendered 1`] = `
aria-label="name"
class="euiAvatar euiAvatar--m euiAvatar--user emotion-euiAvatar-user-m-uppercase"
role="img"
style="background-color: rgb(228, 166, 199); color: rgb(0, 0, 0);"
style="background-image: url(image-url.gif);"
title="name"
/>
`;
Expand Down
1 change: 1 addition & 0 deletions src/components/avatar/avatar.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export const euiAvatarStyles = ({ euiTheme }: UseEuiTheme) => ({
align-items: center;
vertical-align: middle;
background-size: cover;
background-color: ${euiTheme.colors.lightShade};
${logicalTextAlignCSS('center')}
${logicalCSS('overflow-x', 'hidden')}
/* Explicitly state weight so it doesn't get overridden by inheritance */
Expand Down
2 changes: 1 addition & 1 deletion src/components/avatar/avatar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('EuiAvatar', () => {
describe('imageUrl', () => {
it('is rendered', () => {
const { container } = render(
<EuiAvatar name="name" imageUrl="image url" />
<EuiAvatar name="name" imageUrl="image-url.gif" />
);

expect(container.firstChild).toMatchSnapshot();
Expand Down
Loading
Loading