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.

Sign up for GitHub

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

Jump to bottom

[Popover] Deprecate transition onX props #22202

Merged
merged 4 commits into from
Aug 15, 2020
Merged

[Popover] Deprecate transition onX props #22202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
edb369a
[Popover] Deprecate transition onX props
mbrookes Aug 14, 2020
db86857
property -> prop
mbrookes Aug 14, 2020
b1e9732
Fix tests
mbrookes Aug 15, 2020
9d64fe3
reduce test LOC
mbrookes Aug 15, 2020
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
12 changes: 6 additions & 6 deletions docs/pages/api-docs/popover.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ 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. |
| <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">onEnter</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired before the component is entering. |
| ~~<span class="prop-name">onEntered</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the component 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 component 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 component is exiting. |
| ~~<span class="prop-name">onExited</span>~~ | <span class="prop-type">func</span> | | *Deprecated*. Use the `TransitionProps` prop instead.<br><br>Callback fired when the component 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 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
2 changes: 1 addition & 1 deletion packages/material-ui/src/Menu/Menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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={{
Expand Down
54 changes: 32 additions & 22 deletions packages/material-ui/src/Menu/Menu.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,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 @@ -70,14 +72,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 @@ -161,28 +165,34 @@ describe('<Menu />', () => {
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(<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', () => {
it('should call TransitionProps.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
13 changes: 7 additions & 6 deletions packages/material-ui/src/Popover/Popover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
*/
Expand Down
162 changes: 148 additions & 14 deletions packages/material-ui/src/Popover/Popover.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,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 @@ -271,7 +271,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 @@ -335,9 +335,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 @@ -481,9 +483,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 @@ -525,9 +529,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 @@ -578,7 +584,7 @@ describe('<Popover />', () => {
<Popover
anchorEl={mockedAnchor}
open
onEntering={handleEntering}
TransitionProps={{ onEntering: handleEntering }}
transitionDuration={0}
marginThreshold={8}
>
Expand Down Expand Up @@ -663,7 +669,7 @@ describe('<Popover />', () => {
<Popover
anchorEl={anchorEl}
open
onEntering={handleEntering}
TransitionProps={{ onEntering: handleEntering }}
marginThreshold={marginThreshold}
PaperProps={{ component: FakePaper }}
>
Expand Down Expand Up @@ -785,7 +791,7 @@ describe('<Popover />', () => {
mount(
<Popover
anchorEl={mockedAnchorEl}
onEntering={handleEntering}
TransitionProps={{ onEntering: handleEntering }}
getContentAnchorEl={getContentAnchorEl}
open
>
Expand Down Expand Up @@ -822,6 +828,15 @@ describe('<Popover />', () => {
});

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();
Expand Down Expand Up @@ -861,4 +876,123 @@ describe('<Popover />', () => {
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.',
);
});
});
});
});
2 changes: 1 addition & 1 deletion packages/material-ui/src/Select/Select.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,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