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

Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
7ba4b13
remove ownerState if custom transition slot is provided
ZeeshanTamboli Nov 13, 2024
ac5ca15
prettier
ZeeshanTamboli Nov 13, 2024
3018428
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Nov 13, 2024
abc5ada
add description and test
ZeeshanTamboli Nov 13, 2024
131adc5
fix test
ZeeshanTamboli Nov 13, 2024
11dc5f5
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Nov 13, 2024
17478d6
Update packages/mui-material/src/utils/useSlot.ts
ZeeshanTamboli Nov 16, 2024
f4d4d07
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Nov 20, 2024
c435a13
prevent ownerState propagation in Zoom and Fade components
ZeeshanTamboli Nov 20, 2024
c3ac857
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Nov 20, 2024
fa69642
prevent ownerState in Slide and Grow
ZeeshanTamboli Nov 20, 2024
79cee1b
Add code comments
ZeeshanTamboli Nov 20, 2024
42cc23e
pnpm proptypes
ZeeshanTamboli Nov 20, 2024
4f9606f
pnpm proptypes
ZeeshanTamboli Nov 20, 2024
7ea5845
adjust comment formatting
ZeeshanTamboli Nov 20, 2024
988ac79
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Dec 13, 2024
7c0e6d1
add tests
ZeeshanTamboli Dec 14, 2024
a2287e6
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Dec 14, 2024
158ddde
add defaultExpanded
ZeeshanTamboli Dec 14, 2024
7eec13c
do not forward incoming ownerState in Collapse component
ZeeshanTamboli Dec 17, 2024
03e4eb8
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Dec 17, 2024
f52ac6e
add test
ZeeshanTamboli Dec 18, 2024
99b7f2d
improve
ZeeshanTamboli Dec 18, 2024
b875da5
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
ZeeshanTamboli Dec 18, 2024
6524aa5
Update packages/mui-material/src/Collapse/Collapse.test.js
DiegoAndai Dec 18, 2024
2da1a43
pnpm dedupe
DiegoAndai Dec 18, 2024
82c4ed8
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
DiegoAndai Dec 18, 2024
0fc6f3d
Merge branch 'master' into remove-ownerState-propogation-accordion-tr…
DiegoAndai Dec 18, 2024
8af558e
Merge branch 'remove-ownerState-propogation-accordion-transition-slot…
ZeeshanTamboli Dec 18, 2024
bffe3d7
Fix test
DiegoAndai Dec 18, 2024
52912dd
Merge branch 'remove-ownerState-propogation-accordion-transition-slot…
ZeeshanTamboli Dec 18, 2024
8511dc8
fix lint
ZeeshanTamboli Dec 18, 2024
ea60c29
pnpm dedupe
ZeeshanTamboli Dec 18, 2024
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