Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[material-ui] Prevent ownerState propagation for transition slots #44401

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/mui-material/src/Accordion/Accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ const Accordion = React.forwardRef(function Accordion(inProps, ref) {
elementType: Collapse,
externalForwardedProps,
ownerState,
shouldAppendOwnerState: false,
});

return (
Expand Down
9 changes: 2 additions & 7 deletions packages/mui-material/src/Backdrop/Backdrop.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -100,11 +95,11 @@ const Backdrop = React.forwardRef(function Backdrop(inProps, ref) {
elementType: Fade,
externalForwardedProps,
ownerState,
shouldAppendOwnerState: false,
});
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>
Expand Down
4 changes: 1 addition & 3 deletions packages/mui-material/src/Collapse/Collapse.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,9 @@ const Collapse = React.forwardRef(function Collapse(inProps, ref) {
[isHorizontal ? 'minWidth' : 'minHeight']: collapsedSize,
...style,
}}
ownerState={{ ...ownerState, state }}
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 }}
>
<CollapseWrapper
ownerState={{ ...ownerState, state }}
Expand Down
34 changes: 34 additions & 0 deletions packages/mui-material/src/utils/useSlot.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as React from 'react';
import { expect } from 'chai';
import { createRenderer } from '@mui/internal-test-utils';
import Fade from '@mui/material/Fade';
import { TransitionProps } from '@mui/material/transitions';
import Popper from '../Popper/BasePopper';
import { styled } from '../styles';
import { SlotProps } from './types';
Expand Down Expand Up @@ -36,6 +38,38 @@ describe('useSlot', () => {
});
});

describe('transition slot', () => {
function Transition(
props: TransitionProps & {
component?: React.ElementType;
slots?: {
transition?: React.ElementType;
};
slotProps?: {
transition?: SlotProps<React.ElementType, Record<string, any>, {}>;
};
},
) {
const [SlotTransition, transitionProps] = useSlot('transition', {
className: 'transition',
elementType: Fade,
externalForwardedProps: props,
ownerState: { disabled: true },
shouldAppendOwnerState: false,
});
return (
<SlotTransition {...transitionProps}>
<div data-testid="root" />
</SlotTransition>
);
}

it('should not propagate ownerState props to the DOM', () => {
const { getByTestId } = render(<Transition />);
expect(getByTestId('root')).not.to.have.attribute('ownerstate');
});
});

describe('multiple slots', () => {
const ItemRoot = styled('button')({});
const ItemDecorator = styled('span')({});
Expand Down
33 changes: 20 additions & 13 deletions packages/mui-material/src/utils/useSlot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,16 @@ export default function useSlot<
* e.g. Autocomplete's listbox uses Popper + StyledComponent
*/
internalForwardedProps?: any;
/**
* If `false`, does not append `ownerState` prop to the element.
*
ZeeshanTamboli marked this conversation as resolved.
Show resolved Hide resolved
* @default true
*/
shouldAppendOwnerState?: boolean;
},
) {
const {
shouldAppendOwnerState = true,
className,
elementType: initialElementType,
ownerState,
Expand Down Expand Up @@ -141,19 +148,19 @@ export default function useSlot<
| React.ElementType
| undefined;

const props = appendOwnerState(
elementType,
{
...(name === 'root' && !rootComponent && !slots[name] && internalForwardedProps),
...(name !== 'root' && !slots[name] && internalForwardedProps),
...mergedProps,
...(LeafComponent && {
as: LeafComponent,
}),
ref,
},
finalOwnerState,
);
let props = {
...(name === 'root' && !rootComponent && !slots[name] && internalForwardedProps),
...(name !== 'root' && !slots[name] && internalForwardedProps),
...mergedProps,
...(LeafComponent && {
as: LeafComponent,
}),
ref,
};

if (shouldAppendOwnerState) {
props = appendOwnerState(elementType, props, finalOwnerState);
}

Object.keys(slotOwnerState).forEach((propName) => {
delete props[propName];
Expand Down
Loading