Skip to content

Commit

Permalink
[Popover] Add warning when non-ref-holding component is used in Paper
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon committed Apr 3, 2019
1 parent ae24eea commit f7d4cc3
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
7 changes: 5 additions & 2 deletions packages/material-ui/src/Popover/Popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import warning from 'warning';
import debounce from 'debounce'; // < 1kb payload overhead when lodash/debounce is > 3kb.
import EventListener from 'react-event-listener';
import clsx from 'clsx';
import { chainPropTypes } from '@material-ui/utils';
import { chainPropTypes, elementTypeAcceptingRef } from '@material-ui/utils';
import ownerDocument from '../utils/ownerDocument';
import ownerWindow from '../utils/ownerWindow';
import { createChainedFunction } from '../utils/helpers';
Expand Down Expand Up @@ -348,6 +348,7 @@ class Popover extends React.Component {
data-mui-test="Popover"
elevation={elevation}
ref={ref => {
// #StrictMode ready
this.paperRef = ReactDOM.findDOMNode(ref);
}}
{...PaperProps}
Expand Down Expand Up @@ -521,7 +522,9 @@ Popover.propTypes = {
/**
* Properties applied to the [`Paper`](/api/paper/) element.
*/
PaperProps: PropTypes.object,
PaperProps: PropTypes.shape({
component: elementTypeAcceptingRef,
}),
/**
* This is the point on the popover which
* will attach to the anchor's origin.
Expand Down
9 changes: 9 additions & 0 deletions packages/material-ui/src/Popover/Popover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,15 @@ describe('<Popover />', () => {
assert.include(consoleErrorMock.args()[0][0], 'It should be a HTMLElement instance');
});

it('warns if a component for the Paper is used that cant hold a ref', () => {
mount(<Popover {...defaultProps} PaperProps={{ component: () => <div />, elevation: 4 }} />);
assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.args()[0][0],
'Warning: Failed prop type: Invalid prop `PaperProps.component` supplied to `Popover`',
);
});

// it('should warn if anchorEl is not visible', () => {
// mount(<Popover open anchorEl={document.createElement('div')} />);
// assert.strictEqual(consoleErrorMock.callCount(), 1);
Expand Down
2 changes: 1 addition & 1 deletion pages/api/popover.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import Popover from '@material-ui/core/Popover';
| <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&nbsp;*</span> | <span class="prop-type">bool</span> | | If `true`, the popover is visible. |
| <span class="prop-name">PaperProps</span> | <span class="prop-type">object</span> | | Properties applied to the [`Paper`](/api/paper/) element. |
| <span class="prop-name">PaperProps</span> | <span class="prop-type">{ component?: element type }</span> | | Properties applied to the [`Paper`](/api/paper/) element. |
| <span class="prop-name">transformOrigin</span> | <span class="prop-type">{ horizontal: union:&nbsp;number&nbsp;&#124;<br>&nbsp;enum:&nbsp;'left'&nbsp;&#124;<br>&nbsp;'center'&nbsp;&#124;<br>&nbsp;'right'<br><br>, vertical: union:&nbsp;number&nbsp;&#124;<br>&nbsp;enum:&nbsp;'top'&nbsp;&#124;<br>&nbsp;'center'&nbsp;&#124;<br>&nbsp;'bottom'<br><br> }</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)]. |
| <span class="prop-name">TransitionComponent</span> | <span class="prop-type">elementType</span> | <span class="prop-default">Grow</span> | The component used for the transition. |
| <span class="prop-name">transitionDuration</span> | <span class="prop-type">union:&nbsp;number&nbsp;&#124;<br>&nbsp;{ enter?: number, exit?: number }&nbsp;&#124;<br>&nbsp;enum:&nbsp;'auto'<br><br></span> | <span class="prop-default">'auto'</span> | Set to 'auto' to automatically calculate transition time based on height. |
Expand Down

0 comments on commit f7d4cc3

Please sign in to comment.