From c3606198278659908bd907a4db36a45d58314c09 Mon Sep 17 00:00:00 2001 From: Matt Date: Sat, 15 Aug 2020 22:17:12 +0100 Subject: [PATCH] [Popover] Deprecate transition onX props (#22202) --- docs/pages/api-docs/popover.md | 12 +- packages/material-ui/src/Menu/Menu.js | 2 +- packages/material-ui/src/Menu/Menu.test.js | 54 +++--- packages/material-ui/src/Popover/Popover.js | 13 +- .../material-ui/src/Popover/Popover.test.js | 162 ++++++++++++++++-- .../material-ui/src/Select/Select.test.js | 2 +- 6 files changed, 195 insertions(+), 50 deletions(-) diff --git a/docs/pages/api-docs/popover.md b/docs/pages/api-docs/popover.md index 89cc1c7f137400..d90982d9d3d16f 100644 --- a/docs/pages/api-docs/popover.md +++ b/docs/pages/api-docs/popover.md @@ -40,12 +40,12 @@ The `MuiPopover` name can be used for providing [default props](/customization/g | getContentAnchorEl | func | | This function is called in order to retrieve the content anchor element. It's the opposite of the `anchorEl` prop. The content anchor element should be an element inside the popover. It's used to correctly scroll and set the position of the popover. The positioning strategy tries to make the content anchor element just above the anchor element. | | marginThreshold | number | 16 | Specifies how close to the edge of the window the popover can appear. | | onClose | func | | Callback fired when the component requests to be closed. | -| onEnter | func | | Callback fired before the component is entering. | -| onEntered | func | | Callback fired when the component has entered. | -| onEntering | func | | Callback fired when the component is entering. | -| onExit | func | | Callback fired before the component is exiting. | -| onExited | func | | Callback fired when the component has exited. | -| onExiting | func | | Callback fired when the component is exiting. | +| ~~onEnter~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired before the component is entering. | +| ~~onEntered~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the component has entered. | +| ~~onEntering~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the component is entering. | +| ~~onExit~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired before the component is exiting. | +| ~~onExited~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the component has exited. | +| ~~onExiting~~ | func | | *Deprecated*. Use the `TransitionProps` prop instead.

Callback fired when the component is exiting. | | open* | bool | | If `true`, the popover is visible. | | PaperProps | { component?: element type } | {} | Props applied to the [`Paper`](/api/paper/) element. | | transformOrigin | { horizontal: 'center'
| 'left'
| 'right'
| number, vertical: 'bottom'
| 'center'
| 'top'
| number } | { vertical: 'top', horizontal: 'left',} | This is the point on the popover which will attach to the anchor's origin.
Options: vertical: [top, center, bottom, x(px)]; horizontal: [left, center, right, x(px)]. | diff --git a/packages/material-ui/src/Menu/Menu.js b/packages/material-ui/src/Menu/Menu.js index deae57fb09f20d..cacf8c2097b25b 100644 --- a/packages/material-ui/src/Menu/Menu.js +++ b/packages/material-ui/src/Menu/Menu.js @@ -135,7 +135,7 @@ const Menu = React.forwardRef(function Menu(props, ref) { getContentAnchorEl={getContentAnchorEl} classes={PopoverClasses} onClose={onClose} - onEntering={handleEntering} + TransitionProps={{ onEntering: handleEntering }} anchorOrigin={theme.direction === 'rtl' ? RTL_ORIGIN : LTR_ORIGIN} transformOrigin={theme.direction === 'rtl' ? RTL_ORIGIN : LTR_ORIGIN} PaperProps={{ diff --git a/packages/material-ui/src/Menu/Menu.test.js b/packages/material-ui/src/Menu/Menu.test.js index 300b5f31c7feb9..984856f524e3c4 100644 --- a/packages/material-ui/src/Menu/Menu.test.js +++ b/packages/material-ui/src/Menu/Menu.test.js @@ -44,14 +44,16 @@ describe('', () => { const wrapper = mount( { - expect(handleEnter.callCount).to.equal(1); - expect(handleEnter.args[0].length).to.equal(2); - expect(handleEntering.callCount).to.equal(1); - expect(handleEntering.args[0].length).to.equal(2); - done(); + TransitionProps={{ + onEnter: handleEnter, + onEntering: handleEntering, + onEntered: () => { + expect(handleEnter.callCount).to.equal(1); + expect(handleEnter.args[0].length).to.equal(2); + expect(handleEntering.callCount).to.equal(1); + expect(handleEntering.args[0].length).to.equal(2); + done(); + }, }} {...defaultProps} />, @@ -70,14 +72,16 @@ describe('', () => { const wrapper = mount( { - expect(handleExit.callCount).to.equal(1); - expect(handleExit.args[0].length).to.equal(1); - expect(handleExiting.callCount).to.equal(1); - expect(handleExiting.args[0].length).to.equal(1); - done(); + TransitionProps={{ + onExit: handleExit, + onExiting: handleExiting, + onExited: () => { + expect(handleExit.callCount).to.equal(1); + expect(handleExit.args[0].length).to.equal(1); + expect(handleExiting.callCount).to.equal(1); + expect(handleExiting.args[0].length).to.equal(1); + done(); + }, }} {...defaultProps} open @@ -161,28 +165,34 @@ describe('', () => { expect(false).to.equal(menuEl.contains(document.activeElement)); }); - it('should call props.onEntering with element if exists', () => { + it('should call TransitionProps.onEntering with element if exists', () => { const onEnteringSpy = spy(); - const wrapper = mount(); + const wrapper = mount( + , + ); const popover = wrapper.find(Popover); const elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT }; - popover.props().onEntering(elementForHandleEnter); + popover.props().TransitionProps.onEntering(elementForHandleEnter); expect(onEnteringSpy.callCount).to.equal(1); expect(onEnteringSpy.calledWith(elementForHandleEnter)).to.equal(true); }); - it('should call props.onEntering, disableAutoFocusItem', () => { + it('should call TransitionProps.onEntering, disableAutoFocusItem', () => { const onEnteringSpy = spy(); const wrapper = mount( - , + , ); const popover = wrapper.find(Popover); const elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT }; - popover.props().onEntering(elementForHandleEnter); + popover.props().TransitionProps.onEntering(elementForHandleEnter); expect(onEnteringSpy.callCount).to.equal(1); expect(onEnteringSpy.calledWith(elementForHandleEnter)).to.equal(true); }); diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index d3806321122739..769a569bd5498f 100644 --- a/packages/material-ui/src/Popover/Popover.js +++ b/packages/material-ui/src/Popover/Popover.js @@ -12,6 +12,7 @@ import clsx from 'clsx'; import ownerDocument from '../utils/ownerDocument'; import ownerWindow from '../utils/ownerWindow'; import createChainedFunction from '../utils/createChainedFunction'; +import deprecatedPropType from '../utils/deprecatedPropType'; import withStyles from '../styles/withStyles'; import Modal from '../Modal'; import Grow from '../Grow'; @@ -554,27 +555,27 @@ Popover.propTypes = { /** * Callback fired before the component is entering. */ - onEnter: PropTypes.func, + onEnter: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'), /** * Callback fired when the component has entered. */ - onEntered: PropTypes.func, + onEntered: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'), /** * Callback fired when the component is entering. */ - onEntering: PropTypes.func, + onEntering: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'), /** * Callback fired before the component is exiting. */ - onExit: PropTypes.func, + onExit: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'), /** * Callback fired when the component has exited. */ - onExited: PropTypes.func, + onExited: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'), /** * Callback fired when the component is exiting. */ - onExiting: PropTypes.func, + onExiting: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` prop instead.'), /** * If `true`, the popover is visible. */ diff --git a/packages/material-ui/src/Popover/Popover.test.js b/packages/material-ui/src/Popover/Popover.test.js index 92b0b6502860d9..9bc8a4e4d0db92 100644 --- a/packages/material-ui/src/Popover/Popover.test.js +++ b/packages/material-ui/src/Popover/Popover.test.js @@ -215,7 +215,7 @@ describe('', () => { // transitions towards entered const wrapper = mount( - +
, ); @@ -271,7 +271,7 @@ describe('', () => { it('should set the inline styles for the enter phase', () => { const handleEntering = spy(); const wrapper = mount( - +
, ); @@ -335,9 +335,11 @@ describe('', () => { anchorEl={anchorEl} anchorOrigin={anchorOrigin} transitionDuration={0} - onEntered={() => { - popoverEl = document.querySelector('[data-mui-test="Popover"]'); - resolve(); + TransitionProps={{ + onEntered: () => { + popoverEl = document.querySelector('[data-mui-test="Popover"]'); + resolve(); + }, }} >
@@ -481,9 +483,11 @@ describe('', () => { anchorPosition={anchorPosition} anchorOrigin={anchorOrigin} transitionDuration={0} - onEntered={() => { - popoverEl = document.querySelector('[data-mui-test="Popover"]'); - resolve(); + TransitionProps={{ + onEntered: () => { + popoverEl = document.querySelector('[data-mui-test="Popover"]'); + resolve(); + }, }} >
@@ -525,9 +529,11 @@ describe('', () => { {...defaultProps} anchorReference="none" transitionDuration={0} - onEntered={() => { - popoverEl = document.querySelector('[data-mui-test="Popover"]'); - resolve(); + TransitionProps={{ + onEntered: () => { + popoverEl = document.querySelector('[data-mui-test="Popover"]'); + resolve(); + }, }} PaperProps={{ style: { @@ -578,7 +584,7 @@ describe('', () => { @@ -663,7 +669,7 @@ describe('', () => { @@ -785,7 +791,7 @@ describe('', () => { mount( @@ -822,6 +828,15 @@ describe('', () => { }); describe('prop: TransitionProp', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + it('chains onEntering with the apparent onEntering prop', () => { const apparentHandler = spy(); const transitionHandler = spy(); @@ -861,4 +876,123 @@ describe('', () => { expect(transitionHandler.callCount).to.equal(1); }); }); + + describe('deprecated transition callback props', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + + describe('prop: onEnter', () => { + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onEnter: () => [], + }, + 'prop', + 'Popover', + ); + + expect(console.error.callCount).to.equal(2); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed prop type: The prop `onEnter` of `Popover` is deprecated. Use the `TransitionProps` prop instead.', + ); + }); + }); + + describe('prop: onEntering', () => { + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onEntering: () => [], + }, + 'prop', + 'Popover', + ); + + expect(console.error.callCount).to.equal(2); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed prop type: The prop `onEntering` of `Popover` is deprecated. Use the `TransitionProps` prop instead.', + ); + }); + }); + + describe('prop: onEntered', () => { + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onEntered: () => [], + }, + 'prop', + 'Popover', + ); + + expect(console.error.callCount).to.equal(2); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed prop type: The prop `onEntered` of `Popover` is deprecated. Use the `TransitionProps` prop instead.', + ); + }); + }); + + describe('prop: onExit', () => { + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onExit: () => [], + }, + 'prop', + 'Popover', + ); + + expect(console.error.callCount).to.equal(2); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed prop type: The prop `onExit` of `Popover` is deprecated. Use the `TransitionProps` prop instead.', + ); + }); + }); + + describe('prop: onExiting', () => { + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onExiting: () => [], + }, + 'prop', + 'Popover', + ); + + expect(console.error.callCount).to.equal(2); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed prop type: The prop `onExiting` of `Popover` is deprecated. Use the `TransitionProps` prop instead.', + ); + }); + }); + + describe('prop: onExited', () => { + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onExited: () => [], + }, + 'prop', + 'Popover', + ); + + expect(console.error.callCount).to.equal(2); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed prop type: The prop `onExited` of `Popover` is deprecated. Use the `TransitionProps` prop instead.', + ); + }); + }); + }); }); diff --git a/packages/material-ui/src/Select/Select.test.js b/packages/material-ui/src/Select/Select.test.js index ac3f47dafbe5f3..9c4a1c67b1f752 100644 --- a/packages/material-ui/src/Select/Select.test.js +++ b/packages/material-ui/src/Select/Select.test.js @@ -540,7 +540,7 @@ describe(' + , );