Skip to content

Commit

Permalink
[FIX] Ensure that css props do not override baseline EUI css
Browse files Browse the repository at this point in the history
- this is the main fix/guard/detection for the buggy behavior found in this PR

- it acts by rendering the baseline component w/ no custom `css`, grabbing the Emotion className generated, and ensuring the custom `css` is appended to the end of it instead of gobbling it

- this behavior primarily affects nested `childProps` components, as Emotion does not auto-merge `css` props that isn't the immediately returned element / isn't at the top level of the component
  • Loading branch information
cee-chen committed Jun 30, 2023
1 parent 7a2eb0d commit 9be0f79
Showing 1 changed file with 34 additions and 5 deletions.
39 changes: 34 additions & 5 deletions src/test/internal/render_custom_styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,22 +66,25 @@ export const shouldRenderCustomStyles = (
// 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 () => {
const euiCss = await getEuiEmotionCss();
const { baseElement } = await renderWith(testProps);
assertOutputStyles(baseElement);
assertOutputStyles(baseElement, euiCss);
});
}

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

const mergedChildProps = mergeChildProps(
component.props,
childProps,
testProps
);
const { baseElement } = await renderWith(mergedChildProps);

assertOutputStyles(baseElement);
assertOutputStyles(baseElement, euiCss);
});
});
}
Expand All @@ -90,14 +93,14 @@ export const shouldRenderCustomStyles = (
* Internal utils
*/

const assertOutputStyles = (rendered: HTMLElement) => {
const assertOutputStyles = (rendered: HTMLElement, euiCss: string = '') => {
// 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
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
Expand All @@ -109,6 +112,32 @@ 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) => {
const testProps = childProps
? mergeChildProps(component.props, childProps, {
className: customStyles.className,
})
: { className: customStyles.className };

const { baseElement, unmount } = await renderWith(testProps);
const target = baseElement.querySelector(`.${customStyles.className}`)!;
const classes = target.getAttribute('class')?.split(' ');
unmount(); // Ensure this baseline render doesn't pollute following renders

let euiCss = '';
classes?.forEach((classString: string) => {
if (!classString.startsWith('css-')) return;

const matches = classString.match(/css-[\d\w-]{4,12}-(?<euiCss>eui.+)/);
if (matches?.groups?.euiCss) {
euiCss = matches.groups.euiCss;
}
});
return euiCss;
};

// Render element with specified props (merging CSS correctly as Emotion does)
// and DRYing out `options` handling
const renderWith = async (props: unknown) => {
Expand Down

0 comments on commit 9be0f79

Please sign in to comment.