Skip to content

Commit

Permalink
[Popover] Remove transition onX props (#22184)
Browse files Browse the repository at this point in the history
  • Loading branch information
mbrookes authored Aug 14, 2020
1 parent 862fade commit 183f267
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 146 deletions.
6 changes: 0 additions & 6 deletions docs/pages/api-docs/popover.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ The `MuiPopover` name can be used for providing [default props](/customization/g
| <span class="prop-name">getContentAnchorEl</span> | <span class="prop-type">func</span> | | 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. |
| <span class="prop-name">marginThreshold</span> | <span class="prop-type">number</span> | <span class="prop-default">16</span> | Specifies how close to the edge of the window the popover can appear. |
| <span class="prop-name">onClose</span> | <span class="prop-type">func</span> | | Callback fired when the component requests to be closed. The `reason` parameter can optionally be used to control the response to `onClose`. |
| <span class="prop-name">onEnter</span> | <span class="prop-type">func</span> | | Callback fired before the component is entering. |
| <span class="prop-name">onEntered</span> | <span class="prop-type">func</span> | | Callback fired when the component has entered. |
| <span class="prop-name">onEntering</span> | <span class="prop-type">func</span> | | Callback fired when the component is entering. |
| <span class="prop-name">onExit</span> | <span class="prop-type">func</span> | | Callback fired before the component is exiting. |
| <span class="prop-name">onExited</span> | <span class="prop-type">func</span> | | Callback fired when the component has exited. |
| <span class="prop-name">onExiting</span> | <span class="prop-type">func</span> | | Callback fired when the component is exiting. |
| <span class="prop-name required">open<abbr title="required">*</abbr></span> | <span class="prop-type">bool</span> | | If `true`, the popover is visible. |
| <span class="prop-name">PaperProps</span> | <span class="prop-type">{ component?: element type }</span> | <span class="prop-default">{}</span> | Props applied to the [`Paper`](/api/paper/) element. |
| <span class="prop-name">transformOrigin</span> | <span class="prop-type">{ horizontal: 'center'<br>&#124;&nbsp;'left'<br>&#124;&nbsp;'right'<br>&#124;&nbsp;number, vertical: 'bottom'<br>&#124;&nbsp;'center'<br>&#124;&nbsp;'top'<br>&#124;&nbsp;number }</span> | <span class="prop-default">{ vertical: 'top', horizontal: 'left',}</span> | This is the point on the popover which will attach to the anchor's origin.<br>Options: vertical: [top, center, bottom, x(px)]; horizontal: [left, center, right, x(px)]. |
Expand Down
23 changes: 23 additions & 0 deletions docs/src/pages/guides/migration-v4/migration-v4.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,29 @@ This change affects almost all components where you're using the `component` pro
+<PaginationItem shape="circular">
```

### Popover

- The onE\* transition props were removed. Use TransitionProps instead.

```diff
<Popover
- onEnter={onEnter}
- onEntered={onEntered},
- onEntering={onEntered},
- onExit={onEntered},
- onExited={onEntered},
- onExiting={onEntered}
+ TransitionProps={{
+ onEnter,
+ onEntered,
+ onEntering,
+ onExit,
+ onExited,
+ onExiting,
+ }}
/>
```

### Rating

- Rename `visuallyhidden` to `visuallyHidden` for consistency:
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Menu/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,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={{
Expand Down
52 changes: 31 additions & 21 deletions packages/material-ui/src/Menu/Menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,16 @@ describe('<Menu />', () => {

const wrapper = mount(
<Menu
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();
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}
/>,
Expand All @@ -67,14 +69,16 @@ describe('<Menu />', () => {

const wrapper = mount(
<Menu
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();
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
Expand Down Expand Up @@ -158,28 +162,34 @@ describe('<Menu />', () => {
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(<Menu {...defaultProps} onEntering={onEnteringSpy} />);
const wrapper = mount(
<Menu {...defaultProps} TransitionProps={{ onEntering: onEnteringSpy }} />,
);
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', () => {
const onEnteringSpy = spy();
const wrapper = mount(
<Menu disableAutoFocusItem {...defaultProps} onEntering={onEnteringSpy} />,
<Menu
disableAutoFocusItem
{...defaultProps}
TransitionProps={{ onEntering: onEnteringSpy }}
/>,
);
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);
});
Expand Down
24 changes: 0 additions & 24 deletions packages/material-ui/src/Popover/Popover.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
40 changes: 2 additions & 38 deletions packages/material-ui/src/Popover/Popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 = {
Expand All @@ -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();
Expand Down Expand Up @@ -397,14 +390,9 @@ const Popover = React.forwardRef(function Popover(props, ref) {
<TransitionComponent
appear
in={open}
onEnter={onEnter}
onEntered={onEntered}
onExit={onExit}
onExited={onExited}
onExiting={onExiting}
timeout={transitionDuration}
onEntering={handleEntering}
{...TransitionProps}
onEntering={createChainedFunction(handleEntering, TransitionProps.onEntering)}
>
<Paper
data-mui-test="Popover"
Expand Down Expand Up @@ -546,30 +534,6 @@ Popover.propTypes = {
* The `reason` parameter can optionally be used to control the response to `onClose`.
*/
onClose: PropTypes.func,
/**
* Callback fired before the component is entering.
*/
onEnter: PropTypes.func,
/**
* Callback fired when the component has entered.
*/
onEntered: PropTypes.func,
/**
* Callback fired when the component is entering.
*/
onEntering: PropTypes.func,
/**
* Callback fired before the component is exiting.
*/
onExit: PropTypes.func,
/**
* Callback fired when the component has exited.
*/
onExited: PropTypes.func,
/**
* Callback fired when the component is exiting.
*/
onExiting: PropTypes.func,
/**
* If `true`, the popover is visible.
*/
Expand Down
75 changes: 20 additions & 55 deletions packages/material-ui/src/Popover/Popover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ describe('<Popover />', () => {

// transitions towards entered
const wrapper = mount(
<Popover {...defaultProps} open transitionDuration={0} {...handlers}>
<Popover {...defaultProps} open transitionDuration={0} TransitionProps={{ ...handlers }}>
<div />
</Popover>,
);
Expand Down Expand Up @@ -268,7 +268,7 @@ describe('<Popover />', () => {
it('should set the inline styles for the enter phase', () => {
const handleEntering = spy();
const wrapper = mount(
<Popover {...defaultProps} onEntering={handleEntering}>
<Popover {...defaultProps} TransitionProps={{ onEntering: handleEntering }}>
<div />
</Popover>,
);
Expand Down Expand Up @@ -332,9 +332,11 @@ describe('<Popover />', () => {
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();
},
}}
>
<div />
Expand Down Expand Up @@ -471,9 +473,11 @@ describe('<Popover />', () => {
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();
},
}}
>
<div />
Expand Down Expand Up @@ -515,9 +519,11 @@ describe('<Popover />', () => {
{...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: {
Expand Down Expand Up @@ -568,7 +574,7 @@ describe('<Popover />', () => {
<Popover
anchorEl={mockedAnchor}
open
onEntering={handleEntering}
TransitionProps={{ onEntering: handleEntering }}
transitionDuration={0}
marginThreshold={8}
>
Expand Down Expand Up @@ -653,7 +659,7 @@ describe('<Popover />', () => {
<Popover
anchorEl={anchorEl}
open
onEntering={handleEntering}
TransitionProps={{ onEntering: handleEntering }}
marginThreshold={marginThreshold}
PaperProps={{ component: FakePaper }}
>
Expand Down Expand Up @@ -775,7 +781,7 @@ describe('<Popover />', () => {
mount(
<Popover
anchorEl={mockedAnchorEl}
onEntering={handleEntering}
TransitionProps={{ onEntering: handleEntering }}
getContentAnchorEl={getContentAnchorEl}
open
>
Expand Down Expand Up @@ -810,45 +816,4 @@ describe('<Popover />', () => {
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(
<Popover
{...defaultProps}
open
TransitionProps={{ onEntering: transitionHandler }}
onEntering={apparentHandler}
>
<div />
</Popover>,
);

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(
<Popover
{...defaultProps}
open
TransitionProps={{ onExiting: transitionHandler }}
onExiting={apparentHandler}
>
<div />
</Popover>,
);

wrapper.setProps({ open: false });

expect(apparentHandler.callCount).to.equal(0);
expect(transitionHandler.callCount).to.equal(1);
});
});
});
2 changes: 1 addition & 1 deletion packages/material-ui/src/Select/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ describe('<Select />', () => {
it('should apply additional props to the Menu component', () => {
const onEntered = spy();
const { getByRole } = render(
<Select MenuProps={{ onEntered, transitionDuration: 100 }} value="10">
<Select MenuProps={{ TransitionProps: { onEntered }, transitionDuration: 100 }} value="10">
<MenuItem value="10">Ten</MenuItem>
</Select>,
);
Expand Down

0 comments on commit 183f267

Please sign in to comment.