Skip to content

Commit

Permalink
[Emotion] Fix multiple css props not being properly cascaded/merged t…
Browse files Browse the repository at this point in the history
…o child props (#6239)

* Update `shouldRenderCustomStyles` to use RTL instead of enzyme

* Add support for testing child componentProps to `shouldRenderCustomStyles`

* [EuiPageSection] Fix `contentProps` not correctly merging passed `css`

* Fix EuiPageHeaderContent not correctly merging `className`

+ add regression tests for child props

* Fix EuiAccordion not correctly inheriting `css` props

* Fix EuiProgress not correctly inheriting `css` prop

* Fix EuiImage wrapperProps

* add childProps regression tests to components that already work

* changelog
  • Loading branch information
Constance authored Sep 15, 2022
1 parent de86d7d commit 7f89bfe
Show file tree
Hide file tree
Showing 17 changed files with 107 additions and 36 deletions.
6 changes: 6 additions & 0 deletions src/components/accordion/accordion.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@
import React from 'react';
import { render, mount } from 'enzyme';
import { requiredProps } from '../../test/required_props';
import { shouldRenderCustomStyles } from '../../test/internal';

import { EuiAccordion } from './accordion';

let id = 0;
const getId = () => `${id++}`;

describe('EuiAccordion', () => {
shouldRenderCustomStyles(<EuiAccordion id="styles" />, [
'buttonProps',
'arrowProps',
]);

test('is rendered', () => {
const component = render(<EuiAccordion id={getId()} {...requiredProps} />);

Expand Down
4 changes: 2 additions & 2 deletions src/components/accordion/accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,9 @@ export class EuiAccordionClass extends Component<
iconButton = (
<EuiButtonIcon
color="text"
css={cssIconButtonStyles}
{...arrowProps}
className={iconButtonClasses}
css={cssIconButtonStyles}
iconType="arrowRight"
onClick={this.onToggle}
aria-controls={id}
Expand Down Expand Up @@ -369,10 +369,10 @@ export class EuiAccordionClass extends Component<

const button = (
<ButtonElement
css={cssButtonStyles}
{...buttonProps}
id={buttonId}
className={buttonClasses}
css={cssButtonStyles}
aria-controls={id}
aria-expanded={isOpen}
onClick={isDisabled ? undefined : this.onToggle}
Expand Down
9 changes: 7 additions & 2 deletions src/components/description_list/description_list.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@
import React from 'react';
import { render } from 'enzyme';
import { requiredProps } from '../../test/required_props';
import { shouldRenderCustomStyles } from '../../test/internal';

import { EuiDescriptionList } from './description_list';
import { TYPES, ALIGNMENTS, GUTTER_SIZES } from './description_list_types';
import { shouldRenderCustomStyles } from '../../test/internal';

describe('EuiDescriptionList', () => {
shouldRenderCustomStyles(<EuiDescriptionList />);
shouldRenderCustomStyles(
<EuiDescriptionList
listItems={[{ title: 'hello', description: 'world' }]}
/>,
['titleProps', 'descriptionProps']
);

test('is rendered', () => {
const component = render(
Expand Down
3 changes: 2 additions & 1 deletion src/components/expression/expression.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import { EuiExpression, COLORS } from './expression';

describe('EuiExpression', () => {
shouldRenderCustomStyles(
<EuiExpression description="the answer is" value="42" />
<EuiExpression description="the answer is" value="42" />,
['descriptionProps', 'valueProps']
);

test('renders', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/image/image.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('EuiImage', () => {
src: '/cat.jpg',
};

shouldRenderCustomStyles(<EuiImage {...requiredProps} />);
shouldRenderCustomStyles(<EuiImage {...requiredProps} />, ['wrapperProps']);

test('is rendered', () => {
const component = render(<EuiImage {...requiredProps} />);
Expand Down
2 changes: 1 addition & 1 deletion src/components/image/image_wrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ export const EuiImageWrapper: FunctionComponent<EuiImageWrapperProps> = ({
return (
<figure
aria-label={optionalCaptionText}
css={cssFigureStyles}
{...wrapperProps}
className={classes}
css={cssFigureStyles}
>
{allowFullScreen ? (
<>
Expand Down
2 changes: 1 addition & 1 deletion src/components/page/page_body/page_body.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { PADDING_SIZES } from '../../../global_styling';
import { EuiPageBody } from './page_body';

describe('EuiPageBody', () => {
shouldRenderCustomStyles(<EuiPageBody />);
shouldRenderCustomStyles(<EuiPageBody panelled />, ['panelProps']);

test('is rendered', () => {
const component = render(<EuiPageBody {...requiredProps} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`EuiPageHeaderContent is rendered 1`] = `
<div
aria-label="aria-label"
class="euiPageHeaderContent emotion-euiPageHeaderContent"
class="euiPageHeaderContent testClass1 testClass2 emotion-euiPageHeaderContent"
data-test-subj="test subject string"
>
<div
Expand Down
18 changes: 18 additions & 0 deletions src/components/page/page_header/page_header_content.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import React from 'react';
import { render } from 'enzyme';
import { requiredProps } from '../../../test/required_props';
import { shouldRenderCustomStyles } from '../../../test/internal';

import {
ALIGN_ITEMS,
Expand Down Expand Up @@ -50,6 +51,23 @@ const breadcrumbs: EuiBreadcrumb[] = [
];

describe('EuiPageHeaderContent', () => {
shouldRenderCustomStyles(
<EuiPageHeaderContent
pageTitle="Hello world"
iconType="logoElastic"
rightSideItems={[<button />]}
breadcrumbs={[{ text: 'breadcrumb' }]}
tabs={[{ label: 'tab' }]}
/>,
[
'pageTitleProps',
'iconProps',
'rightSideGroupProps',
'breadcrumbProps',
'tabsProps',
]
);

test('is rendered', () => {
const component = render(<EuiPageHeaderContent {...requiredProps} />);

Expand Down
2 changes: 1 addition & 1 deletion src/components/page/page_header/page_header_content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export const EuiPageHeaderContent: FunctionComponent<EuiPageHeaderContentProps>
);

const useTheme = useEuiTheme();
const classes = classNames('euiPageHeaderContent');
const classes = classNames('euiPageHeaderContent', className);
const pageHeaderStyles = euiPageHeaderStyles(useTheme);
const contentStyles = euiPageHeaderContentStyles(useTheme);
const styles = setStyleForRestrictedPageWidth(restrictWidth, style);
Expand Down
3 changes: 3 additions & 0 deletions src/components/page/page_section/page_section.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@
import React from 'react';
import { render } from 'enzyme';
import { requiredProps } from '../../../test/required_props';
import { shouldRenderCustomStyles } from '../../../test/internal';

import { EuiPageSection } from './page_section';
import { ALIGNMENTS } from './page_section.styles';
import { PADDING_SIZES, BACKGROUND_COLORS } from '../../../global_styling';

describe('EuiPageSection', () => {
shouldRenderCustomStyles(<EuiPageSection />, ['contentProps']);

test('is rendered', () => {
const component = render(<EuiPageSection {...requiredProps} />);

Expand Down
3 changes: 2 additions & 1 deletion src/components/page/page_section/page_section.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,12 @@ export const EuiPageSection: FunctionComponent<EuiPageSectionProps> = ({
bottomBorder === true && styles.border,
alignment.toLowerCase().includes('center') && contentStyles.center,
restrictWidth && contentStyles.restrictWidth,
contentProps?.css && contentProps.css,
];

return (
<Component css={cssStyles} {...rest}>
<div css={cssContentStyles} {...contentProps} style={widthStyles}>
<div {...contentProps} css={cssContentStyles} style={widthStyles}>
{children}
</div>
</Component>
Expand Down
2 changes: 1 addition & 1 deletion src/components/page_template/page_template.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { PADDING_SIZES } from '../../global_styling';
import { EuiPageTemplate } from './page_template';

describe('EuiPageTemplate', () => {
shouldRenderCustomStyles(<EuiPageTemplate />);
shouldRenderCustomStyles(<EuiPageTemplate />, ['mainProps']);

test('is rendered', () => {
const component = render(<EuiPageTemplate {...requiredProps} />);
Expand Down
4 changes: 3 additions & 1 deletion src/components/progress/progress.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ describe('EuiProgress', () => {
});

shouldRenderCustomStyles(<EuiProgress />);
shouldRenderCustomStyles(<EuiProgress max={100} />);
shouldRenderCustomStyles(<EuiProgress max={100} label="Test" />, [
'labelProps',
]);

test('has max', () => {
const component = render(<EuiProgress max={100} {...requiredProps} />);
Expand Down
2 changes: 1 addition & 1 deletion src/components/progress/progress.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ export const EuiProgress: FunctionComponent<ExclusiveUnion<
<span
title={innerText}
ref={ref}
{...labelProps}
css={labelCssStyles}
{...labelProps}
className={labelClasses}
>
{label}
Expand Down
72 changes: 50 additions & 22 deletions src/test/internal/render_custom_styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,67 @@
*/

import React, { ReactElement } from 'react';
import { render } from 'enzyme';
import { render } from '@testing-library/react';
import { css } from '@emotion/react';

const customStyles = {
className: 'hello',
css: css`
color: red;
`,
style: { content: "'world'" },
};

const assertOutputStyles = (rendered: HTMLElement) => {
// className
const componentNode = rendered.querySelector('.hello');
expect(componentNode).not.toBeNull();
// css
expect(componentNode!.getAttribute('class')).toEqual(
expect.stringMatching(/css-[\d\w-]{6,}-css/) // should have generated an emotion class ending with -css
);
// style
expect(componentNode!.getAttribute('style')).toContain("content: 'world';");
};

/**
* Use this test helper to quickly check that the component supports custom
* `className`, `css`, and `style` properties.
*
* Use the second childProps arg to ensure that any child component props
* also correctly accept custom css/classes/styles.
*
* Example usage:
*
* shouldRenderCustomStyles(<EuiMark {...requiredProps} />Marked</EuiMark>);
* shouldRenderCustomStyles(<EuiPageSection />, ['contentProps']);
*/
export const shouldRenderCustomStyles = (component: ReactElement) => {
export const shouldRenderCustomStyles = (
component: ReactElement,
childProps?: string[]
) => {
it('should render custom classNames, css, and styles', () => {
const rendered = render(
<div>
{React.cloneElement(component, {
className: 'hello',
css: css`
color: red;
`,
style: { content: "'world'" },
})}
</div>
);

// className
const componentNode = rendered.find('.hello');
expect(componentNode).toHaveLength(1);
// css
expect(componentNode.attr('class')).toEqual(
expect.stringMatching(/css-[\d\w-]{6,}-css/) // should have generated an emotion class ending with -css
const { baseElement } = render(
<div>{React.cloneElement(component, customStyles)}</div>
);
// style
expect(componentNode.attr('style')).toContain("content:'world'");
assertOutputStyles(baseElement);
});

if (childProps) {
childProps.forEach((_childProps) => {
it(`should render custom classNames, css, and styles on ${_childProps}`, () => {
const { baseElement } = render(
<div>
{React.cloneElement(component, {
[_childProps]: {
...(component.props[_childProps] || {}),
...customStyles,
},
})}
</div>
);
assertOutputStyles(baseElement);
});
});
}
};
7 changes: 7 additions & 0 deletions upcoming_changelogs/6239.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
**Bug fixes**

- Fixed `EuiPageSection` not correctly merging `contentProps.css`
- Fixed `EuiPageHeaderContent` not correctly merging passed `className`s
- Fixed `EuiAccordion` not correctly merging `buttonProps.css` and `arrowProps.css`
- Fixed `EuiProgress` not correctly merging `labelProps.css`
- Fixed `EuiImage` not correctly merging `wrapperProps.css`

0 comments on commit 7f89bfe

Please sign in to comment.