[material-ui] Prevent ownerState propagation for transition slots (m…

…ui#44401) Signed-off-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com> Signed-off-by: Diego Andai <diego@mui.com> Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> Co-authored-by: Diego Andai <diego@mui.com>
DiegoAndai · Dec 19, 2024 · 3f851ce · 3f851ce
1 parent 523b329
commit 3f851ce
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 20 deletions.
diff --git a/packages/mui-material/src/Accordion/Accordion.test.js b/packages/mui-material/src/Accordion/Accordion.test.js
@@ -2,9 +2,14 @@ import * as React from 'react';
 import PropTypes from 'prop-types';
 import { expect } from 'chai';
 import { spy } from 'sinon';
-import { createRenderer, fireEvent, reactMajor } from '@mui/internal-test-utils';
+import { createRenderer, fireEvent, reactMajor, screen } from '@mui/internal-test-utils';
 import Accordion, { accordionClasses as classes } from '@mui/material/Accordion';
 import Paper from '@mui/material/Paper';
+import Collapse from '@mui/material/Collapse';
+import Fade from '@mui/material/Fade';
+import Slide from '@mui/material/Slide';
+import Grow from '@mui/material/Grow';
+import Zoom from '@mui/material/Zoom';
 import AccordionSummary from '@mui/material/AccordionSummary';
 import describeConformance from '../../test/describeConformance';
 
@@ -261,4 +266,48 @@ describe('<Accordion />', () => {
       expect(queryByTestId('details')).to.equal(null);
     });
   });
+
+  describe('should not forward ownerState prop to the underlying DOM element when using transition slot', () => {
+    const transitions = [
+      {
+        component: Collapse,
+        name: 'Collapse',
+      },
+      {
+        component: Fade,
+        name: 'Fade',
+      },
+      {
+        component: Grow,
+        name: 'Grow',
+      },
+      {
+        component: Slide,
+        name: 'Slide',
+      },
+      {
+        component: Zoom,
+        name: 'Zoom',
+      },
+    ];
+
+    transitions.forEach((transition) => {
+      it(transition.name, () => {
+        render(
+          <Accordion
+            defaultExpanded
+            slots={{
+              transition: transition.component,
+            }}
+            slotProps={{ transition: { timeout: 400 } }}
+          >
+            <AccordionSummary>Summary</AccordionSummary>
+            Details
+          </Accordion>,
+        );
+
+        expect(screen.getByRole('region')).not.to.have.attribute('ownerstate');
+      });
+    });
+  });
 });
diff --git a/packages/mui-material/src/Backdrop/Backdrop.js b/packages/mui-material/src/Backdrop/Backdrop.js
@@ -9,11 +9,6 @@ import useSlot from '../utils/useSlot';
 import Fade from '../Fade';
 import { getBackdropUtilityClass } from './backdropClasses';
 
-const removeOwnerState = (props) => {
-  const { ownerState, ...rest } = props;
-  return rest;
-};
-
 const useUtilityClasses = (ownerState) => {
   const { classes, invisible } = ownerState;
 
@@ -101,10 +96,9 @@ const Backdrop = React.forwardRef(function Backdrop(inProps, ref) {
     externalForwardedProps,
     ownerState,
   });
-  const transitionPropsRemoved = removeOwnerState(transitionProps);
 
   return (
-    <TransitionSlot in={open} timeout={transitionDuration} {...other} {...transitionPropsRemoved}>
+    <TransitionSlot in={open} timeout={transitionDuration} {...other} {...transitionProps}>
       <RootSlot aria-hidden {...rootProps} classes={classes} ref={ref}>
         {children}
       </RootSlot>

diff --git a/packages/mui-material/src/Collapse/Collapse.js b/packages/mui-material/src/Collapse/Collapse.js
@@ -308,7 +308,8 @@ const Collapse = React.forwardRef(function Collapse(inProps, ref) {
       timeout={timeout === 'auto' ? null : timeout}
       {...other}
     >
-      {(state, childProps) => (
+      {/* Destructure child props to prevent the component's "ownerState" from being overridden by incomingOwnerState. */}
+      {(state, { ownerState: incomingOwnerState, ...restChildProps }) => (
         <CollapseRoot
           as={component}
           className={clsx(
@@ -324,10 +325,8 @@ const Collapse = React.forwardRef(function Collapse(inProps, ref) {
             ...style,
           }}
           ref={handleRef}
-          {...childProps}
-          // `ownerState` is set after `childProps` to override any existing `ownerState` property in `childProps`
-          // that might have been forwarded from the Transition component.
           ownerState={{ ...ownerState, state }}
+          {...restChildProps}
         >
           <CollapseWrapper
             ownerState={{ ...ownerState, state }}

diff --git a/packages/mui-material/src/Collapse/Collapse.test.js b/packages/mui-material/src/Collapse/Collapse.test.js
@@ -268,4 +268,22 @@ describe('<Collapse />', () => {
       expect(handleExiting.args[0][0].style.height).to.equal(collapsedSize);
     });
   });
+
+  // Test for https://github.com/mui/material-ui/issues/40653
+  it('should render correctly when external ownerState prop is passed', function test() {
+    if (/jsdom/.test(window.navigator.userAgent)) {
+      this.skip();
+    }
+
+    const { container } = render(
+      <Collapse in ownerState={{}}>
+        <div style={{ height: '100px' }} />
+      </Collapse>,
+    );
+    const collapse = container.firstChild;
+
+    expect(collapse).toHaveComputedStyle({
+      height: '100px',
+    });
+  });
 });
diff --git a/packages/mui-material/src/Fade/Fade.js b/packages/mui-material/src/Fade/Fade.js
@@ -128,7 +128,8 @@ const Fade = React.forwardRef(function Fade(props, ref) {
       timeout={timeout}
       {...other}
     >
-      {(state, childProps) => {
+      {/* Ensure "ownerState" is not forwarded to the child DOM element when a direct HTML element is used. This avoids unexpected behavior since "ownerState" is intended for internal styling, component props and not as a DOM attribute. */}
+      {(state, { ownerState, ...restChildProps }) => {
         return React.cloneElement(children, {
           style: {
             opacity: 0,
@@ -138,7 +139,7 @@ const Fade = React.forwardRef(function Fade(props, ref) {
             ...children.props.style,
           },
           ref: handleRef,
-          ...childProps,
+          ...restChildProps,
         });
       }}
     </TransitionComponent>

diff --git a/packages/mui-material/src/Grow/Grow.js b/packages/mui-material/src/Grow/Grow.js
@@ -189,7 +189,8 @@ const Grow = React.forwardRef(function Grow(props, ref) {
       timeout={timeout === 'auto' ? null : timeout}
       {...other}
     >
-      {(state, childProps) => {
+      {/* Ensure "ownerState" is not forwarded to the child DOM element when a direct HTML element is used. This avoids unexpected behavior since "ownerState" is intended for internal styling, component props and not as a DOM attribute. */}
+      {(state, { ownerState, ...restChildProps }) => {
         return React.cloneElement(children, {
           style: {
             opacity: 0,
@@ -200,7 +201,7 @@ const Grow = React.forwardRef(function Grow(props, ref) {
             ...children.props.style,
           },
           ref: handleRef,
-          ...childProps,
+          ...restChildProps,
         });
       }}
     </TransitionComponent>

diff --git a/packages/mui-material/src/Slide/Slide.js b/packages/mui-material/src/Slide/Slide.js
@@ -252,15 +252,16 @@ const Slide = React.forwardRef(function Slide(props, ref) {
       timeout={timeout}
       {...other}
     >
-      {(state, childProps) => {
+      {/* Ensure "ownerState" is not forwarded to the child DOM element when a direct HTML element is used. This avoids unexpected behavior since "ownerState" is intended for internal styling, component props and not as a DOM attribute. */}
+      {(state, { ownerState, ...restChildProps }) => {
         return React.cloneElement(children, {
           ref: handleRef,
           style: {
             visibility: state === 'exited' && !inProp ? 'hidden' : undefined,
             ...style,
             ...children.props.style,
           },
-          ...childProps,
+          ...restChildProps,
         });
       }}
     </TransitionComponent>

diff --git a/packages/mui-material/src/Zoom/Zoom.js b/packages/mui-material/src/Zoom/Zoom.js
@@ -128,7 +128,8 @@ const Zoom = React.forwardRef(function Zoom(props, ref) {
       timeout={timeout}
       {...other}
     >
-      {(state, childProps) => {
+      {/* Ensure "ownerState" is not forwarded to the child DOM element when a direct HTML element is used. This avoids unexpected behavior since "ownerState" is intended for internal styling, component props and not as a DOM attribute. */}
+      {(state, { ownerState, ...restChildProps }) => {
         return React.cloneElement(children, {
           style: {
             transform: 'scale(0)',
@@ -138,7 +139,7 @@ const Zoom = React.forwardRef(function Zoom(props, ref) {
             ...children.props.style,
           },
           ref: handleRef,
-          ...childProps,
+          ...restChildProps,
         });
       }}
     </TransitionComponent>