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

[Menu] Deprecate transition onX props #22213

Merged
merged 4 commits into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 7 additions & 6 deletions docs/pages/api-docs/menu.md
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
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
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
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