Skip to content

Commit

Permalink
[Material][Menu] Fix MenuPaper class composition precedence (#37390)
Browse files Browse the repository at this point in the history
  • Loading branch information
DiegoAndai committed May 31, 2023
1 parent 3cab242 commit 5c8b339
Show file tree
Hide file tree
Showing 8 changed files with 319 additions and 69 deletions.
13 changes: 13 additions & 0 deletions docs/pages/material-ui/api/popover.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@
"onClose": { "type": { "name": "func" } },
"PaperProps": {
"type": { "name": "shape", "description": "{ component?: element type }" },
"default": "{}",
"deprecated": true,
"deprecationInfo": "Use <code>slotProps.paper</code> instead."
},
"slotProps": {
"type": {
"name": "shape",
"description": "{ paper?: func<br>&#124;&nbsp;object, root?: func<br>&#124;&nbsp;object }"
},
"default": "{}"
},
"slots": {
"type": { "name": "shape", "description": "{ paper?: elementType, root?: elementType }" },
"default": "{}"
},
"sx": {
Expand Down
4 changes: 3 additions & 1 deletion docs/translations/api-docs/popover/popover.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
"marginThreshold": "Specifies how close to the edge of the window the popover can appear.",
"onClose": "Callback fired when the component requests to be closed. The <code>reason</code> parameter can optionally be used to control the response to <code>onClose</code>.",
"open": "If <code>true</code>, the component is shown.",
"PaperProps": "Props applied to the <a href=\"/material-ui/api/paper/\"><code>Paper</code></a> element.",
"PaperProps": "Props applied to the <a href=\"/material-ui/api/paper/\"><code>Paper</code></a> element.<br>This prop is an alias for <code>slotProps.paper</code> and will be overriden by it if both are used.",
"slotProps": "The extra props for the slot components. You can override the existing props or add new ones.",
"slots": "The components used for each slot inside.",
"sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the <a href=\"/system/getting-started/the-sx-prop/\">`sx` page</a> for more details.",
"transformOrigin": "This is the point on the popover which will attach to the anchor&#39;s origin.<br>Options: vertical: [top, center, bottom, x(px)]; horizontal: [left, center, right, x(px)].",
"TransitionComponent": "The component used for the transition. <a href=\"/material-ui/transitions/#transitioncomponent-prop\">Follow this guide</a> to learn more about the requirements for this component.",
Expand Down
3 changes: 3 additions & 0 deletions packages/mui-material/src/Menu/Menu.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import { SxProps } from '@mui/system';
import { InternalStandardProps as StandardProps } from '..';
import { PaperProps } from '../Paper';
import { PopoverProps } from '../Popover';
import { MenuListProps } from '../MenuList';
import { Theme } from '../styles';
Expand Down Expand Up @@ -79,6 +80,8 @@ export interface MenuProps extends StandardProps<PopoverProps> {
variant?: 'menu' | 'selectedMenu';
}

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

/**
*
* Demos:
Expand Down
19 changes: 10 additions & 9 deletions packages/mui-material/src/Menu/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import clsx from 'clsx';
import { unstable_composeClasses as composeClasses } from '@mui/base';
import { HTMLElementType } from '@mui/utils';
import MenuList from '../MenuList';
import Paper from '../Paper';
import Popover from '../Popover';
import Popover, { PopoverPaper } from '../Popover';
import styled, { rootShouldForwardProp } from '../styles/styled';
import useTheme from '../styles/useTheme';
import useThemeProps from '../styles/useThemeProps';
Expand Down Expand Up @@ -41,7 +40,7 @@ const MenuRoot = styled(Popover, {
overridesResolver: (props, styles) => styles.root,
})({});

const MenuPaper = styled(Paper, {
export const MenuPaper = styled(PopoverPaper, {
name: 'MuiMenu',
slot: 'Paper',
overridesResolver: (props, styles) => styles.paper,
Expand Down Expand Up @@ -164,12 +163,14 @@ const Menu = React.forwardRef(function Menu(inProps, ref) {
horizontal: isRtl ? 'right' : 'left',
}}
transformOrigin={isRtl ? RTL_ORIGIN : LTR_ORIGIN}
PaperProps={{
as: MenuPaper,
...PaperProps,
classes: {
...PaperProps.classes,
root: classes.paper,
slots={{ paper: MenuPaper }}
slotProps={{
paper: {
...PaperProps,
classes: {
...PaperProps.classes,
root: classes.paper,
},
},
}}
className={classes.root}
Expand Down
62 changes: 53 additions & 9 deletions packages/mui-material/src/Menu/Menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { spy } from 'sinon';
import { expect } from 'chai';
import {
createRenderer,
createMount,
describeConformance,
screen,
fireEvent,
Expand All @@ -11,9 +12,11 @@ import {
import Menu, { menuClasses as classes } from '@mui/material/Menu';
import Popover from '@mui/material/Popover';
import { createTheme, ThemeProvider } from '@mui/material/styles';
import { MenuPaper } from './Menu';

describe('<Menu />', () => {
const { render } = createRenderer({ clock: 'fake' });
const mount = createMount();

describeConformance(<Menu anchorEl={() => document.createElement('div')} open />, () => ({
classes,
Expand Down Expand Up @@ -127,6 +130,27 @@ describe('<Menu />', () => {
});
});

describe('prop: PaperProps', () => {
it('should be passed to the paper component', () => {
const customElevation = 12;
const customClasses = { rounded: { borderRadius: 12 } };
const wrapper = mount(
<Menu
anchorEl={document.createElement('div')}
open
PaperProps={{
'data-testid': 'paper',
elevation: customElevation,
classes: customClasses,
}}
/>,
);

expect(wrapper.find(MenuPaper).props().elevation).to.equal(customElevation);
expect(wrapper.find(MenuPaper).props().classes).to.contain(customClasses);
});
});

it('should pass onClose prop to Popover', () => {
const handleClose = spy();
render(<Menu anchorEl={document.createElement('div')} open onClose={handleClose} />);
Expand Down Expand Up @@ -275,15 +299,21 @@ describe('<Menu />', () => {
});
});

describe('should be customizable in theme', () => {
it('should override Paper styles in Menu taking MuiMenu.paper styles into account', function test() {
describe('theme customization', () => {
it('should override Menu Paper styles following correct precedence', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
this.skip();
}

const menuPaperOverrides = { borderRadius: 4 };
const popoverPaperOverrides = { borderRadius: 8, height: 100 };
const rootPaperOverrides = { borderRadius: 16, height: 200, width: 200 };

const theme = createTheme({
components: {
MuiMenu: { styleOverrides: { paper: { borderRadius: 4 } } },
MuiPaper: { styleOverrides: { rounded: { borderRadius: 90 } } },
MuiMenu: { styleOverrides: { paper: menuPaperOverrides } },
MuiPopover: { styleOverrides: { paper: popoverPaperOverrides } },
MuiPaper: { styleOverrides: { root: rootPaperOverrides } },
},
});

Expand All @@ -301,14 +331,16 @@ describe('<Menu />', () => {

const paper = screen.getByTestId('paper');
expect(paper).toHaveComputedStyle({
borderTopLeftRadius: '4px',
borderBottomLeftRadius: '4px',
borderTopRightRadius: '4px',
borderBottomRightRadius: '4px',
borderTopLeftRadius: `${menuPaperOverrides.borderRadius}px`,
borderBottomLeftRadius: `${menuPaperOverrides.borderRadius}px`,
borderTopRightRadius: `${menuPaperOverrides.borderRadius}px`,
borderBottomRightRadius: `${menuPaperOverrides.borderRadius}px`,
height: `${popoverPaperOverrides.height}px`,
width: `${rootPaperOverrides.width}px`,
});
});

it('should override Paper styles in Menu using styles in MuiPaper slot', function test() {
it('should override Menu Paper styles using styles in MuiPaper slot', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
this.skip();
}
Expand Down Expand Up @@ -340,4 +372,16 @@ describe('<Menu />', () => {
});
});
});

describe('paper', () => {
it('should use MenuPaper component', () => {
const wrapper = mount(
<Menu anchorEl={document.createElement('div')} open>
<div />
</Menu>,
);

expect(wrapper.find(MenuPaper)).to.have.length(1);
});
});
});
37 changes: 34 additions & 3 deletions packages/mui-material/src/Popover/Popover.d.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import * as React from 'react';
import { SxProps } from '@mui/system';
import { SlotComponentProps } from '@mui/base';
import { InternalStandardProps as StandardProps } from '..';
import { PaperProps } from '../Paper';
import { ModalProps } from '../Modal';
import Paper, { PaperProps } from '../Paper';
import Modal, { ModalOwnerState, ModalProps } from '../Modal';
import { Theme } from '../styles';
import { TransitionProps } from '../transitions/transition';
import { PopoverClasses } from './popoverClasses';
Expand All @@ -19,7 +20,8 @@ export interface PopoverPosition {

export type PopoverReference = 'anchorEl' | 'anchorPosition' | 'none';

export interface PopoverProps extends StandardProps<ModalProps, 'children'> {
export interface PopoverProps
extends StandardProps<Omit<ModalProps, 'slots' | 'slotProps'>, 'children'> {
/**
* A ref for imperative actions.
* It currently only supports updatePosition() action.
Expand Down Expand Up @@ -88,9 +90,32 @@ export interface PopoverProps extends StandardProps<ModalProps, 'children'> {
open: boolean;
/**
* Props applied to the [`Paper`](/material-ui/api/paper/) element.
*
* This prop is an alias for `slotProps.paper` and will be overriden by it if both are used.
* @deprecated Use `slotProps.paper` instead.
*
* @default {}
*/
PaperProps?: Partial<PaperProps>;
/**
* The components used for each slot inside.
*
* @default {}
*/
slots?: {
root?: React.ElementType;
paper?: React.ElementType;
};
/**
* The extra props for the slot components.
* You can override the existing props or add new ones.
*
* @default {}
*/
slotProps?: {
root?: SlotComponentProps<typeof Modal, {}, ModalOwnerState>;
paper?: SlotComponentProps<typeof Paper, {}, {}>;
};
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
Expand Down Expand Up @@ -140,6 +165,12 @@ export function getOffsetLeft(
horizontal: number | 'center' | 'right' | 'left',
): number;

type PopoverRootProps = NonNullable<PopoverProps['slotProps']>['root'];
type PopoverPaperProps = NonNullable<PopoverProps['slotProps']>['paper'];

export declare const PopoverRoot: React.FC<PopoverRootProps>;
export declare const PopoverPaper: React.FC<PopoverPaperProps>;

/**
*
* Demos:
Expand Down
Loading

0 comments on commit 5c8b339

Please sign in to comment.