Skip to content

Commit

Permalink
[enhance] Improve skip API of shouldRenderCustomStyle
Browse files Browse the repository at this point in the history
- make it an obj and allow for more granular prop skipping

- update all tests to use new API

- update tests that skipped due to different `style` location with `targetSelector`
  • Loading branch information
cee-chen committed Jul 5, 2023
1 parent 6573580 commit 42ad99f
Show file tree
Hide file tree
Showing 17 changed files with 111 additions and 58 deletions.
7 changes: 6 additions & 1 deletion src/components/card/checkable_card/checkable_card.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ describe('EuiCheckableCard', () => {

shouldRenderCustomStyles(
<EuiCheckableCard {...checkablePanelRequiredProps} />,
{ skipStyles: true } // `style` goes with ...rest onto the child check/radio input
{ skip: { style: true } }
);
// `style` goes with ...rest onto the child check/radio input, unlike className/css
shouldRenderCustomStyles(
<EuiCheckableCard {...checkablePanelRequiredProps} />,
{ targetSelector: '.euiRadio', skip: { className: true, css: true } }
);

test('renders panel props', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ const findInternalInstance = (

describe('EuiSuperDatePicker', () => {
shouldRenderCustomStyles(<EuiSuperDatePicker onTimeChange={noop} />, {
skipStyles: true,
skip: { style: true, css: true },
});
shouldRenderCustomStyles(<EuiSuperDatePicker onTimeChange={noop} />, {
childProps: ['updateButtonProps'],
skipParentTest: true,
skip: { parentTest: true },
});

it('renders', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('EuiSuperUpdateButton', () => {
/>,
{
childProps: ['toolTipProps'],
skipParentTest: true,
skip: { parentTest: true },
renderCallback: async ({ getByTestSubject }) => {
fireEvent.mouseOver(getByTestSubject('trigger'));
await waitForEuiToolTipVisible();
Expand Down
14 changes: 9 additions & 5 deletions src/components/form/checkbox/checkbox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@ const checkboxRequiredProps = {
};

describe('EuiCheckbox', () => {
shouldRenderCustomStyles(
<EuiCheckbox {...checkboxRequiredProps} />,
{ skipStyles: true } // styles get applied to the nested input, not to the className wrapper
);
shouldRenderCustomStyles(<EuiCheckbox {...checkboxRequiredProps} />, {
skip: { style: true },
});
// styles get applied to the nested input, not to the className wrapper
shouldRenderCustomStyles(<EuiCheckbox {...checkboxRequiredProps} />, {
targetSelector: '.euiCheckbox__input',
skip: { className: true, css: true },
});
shouldRenderCustomStyles(
<EuiCheckbox {...checkboxRequiredProps} label="test" />,
{ childProps: ['labelProps'], skipParentTest: true }
{ childProps: ['labelProps'], skip: { parentTest: true } }
);

test('is rendered', () => {
Expand Down
12 changes: 8 additions & 4 deletions src/components/form/range/dual_range.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@ const props = {
};

describe('EuiDualRange', () => {
shouldRenderCustomStyles(
<EuiDualRange name="name" id="id" {...props} {...requiredProps} />,
{ skipStyles: true } // style is in ...rest and is spread to a different location than className/css
);
shouldRenderCustomStyles(<EuiDualRange {...props} {...requiredProps} />, {
skip: { style: true },
});
// style is in ...rest and is spread to a different location than className/css
shouldRenderCustomStyles(<EuiDualRange {...props} {...requiredProps} />, {
targetSelector: '.euiRangeSlider',
skip: { className: true, css: true },
});

test('is rendered', () => {
const component = render(
Expand Down
18 changes: 8 additions & 10 deletions src/components/form/range/range.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,14 @@ const props = {
};

describe('EuiRange', () => {
shouldRenderCustomStyles(
<EuiRange
name="name"
id="id"
onChange={() => {}}
{...props}
{...requiredProps}
/>,
{ skipStyles: true } // style is in ...rest and is spread to a different location than className/css
);
shouldRenderCustomStyles(<EuiRange {...props} {...requiredProps} />, {
skip: { style: true },
});
// style is in ...rest and is spread to a different location than className/css
shouldRenderCustomStyles(<EuiRange {...props} {...requiredProps} />, {
targetSelector: '.euiRangeSlider',
skip: { className: true, css: true },
});

test('is rendered', () => {
const component = render(
Expand Down
9 changes: 7 additions & 2 deletions src/components/form/switch/switch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,16 @@ const props = {

describe('EuiSwitch', () => {
shouldRenderCustomStyles(<EuiSwitch {...props} />, {
skipStyles: true, // styles are applied to the nested button instead of the className wrapper
skip: { style: true },
});
// styles are applied to the nested button instead of the className wrapper
shouldRenderCustomStyles(<EuiSwitch {...props} />, {
targetSelector: '.euiSwitch__button',
skip: { className: true, css: true },
});
shouldRenderCustomStyles(<EuiSwitch {...props} />, {
childProps: ['labelProps'],
skipParentTest: true,
skip: { parentTest: true },
});

test('is rendered', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/header/header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('EuiHeader', () => {
<EuiHeader sections={[{ breadcrumbs: [{ text: 'test' }] }]} />,
{
childProps: ['sections[0].breadcrumbProps'],
skipParentTest: true,
skip: { parentTest: true },
}
);

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 @@ -26,7 +26,7 @@ describe('EuiImage', () => {
childProps: ['wrapperProps'],
});
shouldRenderCustomStyles(<EuiImage allowFullScreen {...requiredProps} />, {
skipParentTest: true,
skip: { parentTest: true },
childProps: ['wrapperProps'],
targetSelector: '.euiImageFullScreenWrapper',
renderCallback: ({ getByTestSubject }) => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/key_pad_menu/key_pad_menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('EuiKeyPadMenu', () => {
shouldRenderCustomStyles(<EuiKeyPadMenu />);
shouldRenderCustomStyles(<EuiKeyPadMenu checkable={{ legend: 'test' }} />, {
childProps: ['checkable.legendProps'],
skipParentTest: true,
skip: { parentTest: true },
});

test('is rendered', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/key_pad_menu/key_pad_menu_item.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('EuiKeyPadMenuItem', () => {
Test
</EuiKeyPadMenuItem>,
{
skipParentTest: true,
skip: { parentTest: true },
childProps: ['betaBadgeTooltipProps'],
renderCallback: async ({ getByTestSubject }) => {
fireEvent.mouseOver(getByTestSubject('trigger'));
Expand Down
9 changes: 7 additions & 2 deletions src/components/list_group/list_group_item.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ import { EuiListGroupItem, SIZES, COLORS } from './list_group_item';

describe('EuiListGroupItem', () => {
shouldRenderCustomStyles(<EuiListGroupItem label="Label" />, {
skipStyles: true, // the styles end up on the inner child
skip: { style: true },
});
// the styles end up on the inner child
shouldRenderCustomStyles(<EuiListGroupItem label="Label" />, {
targetSelector: '.euiListGroupItem__text',
skip: { className: true, css: true },
});
shouldRenderCustomStyles(
<EuiListGroupItem
Expand All @@ -25,7 +30,7 @@ describe('EuiListGroupItem', () => {
/>,
{
childProps: ['iconProps', 'extraAction'],
skipParentTest: true,
skip: { parentTest: true },
}
);

Expand Down
5 changes: 4 additions & 1 deletion src/components/popover/popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ describe('EuiPopover', () => {
);
shouldRenderCustomStyles(
<EuiPopover button={<button />} closePopover={() => {}} isOpen />,
{ childProps: ['panelProps'], skipStyles: true, skipParentTest: true }
{
childProps: ['panelProps'],
skip: { parentTest: true, style: true }, // EuiPopoverPanel does not allow custom `style`s
}
);

test('is rendered', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/progress/progress.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('EuiProgress', () => {
shouldRenderCustomStyles(<EuiProgress />);
shouldRenderCustomStyles(<EuiProgress max={100} label="Test" />, {
childProps: ['labelProps'],
skipParentTest: true,
skip: { parentTest: true },
});

test('has max', () => {
Expand Down
6 changes: 5 additions & 1 deletion src/components/resizable_container/resizable_panel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ describe('EuiResizablePanel', () => {
});
shouldRenderCustomStyles(
<EuiResizablePanel wrapperPadding="s">Test</EuiResizablePanel>,
{ wrapper, childProps: ['wrapperProps'], skipParentTest: true }
{
wrapper,
childProps: ['wrapperProps'],
skip: { parentTest: true },
}
);

it('renders', () => {
Expand Down
5 changes: 4 additions & 1 deletion src/components/tour/tour_step.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ describe('EuiTourStep', () => {
<EuiTourStep {...config} {...steps[0]} isStepOpen>
<span>Test</span>
</EuiTourStep>,
{ childProps: ['panelProps'], skipStyles: true, skipParentTest: true }
{
childProps: ['panelProps'],
skip: { parentTest: true, style: true }, // EuiPopoverPanel does not allow custom `style`s
}
);

test('is rendered', () => {
Expand Down
68 changes: 45 additions & 23 deletions src/test/internal/render_custom_styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { get, set } from 'lodash';

import { render } from '../rtl';
import { cloneElementWithCss } from '../../services/theme/clone_element';
import { keysOf } from '../../components';

export const customStyles = {
className: 'hello',
Expand All @@ -29,7 +30,6 @@ export const customStyles = {
*
* shouldRenderCustomStyles(<EuiMark {...requiredProps} />Marked</EuiMark>);
* shouldRenderCustomStyles(<EuiPageSection />, { childProps: ['contentProps'] });
* shouldRenderCustomStyles(<EuiPopover />, { childProps: ['panelProps'], skipStyles: true });
*/
type ShouldRenderCustomStylesOptions = {
/**
Expand All @@ -39,18 +39,26 @@ type ShouldRenderCustomStylesOptions = {
childProps?: string[];
/**
* If a more specific selector needs to be passed for any reason,
* e.g. if there are multiple copies of the element on the page
* e.g. if there are multiple copies of the element on the page, or
* if the node to examine isn't the same one that `className` is applied to
*/
targetSelector?: string;
/**
* Used for components that do not allow custom inline styles, or
* components where `style` isn't on the same DOM node as `className`
* Allows configuring specific assertions skips
*/
skipStyles?: boolean;
/**
* Useful for running separate parent and `childProps` tests/setups
*/
skipParentTest?: boolean;
skip?: {
/**
* Useful for, e.g. components that do not allow custom inline styles
* or components where `style` isn't on the same DOM node as `className`
*/
style?: boolean;
className?: boolean;
css?: boolean;
/**
* Useful for running separate parent and `childProps` tests/setups
*/
parentTest?: boolean;
};
/**
* Passed directly to RTL's `options.wrapper
* Used for components that need (e.g.) a context wrapper
Expand All @@ -67,17 +75,25 @@ export const shouldRenderCustomStyles = (
component: ReactElement,
options: ShouldRenderCustomStylesOptions = {}
) => {
const testCases = options.skipStyles
? 'classNames and css'
: 'classNames, css, and styles';
const testProps = options.skipStyles
? { className: customStyles.className, css: customStyles.css }
: customStyles;
// Account for any skipped props
const testProps = keysOf(customStyles).reduce((map, key) => {
return options.skip?.[key] ? map : { ...map, [key]: customStyles[key] };
}, {} as Partial<typeof customStyles>);

// Generate a grammatically excellent test title from the list of props being tested
const propsToTestArr = Object.keys(testProps);
const propsToTest =
propsToTestArr.length > 1
? // you'll pry the oxford comma from my cold dead hands
`${propsToTestArr.slice(0, -1).join(', ')}${
propsToTestArr.length > 2 ? ',' : ''
} and ${propsToTestArr.slice(-1)}`
: propsToTestArr[0];

// Some tests run separate child props tests with different settings & don't need
// to run the base parent test multiple times. If so, allow skipping this test
if (!(options.skipParentTest === true && options.childProps)) {
it(`should render custom ${testCases}`, async () => {
if (!(options.skip?.parentTest && options.childProps)) {
it(`should render custom ${propsToTest}`, async () => {
const euiCss = await getEuiEmotionCss();
const { baseElement } = await renderWith(testProps);
assertOutputStyles(baseElement, euiCss);
Expand All @@ -86,7 +102,7 @@ export const shouldRenderCustomStyles = (

if (options.childProps) {
options.childProps.forEach((childProps) => {
it(`should render custom ${testCases} on ${childProps}`, async () => {
it(`should render custom ${propsToTest} on ${childProps}`, async () => {
const euiCss = await getEuiEmotionCss(childProps);

const mergedChildProps = mergeChildProps(
Expand All @@ -108,7 +124,9 @@ export const shouldRenderCustomStyles = (
const assertOutputStyles = (rendered: HTMLElement, euiCss: string = '') => {
// className
const renderedClassName = rendered.querySelector('.hello');
expect(renderedClassName).not.toBeNull();
if (!options?.skip?.className) {
expect(renderedClassName).not.toBeNull();
}

// Set remaining assertions to use `options.targetSelector` if it exists,
// or fall back to the className selector if not
Expand All @@ -117,13 +135,15 @@ export const shouldRenderCustomStyles = (
: renderedClassName;

// css
expect(componentNode!.getAttribute('class')).toContain(
`${euiCss}-css` // should have generated an Emotion class ending with -css while still maintaining any EUI Emotion CSS already set on the component
);
if (!options?.skip?.css) {
expect(componentNode!.getAttribute('class')).toContain(
`${euiCss}-css` // should have generated an Emotion class ending with -css while still maintaining any EUI Emotion CSS already set on the component
);
}

// style
// skippable as some components explicitly do not accept custom inline styles
if (!options.skipStyles) {
if (!options.skip?.style) {
expect(componentNode!.getAttribute('style')).toContain(
"content: 'world';"
);
Expand All @@ -133,6 +153,8 @@ export const shouldRenderCustomStyles = (
// In order to check that consumer css`` is being merged correctly with EUI css``
// instead of overriding it, we need to grab the baseline component's classes
const getEuiEmotionCss = async (childProps?: string) => {
if (options.skip?.css) return;

const testProps = childProps
? mergeChildProps(component.props, childProps, {
className: customStyles.className,
Expand Down

0 comments on commit 42ad99f

Please sign in to comment.