From f7e0549c94cf464d8a1b4b3252ba00b87e58c6a5 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Wed, 25 Oct 2023 17:51:53 +0800 Subject: [PATCH 1/6] Add tests --- .../src/FilledInput/FilledInput.test.tsx | 33 +++++++++++++++++++ .../src/FormControl/FormControl.test.tsx | 33 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/packages/mui-material-next/src/FilledInput/FilledInput.test.tsx b/packages/mui-material-next/src/FilledInput/FilledInput.test.tsx index b989695090cdb3..35a13879c95e85 100644 --- a/packages/mui-material-next/src/FilledInput/FilledInput.test.tsx +++ b/packages/mui-material-next/src/FilledInput/FilledInput.test.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; import { expect } from 'chai'; +import { ClassNames } from '@emotion/react'; import { createRenderer, describeConformance } from '@mui-internal/test-utils'; import { CssVarsProvider, extendTheme } from '@mui/material-next/styles'; import FilledInput, { filledInputClasses as classes } from '@mui/material-next/FilledInput'; @@ -65,4 +66,36 @@ describe('', () => { const root = getByTestId('test-input'); expect(root).toHaveComputedStyle({ marginTop: '10px' }); }); + + describe('Emotion compatibility', () => { + it('classes.root should overwrite built-in styles.', () => { + const { getByTestId } = render( + + {({ css }) => ( + + )} + , + ); + const input = getByTestId('root'); + + expect(getComputedStyle(input).position).to.equal('static'); + }); + + it('className should overwrite classes.root and built-in styles.', () => { + const { getByTestId } = render( + + {({ css }) => ( + + )} + , + ); + const input = getByTestId('root'); + + expect(getComputedStyle(input).position).to.equal('sticky'); + }); + }); }); diff --git a/packages/mui-material-next/src/FormControl/FormControl.test.tsx b/packages/mui-material-next/src/FormControl/FormControl.test.tsx index 1068d199c74b54..c5db1b56d1c691 100644 --- a/packages/mui-material-next/src/FormControl/FormControl.test.tsx +++ b/packages/mui-material-next/src/FormControl/FormControl.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { expect } from 'chai'; import { spy } from 'sinon'; +import { ClassNames } from '@emotion/react'; import { describeConformance, act, createRenderer, fireEvent } from '@mui-internal/test-utils'; import FormControl, { formControlClasses as classes } from '@mui/material-next/FormControl'; import FilledInput from '@mui/material-next/FilledInput'; @@ -444,4 +445,36 @@ describe('', () => { }); }); }); + + describe('Emotion compatibility', () => { + it('classes.root should overwrite built-in styles.', () => { + const { getByTestId } = render( + + {({ css }) => ( + + )} + , + ); + const root = getByTestId('root'); + + expect(getComputedStyle(root).display).to.equal('inline'); + }); + + it('className should overwrite classes.root and built-in styles.', () => { + const { getByTestId } = render( + + {({ css }) => ( + + )} + , + ); + const root = getByTestId('root'); + + expect(getComputedStyle(root).display).to.equal('inline-block'); + }); + }); }); From a92a43c1f6d5fed4a5e1a938288b3acb3887c07b Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 26 Oct 2023 18:30:38 +0800 Subject: [PATCH 2/6] Reorder joined classNames --- packages/mui-base/src/utils/mergeSlotProps.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mui-base/src/utils/mergeSlotProps.ts b/packages/mui-base/src/utils/mergeSlotProps.ts index a236de07f3d5e2..5bd696fbb4f1e3 100644 --- a/packages/mui-base/src/utils/mergeSlotProps.ts +++ b/packages/mui-base/src/utils/mergeSlotProps.ts @@ -90,10 +90,10 @@ export function mergeSlotProps< // The simpler case - getSlotProps is not defined, so no internal event handlers are defined, // so we can simply merge all the props without having to worry about extracting event handlers. const joinedClasses = clsx( + additionalProps?.className, + className, externalForwardedProps?.className, externalSlotProps?.className, - className, - additionalProps?.className, ); const mergedStyle = { From 1128861751c5766b99752b16c04050ded0e64603 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 26 Oct 2023 18:32:14 +0800 Subject: [PATCH 3/6] Revert "Reorder joined classNames" a92a43c1f6d5fed4a5e1a938288b3acb3887c07b --- packages/mui-base/src/utils/mergeSlotProps.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mui-base/src/utils/mergeSlotProps.ts b/packages/mui-base/src/utils/mergeSlotProps.ts index 5bd696fbb4f1e3..a236de07f3d5e2 100644 --- a/packages/mui-base/src/utils/mergeSlotProps.ts +++ b/packages/mui-base/src/utils/mergeSlotProps.ts @@ -90,10 +90,10 @@ export function mergeSlotProps< // The simpler case - getSlotProps is not defined, so no internal event handlers are defined, // so we can simply merge all the props without having to worry about extracting event handlers. const joinedClasses = clsx( - additionalProps?.className, - className, externalForwardedProps?.className, externalSlotProps?.className, + className, + additionalProps?.className, ); const mergedStyle = { From e1ce19203f17610d2efc0de2291dfa610763b2ff Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 26 Oct 2023 18:34:24 +0800 Subject: [PATCH 4/6] Add a test for mergeSlotProps --- .../mui-base/src/utils/mergeSlotProps.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/mui-base/src/utils/mergeSlotProps.test.ts b/packages/mui-base/src/utils/mergeSlotProps.test.ts index f7e02e4f1af7b6..0dbfe4f4abdf6c 100644 --- a/packages/mui-base/src/utils/mergeSlotProps.test.ts +++ b/packages/mui-base/src/utils/mergeSlotProps.test.ts @@ -72,6 +72,33 @@ describe('mergeSlotProps', () => { ); }); + it('joins all the class names in order from internal to external without getSlotProps', () => { + const additionalProps = { + className: 'additional', + }; + + const externalForwardedProps = { + className: 'externalForwarded', + }; + + const externalSlotProps = { + className: 'externalSlot', + }; + + const className = ['class1', 'class2']; + + const merged = mergeSlotProps({ + additionalProps, + externalForwardedProps, + externalSlotProps, + className, + }); + + expect(merged.props.className).to.equal( + 'additional class1 class2 externalForwarded externalSlot', + ); + }); + it('merges the style props', () => { const getSlotProps = () => ({ style: { From 02954df6b51a9a850f86eb381278aff4b4cb8c88 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 26 Oct 2023 18:34:37 +0800 Subject: [PATCH 5/6] Fix --- packages/mui-base/src/utils/mergeSlotProps.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mui-base/src/utils/mergeSlotProps.ts b/packages/mui-base/src/utils/mergeSlotProps.ts index a236de07f3d5e2..5bd696fbb4f1e3 100644 --- a/packages/mui-base/src/utils/mergeSlotProps.ts +++ b/packages/mui-base/src/utils/mergeSlotProps.ts @@ -90,10 +90,10 @@ export function mergeSlotProps< // The simpler case - getSlotProps is not defined, so no internal event handlers are defined, // so we can simply merge all the props without having to worry about extracting event handlers. const joinedClasses = clsx( + additionalProps?.className, + className, externalForwardedProps?.className, externalSlotProps?.className, - className, - additionalProps?.className, ); const mergedStyle = { From 9b6f48e713037f98fea6c9fba6d1f7805c60e532 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 26 Oct 2023 19:00:56 +0800 Subject: [PATCH 6/6] Tweak test --- .../mui-base/src/utils/mergeSlotProps.test.ts | 100 +++++++++--------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/packages/mui-base/src/utils/mergeSlotProps.test.ts b/packages/mui-base/src/utils/mergeSlotProps.test.ts index 0dbfe4f4abdf6c..e41ea0cdfd1736 100644 --- a/packages/mui-base/src/utils/mergeSlotProps.test.ts +++ b/packages/mui-base/src/utils/mergeSlotProps.test.ts @@ -40,63 +40,65 @@ describe('mergeSlotProps', () => { expect(merged.props.prop4).to.equal('internal'); }); - it('joins all the class names in order from internal to external', () => { - const getSlotProps = () => ({ - className: 'internal', - }); - - const additionalProps = { - className: 'additional', - }; - - const externalForwardedProps = { - className: 'externalForwarded', - }; - - const externalSlotProps = { - className: 'externalSlot', - }; - - const className = ['class1', 'class2']; - - const merged = mergeSlotProps({ - getSlotProps, - additionalProps, - externalForwardedProps, - externalSlotProps, - className, + describe('it joins all class names in order from least to most important', () => { + it('when internal classNames from getSlotProps are included', () => { + const getSlotProps = () => ({ + className: 'internal', + }); + + const additionalProps = { + className: 'additional', + }; + + const externalForwardedProps = { + className: 'externalForwarded', + }; + + const externalSlotProps = { + className: 'externalSlot', + }; + + const className = ['class1', 'class2']; + + const merged = mergeSlotProps({ + getSlotProps, + additionalProps, + externalForwardedProps, + externalSlotProps, + className, + }); + + expect(merged.props.className).to.equal( + 'internal additional class1 class2 externalForwarded externalSlot', + ); }); - expect(merged.props.className).to.equal( - 'internal additional class1 class2 externalForwarded externalSlot', - ); - }); + it('when getSlotProps is not present', () => { + const additionalProps = { + className: 'additional', + }; - it('joins all the class names in order from internal to external without getSlotProps', () => { - const additionalProps = { - className: 'additional', - }; + const externalForwardedProps = { + className: 'externalForwarded', + }; - const externalForwardedProps = { - className: 'externalForwarded', - }; + const externalSlotProps = { + className: 'externalSlot', + }; - const externalSlotProps = { - className: 'externalSlot', - }; + const className = ['class1', 'class2']; - const className = ['class1', 'class2']; + const merged = mergeSlotProps({ + additionalProps, + externalForwardedProps, + externalSlotProps, + className, + }); - const merged = mergeSlotProps({ - additionalProps, - externalForwardedProps, - externalSlotProps, - className, + expect(merged.props.className).to.equal( + 'additional class1 class2 externalForwarded externalSlot', + ); }); - - expect(merged.props.className).to.equal( - 'additional class1 class2 externalForwarded externalSlot', - ); }); it('merges the style props', () => {