From d2a646f89a28b4307e3e6d7f03e2916d21547f94 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 17:42:48 -0700 Subject: [PATCH 1/9] Update `shouldRenderCustomStyles` to use RTL instead of enzyme --- src/test/internal/render_custom_styles.tsx | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/test/internal/render_custom_styles.tsx b/src/test/internal/render_custom_styles.tsx index 2bd9c3991aa..0fbe207b728 100644 --- a/src/test/internal/render_custom_styles.tsx +++ b/src/test/internal/render_custom_styles.tsx @@ -7,7 +7,7 @@ */ import React, { ReactElement } from 'react'; -import { render } from 'enzyme'; +import { render } from '@testing-library/react'; import { css } from '@emotion/react'; /** @@ -20,26 +20,18 @@ import { css } from '@emotion/react'; */ export const shouldRenderCustomStyles = (component: ReactElement) => { it('should render custom classNames, css, and styles', () => { - const rendered = render( -
- {React.cloneElement(component, { - className: 'hello', - css: css` - color: red; - `, - style: { content: "'world'" }, - })} -
+ const { baseElement } = render( +
{React.cloneElement(component, customStyles)}
); // className - const componentNode = rendered.find('.hello'); - expect(componentNode).toHaveLength(1); + const componentNode = baseElement.querySelector('.hello'); + expect(componentNode).not.toBeNull(); // css - expect(componentNode.attr('class')).toEqual( + expect(componentNode!.getAttribute('class')).toEqual( expect.stringMatching(/css-[\d\w-]{6,}-css/) // should have generated an emotion class ending with -css ); // style - expect(componentNode.attr('style')).toContain("content:'world'"); + expect(componentNode!.getAttribute('style')).toContain("content: 'world';"); }); }; From 5f4af08b2527af9ea11b840724048b796c5e1861 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 18:48:09 -0700 Subject: [PATCH 2/9] Add support for testing child componentProps to `shouldRenderCustomStyles` --- src/test/internal/render_custom_styles.tsx | 58 ++++++++++++++++++---- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/src/test/internal/render_custom_styles.tsx b/src/test/internal/render_custom_styles.tsx index 0fbe207b728..4d327195bc4 100644 --- a/src/test/internal/render_custom_styles.tsx +++ b/src/test/internal/render_custom_styles.tsx @@ -10,28 +10,64 @@ import React, { ReactElement } from 'react'; 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(Marked); + * shouldRenderCustomStyles(, ['contentProps']); */ -export const shouldRenderCustomStyles = (component: ReactElement) => { +export const shouldRenderCustomStyles = ( + component: ReactElement, + childProps?: string[] +) => { it('should render custom classNames, css, and styles', () => { const { baseElement } = render(
{React.cloneElement(component, customStyles)}
); - - // className - const componentNode = baseElement.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';"); + assertOutputStyles(baseElement); }); + + if (childProps) { + childProps.forEach((_childProps) => { + it(`should render custom classNames, css, and styles on ${_childProps}`, () => { + const { baseElement } = render( +
+ {React.cloneElement(component, { + [_childProps]: { + ...(component.props[_childProps] || {}), + ...customStyles, + }, + })} +
+ ); + assertOutputStyles(baseElement); + }); + }); + } }; From ae6a054a88bd904651b99eccb4fe91a590e63afd Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 18:52:23 -0700 Subject: [PATCH 3/9] [EuiPageSection] Fix `contentProps` not correctly merging passed `css` --- src/components/page/page_section/page_section.test.tsx | 3 +++ src/components/page/page_section/page_section.tsx | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/page/page_section/page_section.test.tsx b/src/components/page/page_section/page_section.test.tsx index 79cae28d2cc..86061a3233b 100644 --- a/src/components/page/page_section/page_section.test.tsx +++ b/src/components/page/page_section/page_section.test.tsx @@ -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(, ['contentProps']); + test('is rendered', () => { const component = render(); diff --git a/src/components/page/page_section/page_section.tsx b/src/components/page/page_section/page_section.tsx index a3dfffadcbe..e87d9caa977 100644 --- a/src/components/page/page_section/page_section.tsx +++ b/src/components/page/page_section/page_section.tsx @@ -100,11 +100,12 @@ export const EuiPageSection: FunctionComponent = ({ bottomBorder === true && styles.border, alignment.toLowerCase().includes('center') && contentStyles.center, restrictWidth && contentStyles.restrictWidth, + contentProps?.css && contentProps.css, ]; return ( -
+
{children}
From 96ea8e9d900a860056986c1910e2028ce46fa00a Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 23:00:46 -0700 Subject: [PATCH 4/9] Fix EuiPageHeaderContent not correctly merging `className` + add regression tests for child props --- .../page_header_content.test.tsx.snap | 2 +- .../page_header/page_header_content.test.tsx | 18 ++++++++++++++++++ .../page/page_header/page_header_content.tsx | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/components/page/page_header/__snapshots__/page_header_content.test.tsx.snap b/src/components/page/page_header/__snapshots__/page_header_content.test.tsx.snap index fffc0d85915..40a94e6935d 100644 --- a/src/components/page/page_header/__snapshots__/page_header_content.test.tsx.snap +++ b/src/components/page/page_header/__snapshots__/page_header_content.test.tsx.snap @@ -3,7 +3,7 @@ exports[`EuiPageHeaderContent is rendered 1`] = `
{ + shouldRenderCustomStyles( + ]} + breadcrumbs={[{ text: 'breadcrumb' }]} + tabs={[{ label: 'tab' }]} + />, + [ + 'pageTitleProps', + 'iconProps', + 'rightSideGroupProps', + 'breadcrumbProps', + 'tabsProps', + ] + ); + test('is rendered', () => { const component = render(); diff --git a/src/components/page/page_header/page_header_content.tsx b/src/components/page/page_header/page_header_content.tsx index 9f60ae106af..42ad7261939 100644 --- a/src/components/page/page_header/page_header_content.tsx +++ b/src/components/page/page_header/page_header_content.tsx @@ -169,7 +169,7 @@ export const EuiPageHeaderContent: FunctionComponent ); 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); From 131c223dd9c979921d80453e0afa711d7f4be055 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 18:54:10 -0700 Subject: [PATCH 5/9] Fix EuiAccordion not correctly inheriting `css` props --- src/components/accordion/accordion.test.tsx | 6 ++++++ src/components/accordion/accordion.tsx | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/components/accordion/accordion.test.tsx b/src/components/accordion/accordion.test.tsx index 6cc19cf896c..0b300b2dcce 100644 --- a/src/components/accordion/accordion.test.tsx +++ b/src/components/accordion/accordion.test.tsx @@ -9,6 +9,7 @@ import React from 'react'; import { render, mount } from 'enzyme'; import { requiredProps } from '../../test/required_props'; +import { shouldRenderCustomStyles } from '../../test/internal'; import { EuiAccordion } from './accordion'; @@ -16,6 +17,11 @@ let id = 0; const getId = () => `${id++}`; describe('EuiAccordion', () => { + shouldRenderCustomStyles(, [ + 'buttonProps', + 'arrowProps', + ]); + test('is rendered', () => { const component = render(); diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index ff27d9962ec..ebceaeaa24e 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -317,9 +317,9 @@ export class EuiAccordionClass extends Component< iconButton = ( Date: Wed, 14 Sep 2022 23:01:20 -0700 Subject: [PATCH 6/9] Fix EuiProgress not correctly inheriting `css` prop --- src/components/progress/progress.test.tsx | 4 +++- src/components/progress/progress.tsx | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/progress/progress.test.tsx b/src/components/progress/progress.test.tsx index e2aa36ac7c2..386e2cff585 100644 --- a/src/components/progress/progress.test.tsx +++ b/src/components/progress/progress.test.tsx @@ -21,7 +21,9 @@ describe('EuiProgress', () => { }); shouldRenderCustomStyles(); - shouldRenderCustomStyles(); + shouldRenderCustomStyles(, [ + 'labelProps', + ]); test('has max', () => { const component = render(); diff --git a/src/components/progress/progress.tsx b/src/components/progress/progress.tsx index b3186c29034..f7e1958a533 100644 --- a/src/components/progress/progress.tsx +++ b/src/components/progress/progress.tsx @@ -161,8 +161,8 @@ export const EuiProgress: FunctionComponent {label} From 7078420ef4bc2c3b5de7ad9d69743affe77abd79 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 23:00:04 -0700 Subject: [PATCH 7/9] Fix EuiImage wrapperProps --- src/components/image/image.test.tsx | 2 +- src/components/image/image_wrapper.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/image/image.test.tsx b/src/components/image/image.test.tsx index d006f715b37..075b10c6129 100644 --- a/src/components/image/image.test.tsx +++ b/src/components/image/image.test.tsx @@ -22,7 +22,7 @@ describe('EuiImage', () => { src: '/cat.jpg', }; - shouldRenderCustomStyles(); + shouldRenderCustomStyles(, ['wrapperProps']); test('is rendered', () => { const component = render(); diff --git a/src/components/image/image_wrapper.tsx b/src/components/image/image_wrapper.tsx index a2a351f6718..6073e95809a 100644 --- a/src/components/image/image_wrapper.tsx +++ b/src/components/image/image_wrapper.tsx @@ -56,9 +56,9 @@ export const EuiImageWrapper: FunctionComponent = ({ return (
{allowFullScreen ? ( <> From b73e7f99f1270b74a22a52eb4450248ab330dc04 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 22:59:07 -0700 Subject: [PATCH 8/9] add childProps regression tests to components that already work --- .../description_list/description_list.test.tsx | 9 +++++++-- src/components/expression/expression.test.tsx | 3 ++- src/components/page/page_body/page_body.test.tsx | 2 +- src/components/page_template/page_template.test.tsx | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/components/description_list/description_list.test.tsx b/src/components/description_list/description_list.test.tsx index 141eae41026..b94684be576 100644 --- a/src/components/description_list/description_list.test.tsx +++ b/src/components/description_list/description_list.test.tsx @@ -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(); + shouldRenderCustomStyles( + , + ['titleProps', 'descriptionProps'] + ); test('is rendered', () => { const component = render( diff --git a/src/components/expression/expression.test.tsx b/src/components/expression/expression.test.tsx index 4bedc4dd0ed..daf70ff003d 100644 --- a/src/components/expression/expression.test.tsx +++ b/src/components/expression/expression.test.tsx @@ -15,7 +15,8 @@ import { EuiExpression, COLORS } from './expression'; describe('EuiExpression', () => { shouldRenderCustomStyles( - + , + ['descriptionProps', 'valueProps'] ); test('renders', () => { diff --git a/src/components/page/page_body/page_body.test.tsx b/src/components/page/page_body/page_body.test.tsx index aff0a3b6470..7fb428b7bce 100644 --- a/src/components/page/page_body/page_body.test.tsx +++ b/src/components/page/page_body/page_body.test.tsx @@ -15,7 +15,7 @@ import { PADDING_SIZES } from '../../../global_styling'; import { EuiPageBody } from './page_body'; describe('EuiPageBody', () => { - shouldRenderCustomStyles(); + shouldRenderCustomStyles(, ['panelProps']); test('is rendered', () => { const component = render(); diff --git a/src/components/page_template/page_template.test.tsx b/src/components/page_template/page_template.test.tsx index 791275720fa..43dd908281e 100644 --- a/src/components/page_template/page_template.test.tsx +++ b/src/components/page_template/page_template.test.tsx @@ -15,7 +15,7 @@ import { PADDING_SIZES } from '../../global_styling'; import { EuiPageTemplate } from './page_template'; describe('EuiPageTemplate', () => { - shouldRenderCustomStyles(); + shouldRenderCustomStyles(, ['mainProps']); test('is rendered', () => { const component = render(); From 818c436b164d5cd738cc7af869d5cf3917f932e7 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Sep 2022 23:18:00 -0700 Subject: [PATCH 9/9] changelog --- upcoming_changelogs/6239.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 upcoming_changelogs/6239.md diff --git a/upcoming_changelogs/6239.md b/upcoming_changelogs/6239.md new file mode 100644 index 00000000000..a5bc57ae40d --- /dev/null +++ b/upcoming_changelogs/6239.md @@ -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`