From edb369a5a13e5b0a102d19b2cb2fed24e2f86478 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 14 Aug 2020 18:31:52 +0100 Subject: [PATCH] [Popover] Deprecate transition onX props --- 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 | 205 ++++++++++++++++-- .../material-ui/src/Select/Select.test.js | 2 +- 6 files changed, 238 insertions(+), 50 deletions(-) diff --git a/docs/pages/api-docs/popover.md b/docs/pages/api-docs/popover.md index 89cc1c7f137400..1a39fcfd28659c 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` property instead.

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

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

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

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

Callback fired when the component has exited. | +| ~~onExiting~~ | func | | *Deprecated*. Use the `TransitionProps` property 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..943c7817469418 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` property instead.'), /** * Callback fired when the component has entered. */ - onEntered: PropTypes.func, + onEntered: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` property instead.'), /** * Callback fired when the component is entering. */ - onEntering: PropTypes.func, + onEntering: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` property instead.'), /** * Callback fired before the component is exiting. */ - onExit: PropTypes.func, + onExit: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` property instead.'), /** * Callback fired when the component has exited. */ - onExited: PropTypes.func, + onExited: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` property instead.'), /** * Callback fired when the component is exiting. */ - onExiting: PropTypes.func, + onExiting: deprecatedPropType(PropTypes.func, 'Use the `TransitionProps` property 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..5e1b4b6e9dd4e5 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,166 @@ describe('', () => { expect(transitionHandler.callCount).to.equal(1); }); }); + + describe('prop: onEnter', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onEnter: () => [], + }, + 'props', + 'Popover', + ); + + // expect(console.error.callCount).to.equal(1); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed props type: The props `onEnter` of `Popover` is deprecated. Use the `TransitionProps` property instead.', + ); + }); + }); + + describe('prop: onEntering', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onEntering: () => [], + }, + 'props', + 'Popover', + ); + + // expect(console.error.callCount).to.equal(1); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed props type: The props `onEntering` of `Popover` is deprecated. Use the `TransitionProps` property instead.', + ); + }); + }); + + describe('prop: onEntered', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onEntered: () => [], + }, + 'props', + 'Popover', + ); + + // expect(console.error.callCount).to.equal(1); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed props type: The props `onEntered` of `Popover` is deprecated. Use the `TransitionProps` property instead.', + ); + }); + }); + + describe('prop: onExit', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onExit: () => [], + }, + 'props', + 'Popover', + ); + + // expect(console.error.callCount).to.equal(1); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed props type: The props `onExit` of `Popover` is deprecated. Use the `TransitionProps` property instead.', + ); + }); + }); + + describe('prop: onExiting', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onExiting: () => [], + }, + 'props', + 'Popover', + ); + + // expect(console.error.callCount).to.equal(1); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed props type: The props `onExiting` of `Popover` is deprecated. Use the `TransitionProps` property instead.', + ); + }); + }); + + describe('prop: onExited', () => { + beforeEach(() => { + PropTypes.resetWarningCache(); + stub(console, 'error'); + }); + + afterEach(() => { + console.error.restore(); + }); + + it('issues a warning', () => { + PropTypes.checkPropTypes( + Popover.Naked.propTypes, + { + onExited: () => [], + }, + 'props', + 'Popover', + ); + + // expect(console.error.callCount).to.equal(1); + expect(console.error.firstCall.args[0]).to.equal( + 'Warning: Failed props type: The props `onExited` of `Popover` is deprecated. Use the `TransitionProps` property 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(' + , );