Skip to content

Commit

Permalink
[Menu] Deprecate transition onX props (#22213)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mbrookes @oliviertassinari
mbrookes authored and oliviertassinari committed Nov 24, 2020
1 parent a3df136 commit 25096df
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 16 deletions.
13 changes: 7 additions & 6 deletions docs/pages/api-docs/menu.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,16 @@ The `MuiMenu` name can be used for providing [default props](/customization/glob
| <span class="prop-name">disableAutoFocusItem</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | When opening the menu will not focus the active item but the `[role="menu"]` unless `autoFocus` is also set to `false`. Not using the default means not following WAI-ARIA authoring practices. Please be considerate about possible accessibility implications. |
| <span class="prop-name">MenuListProps</span> | <span class="prop-type">object</span> | <span class="prop-default">{}</span> | Props applied to the [`MenuList`](/api/menu-list/) element. |
| <span class="prop-name">onClose</span> | <span class="prop-type">func</span> | | Callback fired when the component requests to be closed.<br><br>**Signature:**<br>`function(event: object, reason: string) => void`<br>*event:* The event source of the callback.<br>*reason:* Can be: `"escapeKeyDown"`, `"backdropClick"`, `"tabKeyDown"`. |
| <span class="prop-name">onEnter</span> | <span class="prop-type">func</span> | | Callback fired before the Menu enters. |
| <span class="prop-name">onEntered</span> | <span class="prop-type">func</span> | | Callback fired when the Menu has entered. |
| <span class="prop-name">onEntering</span> | <span class="prop-type">func</span> | | Callback fired when the Menu is entering. |
| <span class="prop-name">onExit</span> | <span class="prop-type">func</span> | | Callback fired before the Menu exits. |
| <span class="prop-name">onExited</span> | <span class="prop-type">func</span> | | Callback fired when the Menu has exited. |
| <span class="prop-name">onExiting</span> | <span class="prop-type">func</span> | | Callback fired when the Menu is exiting. |
| ~~<span class="prop-name">onEnter</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired before the Menu enters. |
| ~~<span class="prop-name">onEntered</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the Menu has entered. |
| ~~<span class="prop-name">onEntering</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the Menu is entering. |
| ~~<span class="prop-name">onExit</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired before the Menu exits. |
| ~~<span class="prop-name">onExited</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the Menu has exited. |
| ~~<span class="prop-name">onExiting</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the Menu is exiting. |
| <span class="prop-name required">open<abbr title="required">*</abbr></span> | <span class="prop-type">bool</span> | | If `true`, the menu is visible. |
| <span class="prop-name">PopoverClasses</span> | <span class="prop-type">object</span> | | `classes` prop applied to the [`Popover`](/api/popover/) element. |
| <span class="prop-name">transitionDuration</span> | <span class="prop-type">'auto'<br>&#124;&nbsp;number<br>&#124;&nbsp;{ appear?: number, enter?: number, exit?: number }</span> | <span class="prop-default">'auto'</span> | The length of the transition in `ms`, or 'auto' |
| <span class="prop-name">TransitionProps</span> | <span class="prop-type">object</span> | <span class="prop-default">{}</span> | Props applied to the transition element. By default, the element is based on this [`Transition`](http://reactcommunity.org/react-transition-group/transition) component. |
| <span class="prop-name">variant</span> | <span class="prop-type">'menu'<br>&#124;&nbsp;'selectedMenu'</span> | <span class="prop-default">'selectedMenu'</span> | The variant to use. Use `menu` to prevent selected items from impacting the initial focus and the vertical alignment relative to the anchor element. |

The `ref` is forwarded to the root element.
Expand Down
5 changes: 5 additions & 0 deletions packages/material-ui/src/Menu/Menu.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ export interface MenuProps
* The length of the transition in `ms`, or 'auto'
*/
transitionDuration?: TransitionProps['timeout'] | 'auto';
/**
* Props applied to the transition element.
* By default, the element is based on this [`Transition`](http://reactcommunity.org/react-transition-group/transition) component.
*/
TransitionProps?: TransitionProps;
/**
* The variant to use. Use `menu` to prevent selected items from impacting the initial focus
* and the vertical alignment relative to the anchor element.
Expand Down
27 changes: 18 additions & 9 deletions packages/material-ui/src/Menu/Menu.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import MenuList from '../MenuList';
import * as ReactDOM from 'react-dom';
import setRef from '../utils/setRef';
import useTheme from '../styles/useTheme';
import deprecatedPropType from '../utils/deprecatedPropType';

const RTL_ORIGIN = {
vertical: 'top',
Expand Down Expand Up @@ -45,11 +46,12 @@ const Menu = React.forwardRef(function Menu(props, ref) {
disableAutoFocusItem = false,
MenuListProps = {},
onClose,
onEntering,
onEntering: onEnteringProp,
open,
PaperProps = {},
PopoverClasses,
transitionDuration = 'auto',
TransitionProps: { onEntering, ...TransitionProps } = {},
variant = 'selectedMenu',
...other
} = props;
Expand All @@ -66,7 +68,9 @@ const Menu = React.forwardRef(function Menu(props, ref) {
if (menuListActionsRef.current) {
menuListActionsRef.current.adjustStyleForScrollbar(element, theme);
}

if (onEnteringProp) {
onEnteringProp(element, isAppearing);
}
if (onEntering) {
onEntering(element, isAppearing);
}
Expand Down Expand Up @@ -135,7 +139,7 @@ const Menu = React.forwardRef(function Menu(props, ref) {
getContentAnchorEl={getContentAnchorEl}
classes={PopoverClasses}
onClose={onClose}
TransitionProps={{ onEntering: handleEntering }}
TransitionProps={{ onEntering: handleEntering, ...TransitionProps }}
anchorOrigin={theme.direction === 'rtl' ? RTL_ORIGIN : LTR_ORIGIN}
transformOrigin={theme.direction === 'rtl' ? RTL_ORIGIN : LTR_ORIGIN}
PaperProps={{
Expand Down Expand Up @@ -216,27 +220,27 @@ Menu.propTypes = {
/**
* Callback fired before the Menu enters.
*/
onEnter: PropTypes.func,
onEnter: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'),
/**
* Callback fired when the Menu has entered.
*/
onEntered: PropTypes.func,
onEntered: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'),
/**
* Callback fired when the Menu is entering.
*/
onEntering: PropTypes.func,
onEntering: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'),
/**
* Callback fired before the Menu exits.
*/
onExit: PropTypes.func,
onExit: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'),
/**
* Callback fired when the Menu has exited.
*/
onExited: PropTypes.func,
onExited: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'),
/**
* Callback fired when the Menu is exiting.
*/
onExiting: PropTypes.func,
onExiting: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'),
/**
* If `true`, the menu is visible.
*/
Expand All @@ -261,6 +265,11 @@ Menu.propTypes = {
exit: PropTypes.number,
}),
]),
/**
* Props applied to the transition element.
* By default, the element is based on this [`Transition`](http://reactcommunity.org/react-transition-group/transition) component.
*/
TransitionProps: PropTypes.object,
/**
* The variant to use. Use `menu` to prevent selected items from impacting the initial focus
* and the vertical alignment relative to the anchor element.
Expand Down
120 changes: 119 additions & 1 deletion packages/material-ui/src/Menu/Menu.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';
import { spy } from 'sinon';
import * as PropTypes from 'prop-types';
import { spy, stub } from 'sinon';
import { expect } from 'chai';
import { getClasses } from '@material-ui/core/test-utils';
import createMount from 'test/utils/createMount';
Expand Down Expand Up @@ -37,6 +38,15 @@ describe('<Menu />', () => {
}));

describe('event callbacks', () => {
beforeEach(() => {
PropTypes.resetWarningCache();
stub(console, 'error');
});

afterEach(() => {
console.error.restore();
});

describe('entering', () => {
it('should fire callbacks', (done) => {
const handleEnter = spy();
Expand Down Expand Up @@ -93,6 +103,114 @@ describe('<Menu />', () => {
});
});
});

describe('prop: onEnter', () => {
it('issues a warning', () => {
PropTypes.checkPropTypes(
Menu.Naked.propTypes,
{
onEnter: () => [],
},
'prop',
'Menu',
);

expect(console.error.callCount).to.equal(2);
expect(console.error.firstCall.args[0]).to.equal(
'Warning: Failed prop type: The prop `onEnter` of `Menu` is deprecated. Use the `TransitionProps` prop instead.',
);
});
});

describe('prop: onEntering', () => {
it('issues a warning', () => {
PropTypes.checkPropTypes(
Menu.Naked.propTypes,
{
onEntering: () => [],
},
'prop',
'Menu',
);

expect(console.error.callCount).to.equal(2);
expect(console.error.firstCall.args[0]).to.equal(
'Warning: Failed prop type: The prop `onEntering` of `Menu` is deprecated. Use the `TransitionProps` prop instead.',
);
});
});

describe('prop: onEntered', () => {
it('issues a warning', () => {
PropTypes.checkPropTypes(
Menu.Naked.propTypes,
{
onEntered: () => [],
},
'prop',
'Menu',
);

expect(console.error.callCount).to.equal(2);
expect(console.error.firstCall.args[0]).to.equal(
'Warning: Failed prop type: The prop `onEntered` of `Menu` is deprecated. Use the `TransitionProps` prop instead.',
);
});
});

describe('prop: onExit', () => {
it('issues a warning', () => {
PropTypes.checkPropTypes(
Menu.Naked.propTypes,
{
onExit: () => [],
},
'prop',
'Menu',
);

expect(console.error.callCount).to.equal(2);
expect(console.error.firstCall.args[0]).to.equal(
'Warning: Failed prop type: The prop `onExit` of `Menu` is deprecated. Use the `TransitionProps` prop instead.',
);
});
});

describe('prop: onExiting', () => {
it('issues a warning', () => {
PropTypes.checkPropTypes(
Menu.Naked.propTypes,
{
onExiting: () => [],
},
'prop',
'Menu',
);

expect(console.error.callCount).to.equal(2);
expect(console.error.firstCall.args[0]).to.equal(
'Warning: Failed prop type: The prop `onExiting` of `Menu` is deprecated. Use the `TransitionProps` prop instead.',
);
});
});

describe('prop: onExited', () => {
it('issues a warning', () => {
PropTypes.checkPropTypes(
Menu.Naked.propTypes,
{
onExited: () => [],
},
'prop',
'Menu',
);

expect(console.error.callCount).to.equal(2);
expect(console.error.firstCall.args[0]).to.equal(
'Warning: Failed prop type: The prop `onExited` of `Menu` is deprecated. Use the `TransitionProps` prop instead.',
);
});
});
});

it('should pass `classes.paper` to the Popover', () => {
Expand Down

0 comments on commit 25096df

Please sign in to comment.