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.

Sign up for GitHub

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][Menu] Deprecate TransitionComponentProps #42218

Closed
wants to merge 3 commits into from
Closed

[material-ui][Menu] Deprecate TransitionComponentProps #42218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fa1011
Depricate Menu Transition
May 13, 2024
764d49c
Merge branch 'mui:next' into depricate-menu-transition
harry-whorlow May 13, 2024
6377230
questions
May 16, 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
26 changes: 25 additions & 1 deletion packages/mui-material/src/Menu/Menu.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,29 @@ import { MenuListProps } from '../MenuList';
import { Theme } from '../styles';
import { TransitionProps } from '../transitions/transition';
import { MenuClasses } from './menuClasses';
import { CreateSlotsAndSlotProps, SlotProps } from '../utils/types';

export interface MenuProps extends StandardProps<PopoverProps> {
export interface MenuSlots {
/**
* The component that renders the transition.
* [Follow this guide](/material-ui/transitions/#transitioncomponent-prop) to learn more about the requirements for this component.
* @default {}
*/
transition?: React.JSXElementConstructor<
TransitionProps & { children: React.ReactElement<any, any> }
>;
}

export type MenuSlotsAndSlotProps = CreateSlotsAndSlotProps<
{},
{
transition: SlotProps<React.JSXElementConstructor<TransitionProps>, {}, MenuOwnerState>;
}
>;

export interface MenuProps
extends Omit<StandardProps<PopoverProps>, 'slots' | 'slotProps'>,
MenuSlotsAndSlotProps {
/**
* An HTML element, or a function that returns one.
* It's used to set the position of the menu.
Expand Down Expand Up @@ -71,6 +92,7 @@ export interface MenuProps extends StandardProps<PopoverProps> {
* Props applied to the transition element.
* By default, the element is based on this [`Transition`](https://reactcommunity.org/react-transition-group/transition/) component.
* @default {}
* @deprecated Use `slotProps.transition` instead. This prop will be removed in v7. [How to migrate](/material-ui/migration/migrating-from-deprecated-apis/)
*/
TransitionProps?: TransitionProps;
/**
Expand All @@ -80,6 +102,8 @@ export interface MenuProps extends StandardProps<PopoverProps> {
variant?: 'menu' | 'selectedMenu';
}

export interface MenuOwnerState extends MenuProps {}

export declare const MenuPaper: React.FC<PaperProps>;

/**
Expand Down
26 changes: 23 additions & 3 deletions packages/mui-material/src/Menu/Menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Popover, { PopoverPaper } from '../Popover';
import rootShouldForwardProp from '../styles/rootShouldForwardProp';
import { styled, createUseThemeProps } from '../zero-styled';
import { getMenuUtilityClass } from './menuClasses';
import useSlot from '../utils/useSlot';

const useThemeProps = createUseThemeProps('MuiMenu');

Expand Down Expand Up @@ -80,7 +81,7 @@ const Menu = React.forwardRef(function Menu(inProps, ref) {
PaperProps = {},
PopoverClasses,
transitionDuration = 'auto',
TransitionProps: { onEntering, ...TransitionProps } = {},
TransitionProps: TransitionPropsProp,
variant = 'selectedMenu',
slots = {},
slotProps = {},
Expand All @@ -89,6 +90,7 @@ const Menu = React.forwardRef(function Menu(inProps, ref) {

const isRtl = useRtl();

const { onEntering, ...TransitionProps } = TransitionPropsProp ?? {};
const ownerState = {
...props,
autoFocus,
Expand Down Expand Up @@ -180,8 +182,26 @@ const Menu = React.forwardRef(function Menu(inProps, ref) {
className: classes.paper,
});

// as transitionComponent doesn't exists is this required? or should i be passing it something else,
// I can see its used in the popover component of the popover paper. so perhaps the component isn't needed
const backwardCompatibleSlots = { transition: TransitionComponentProp, ...slots };
const backwardCompatibleSlotProps = { transition: TransitionPropsProp, ...slotProps };

const externalForwardedProps = {
slots: backwardCompatibleSlots,
slotProps: backwardCompatibleSlotProps,
};
Comment on lines +185 to +193
Copy link
Contributor Author

@harry-whorlow harry-whorlow May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question one

Kind of something like this:
Screenshot 2024-05-17 at 14 51 43


// am I correct in thinking that this replaces menu as you need to provide it the transitionProps
// or should this wrap the menu component? Because in all other examples there was a pre existing transition component
const [TransitionSlot, transitionProps] = useSlot('transition', {
elementType: MenuRoot,
externalForwardedProps,
ownerState,
});

return (
<MenuRoot
<TransitionSlot
Comment on lines +195 to +204
Copy link
Contributor Author

@harry-whorlow harry-whorlow May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question two

Or am I incorrect in that assumption and it should be more something like this... (Avatar.js)
Screenshot 2024-05-17 at 14 56 56

onClose={onClose}
anchorOrigin={{
vertical: 'bottom',
Expand Down Expand Up @@ -215,7 +235,7 @@ const Menu = React.forwardRef(function Menu(inProps, ref) {
>
{children}
</MenuMenuList>
</MenuRoot>
</TransitionSlot>
);
});

Expand Down
Loading