From 5e533b2bfa730afb9186843103136d6a53954c68 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 13 Aug 2020 20:13:00 +0100 Subject: [PATCH 1/4] [Popover] Remove transition onX props --- docs/pages/api-docs/popover.md | 6 --- .../pages/guides/migration-v4/migration-v4.md | 25 ++++++++++++ packages/material-ui/src/Popover/Popover.d.ts | 24 ----------- packages/material-ui/src/Popover/Popover.js | 40 +------------------ 4 files changed, 27 insertions(+), 68 deletions(-) diff --git a/docs/pages/api-docs/popover.md b/docs/pages/api-docs/popover.md index 71b12489cdac6c..f1b6a216f6f307 100644 --- a/docs/pages/api-docs/popover.md +++ b/docs/pages/api-docs/popover.md @@ -40,12 +40,6 @@ 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. The `reason` parameter can optionally be used to control the response to `onClose`. | -| 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. | | 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/docs/src/pages/guides/migration-v4/migration-v4.md b/docs/src/pages/guides/migration-v4/migration-v4.md index 3ede35d1cc163d..ede217e91187ad 100644 --- a/docs/src/pages/guides/migration-v4/migration-v4.md +++ b/docs/src/pages/guides/migration-v4/migration-v4.md @@ -234,6 +234,31 @@ This change affects almost all components where you're using the `component` pro + ``` + ### Popover + +- The onE\* transition props were removed. Use TransitionProps instead. + + ```diff + + + ``` + ### Rating - Rename `visuallyhidden` to `visuallyHidden` for consistency: diff --git a/packages/material-ui/src/Popover/Popover.d.ts b/packages/material-ui/src/Popover/Popover.d.ts index 759aa9c7bb6d4d..702faa5aa111fa 100644 --- a/packages/material-ui/src/Popover/Popover.d.ts +++ b/packages/material-ui/src/Popover/Popover.d.ts @@ -80,30 +80,6 @@ export interface PopoverProps */ marginThreshold?: number; onClose?: ModalProps['onClose']; - /** - * Callback fired before the component is entering. - */ - onEnter?: TransitionHandlerProps['onEnter']; - /** - * Callback fired when the component has entered. - */ - onEntered?: TransitionHandlerProps['onEntered']; - /** - * Callback fired when the component is entering. - */ - onEntering?: TransitionHandlerProps['onEntering']; - /** - * Callback fired before the component is exiting. - */ - onExit?: TransitionHandlerProps['onExit']; - /** - * Callback fired when the component has exited. - */ - onExited?: TransitionHandlerProps['onExited']; - /** - * Callback fired when the component is exiting. - */ - onExiting?: TransitionHandlerProps['onExiting']; /** * If `true`, the popover is visible. */ diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index d6a7cf8d9d5283..59947da08a2714 100644 --- a/packages/material-ui/src/Popover/Popover.js +++ b/packages/material-ui/src/Popover/Popover.js @@ -10,7 +10,6 @@ import clsx from 'clsx'; import debounce from '../utils/debounce'; import ownerDocument from '../utils/ownerDocument'; import ownerWindow from '../utils/ownerWindow'; -import createChainedFunction from '../utils/createChainedFunction'; import withStyles from '../styles/withStyles'; import Modal from '../Modal'; import Grow from '../Grow'; @@ -102,12 +101,6 @@ const Popover = React.forwardRef(function Popover(props, ref) { elevation = 8, getContentAnchorEl, marginThreshold = 16, - onEnter, - onEntered, - onEntering, - onExit, - onExited, - onExiting, open, PaperProps = {}, transformOrigin = { @@ -116,7 +109,7 @@ const Popover = React.forwardRef(function Popover(props, ref) { }, TransitionComponent = Grow, transitionDuration: transitionDurationProp = 'auto', - TransitionProps = {}, + TransitionProps: { onEntering, ...TransitionProps } = {}, ...other } = props; const paperRef = React.useRef(); @@ -397,14 +390,9 @@ const Popover = React.forwardRef(function Popover(props, ref) { Date: Thu, 13 Aug 2020 20:39:36 +0100 Subject: [PATCH 2/4] Update docs/src/pages/guides/migration-v4/migration-v4.md Co-authored-by: Olivier Tassinari --- docs/src/pages/guides/migration-v4/migration-v4.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/src/pages/guides/migration-v4/migration-v4.md b/docs/src/pages/guides/migration-v4/migration-v4.md index ede217e91187ad..e53453a8095875 100644 --- a/docs/src/pages/guides/migration-v4/migration-v4.md +++ b/docs/src/pages/guides/migration-v4/migration-v4.md @@ -246,8 +246,6 @@ This change affects almost all components where you're using the `component` pro - onExit={onEntered}, - onExited={onEntered}, - onExiting={onEntered} - /> - Date: Thu, 13 Aug 2020 23:16:12 +0100 Subject: [PATCH 3/4] Fix tests --- .../pages/guides/migration-v4/migration-v4.md | 2 - packages/material-ui/src/Menu/Menu.js | 2 +- packages/material-ui/src/Menu/Menu.test.js | 46 ++++++------ .../material-ui/src/Popover/Popover.test.js | 72 +++++-------------- .../material-ui/src/Select/Select.test.js | 2 +- 5 files changed, 44 insertions(+), 80 deletions(-) diff --git a/docs/src/pages/guides/migration-v4/migration-v4.md b/docs/src/pages/guides/migration-v4/migration-v4.md index e53453a8095875..751a7e4d8c8056 100644 --- a/docs/src/pages/guides/migration-v4/migration-v4.md +++ b/docs/src/pages/guides/migration-v4/migration-v4.md @@ -313,8 +313,6 @@ This change affects almost all components where you're using the `component` pro - onExit={onEntered}, - onExited={onEntered}, - onExiting={onEntered} - /> - ', () => { 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} />, @@ -67,14 +69,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 @@ -158,14 +162,14 @@ describe('', () => { expect(false).to.equal(menuEl.contains(document.activeElement)); }); - it('should call props.onEntering with element if exists', () => { + it('should call 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); }); @@ -173,13 +177,13 @@ describe('', () => { it('should call props.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.test.js b/packages/material-ui/src/Popover/Popover.test.js index 5fe8d1f2ecb53b..6969a928340cdb 100644 --- a/packages/material-ui/src/Popover/Popover.test.js +++ b/packages/material-ui/src/Popover/Popover.test.js @@ -212,7 +212,7 @@ describe('', () => { // transitions towards entered const wrapper = mount( - +
, ); @@ -268,7 +268,7 @@ describe('', () => { it('should set the inline styles for the enter phase', () => { const handleEntering = spy(); const wrapper = mount( - +
, ); @@ -332,9 +332,10 @@ 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(); + } }} >
@@ -471,9 +472,10 @@ 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(); + } }} >
@@ -515,9 +517,10 @@ 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: { @@ -568,7 +571,7 @@ describe('', () => { @@ -653,7 +656,7 @@ describe('', () => { @@ -775,7 +778,7 @@ describe('', () => { mount( @@ -810,45 +813,4 @@ describe('', () => { expect(wrapper.find(TransitionComponent).props().timeout).to.equal(undefined); }); }); - - describe('prop: TransitionProp', () => { - it('chains onEntering with the apparent onEntering prop', () => { - const apparentHandler = spy(); - const transitionHandler = spy(); - - mount( - -
- , - ); - - expect(apparentHandler.callCount).to.equal(1); - expect(transitionHandler.callCount).to.equal(1); - }); - - it('does not chain other transition callbacks with the apparent ones', () => { - const apparentHandler = spy(); - const transitionHandler = spy(); - const wrapper = mount( - -
- , - ); - - wrapper.setProps({ open: false }); - - expect(apparentHandler.callCount).to.equal(0); - expect(transitionHandler.callCount).to.equal(1); - }); - }); }); diff --git a/packages/material-ui/src/Select/Select.test.js b/packages/material-ui/src/Select/Select.test.js index d5842bf87c0b32..6f6facaee46411 100644 --- a/packages/material-ui/src/Select/Select.test.js +++ b/packages/material-ui/src/Select/Select.test.js @@ -578,7 +578,7 @@ describe(' + , ); From cba829873e35d5f98051521a306a26c841e8da99 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 13 Aug 2020 23:26:54 +0100 Subject: [PATCH 4/4] prettier --- packages/material-ui/src/Menu/Menu.test.js | 14 ++++++++++---- .../material-ui/src/Popover/Popover.test.js | 17 ++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/material-ui/src/Menu/Menu.test.js b/packages/material-ui/src/Menu/Menu.test.js index 0c3ad175868095..c91704f825d957 100644 --- a/packages/material-ui/src/Menu/Menu.test.js +++ b/packages/material-ui/src/Menu/Menu.test.js @@ -50,7 +50,7 @@ describe('', () => { expect(handleEntering.callCount).to.equal(1); expect(handleEntering.args[0].length).to.equal(2); done(); - } + }, }} {...defaultProps} />, @@ -78,7 +78,7 @@ describe('', () => { expect(handleExiting.callCount).to.equal(1); expect(handleExiting.args[0].length).to.equal(1); done(); - } + }, }} {...defaultProps} open @@ -164,7 +164,9 @@ describe('', () => { it('should call 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 }; @@ -177,7 +179,11 @@ describe('', () => { it('should call props.onEntering, disableAutoFocusItem', () => { const onEnteringSpy = spy(); const wrapper = mount( - , + , ); const popover = wrapper.find(Popover); diff --git a/packages/material-ui/src/Popover/Popover.test.js b/packages/material-ui/src/Popover/Popover.test.js index 6969a928340cdb..08a1ed4b8002fc 100644 --- a/packages/material-ui/src/Popover/Popover.test.js +++ b/packages/material-ui/src/Popover/Popover.test.js @@ -212,7 +212,7 @@ describe('', () => { // transitions towards entered const wrapper = mount( - +
, ); @@ -332,10 +332,11 @@ describe('', () => { anchorEl={anchorEl} anchorOrigin={anchorOrigin} transitionDuration={0} - TransitionProps={{ onEntered: () => { + TransitionProps={{ + onEntered: () => { popoverEl = document.querySelector('[data-mui-test="Popover"]'); resolve(); - } + }, }} >
@@ -472,10 +473,11 @@ describe('', () => { anchorPosition={anchorPosition} anchorOrigin={anchorOrigin} transitionDuration={0} - TransitionProps={{ onEntered: () => { + TransitionProps={{ + onEntered: () => { popoverEl = document.querySelector('[data-mui-test="Popover"]'); resolve(); - } + }, }} >
@@ -517,10 +519,11 @@ describe('', () => { {...defaultProps} anchorReference="none" transitionDuration={0} - TransitionProps={{ onEntered: () => { + TransitionProps={{ + onEntered: () => { popoverEl = document.querySelector('[data-mui-test="Popover"]'); resolve(); - } + }, }} PaperProps={{ style: {