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] Fix multiple css props not being properly cascaded/merged to child props #6239

Merged
merged 9 commits into from
Sep 15, 2022
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`