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.

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

[Popper] Fix anchorEl prop types #16004

Merged
merged 5 commits into from
Jun 8, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
6 changes: 2 additions & 4 deletions docs/src/pages/components/popper/SimplePopper.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const useStyles = makeStyles(theme => ({
},
}));

function SimplePopper() {
export default function SimplePopper() {
const classes = useStyles();
const [anchorEl, setAnchorEl] = React.useState(null);

Expand All @@ -21,7 +21,7 @@ function SimplePopper() {
}

const open = Boolean(anchorEl);
const id = open ? 'simple-popper' : null;
const id = open ? 'simple-popper' : undefined;

return (
<div>
Expand All @@ -40,5 +40,3 @@ function SimplePopper() {
</div>
);
}

export default SimplePopper;
44 changes: 44 additions & 0 deletions docs/src/pages/components/popper/SimplePopper.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React from 'react';
import { makeStyles, Theme, createStyles } from '@material-ui/core/styles';
import Popper from '@material-ui/core/Popper';
import Typography from '@material-ui/core/Typography';
import Button from '@material-ui/core/Button';
import Fade from '@material-ui/core/Fade';
import Paper from '@material-ui/core/Paper';

const useStyles = makeStyles((theme: Theme) =>
createStyles({
typography: {
padding: theme.spacing(2),
},
}),
);

export default function SimplePopper() {
const classes = useStyles();
const [anchorEl, setAnchorEl] = React.useState<null | HTMLElement>(null);

function handleClick(event: React.MouseEvent<HTMLElement>) {
setAnchorEl(anchorEl ? null : event.currentTarget);
}

const open = Boolean(anchorEl);
const id = open ? 'simple-popper' : undefined;

return (
<div>
<Button aria-describedby={id} variant="contained" onClick={handleClick}>
Toggle Popper
</Button>
<Popper id={id} open={open} anchorEl={anchorEl} transition>
{({ TransitionProps }) => (
<Fade {...TransitionProps} timeout={350}>
<Paper>
<Typography className={classes.typography}>The content of the Popper.</Typography>
</Paper>
</Fade>
)}
</Popper>
</div>
);
}
2 changes: 1 addition & 1 deletion docs/src/pages/components/popper/popper.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Some important features of the `Popper` component:

- 🕷 Popper relies on the 3rd party library ([Popper.js](https://github.com/FezVrasta/popper.js)) for perfect positioning.
- 💄 It's an alternative API to react-popper. It aims for simplicity.
- 📦 [10 kB gzipped](/size-snapshot).
- 📦 [10 kB gzipped](/size-snapshot) (7 kB from Popper.js).
- The children is [`Portal`](/components/portal/) to the body of the document to avoid rendering problems.
You can disable this behavior with `disablePortal`.
- The scroll isn't blocked like with the [`Popover`](/components/popover/) component.
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Popper/Popper.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export type PopperPlacementType =

export interface PopperProps extends React.HTMLAttributes<HTMLDivElement> {
transition?: boolean;
anchorEl?: null | Element | ReferenceObject | (() => Element);
anchorEl?: null | ReferenceObject | (() => ReferenceObject);
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
children:
| React.ReactNode
| ((props: {
Expand Down
47 changes: 28 additions & 19 deletions packages/material-ui/src/Popper/Popper.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,37 +165,46 @@ const Popper = React.forwardRef(function Popper(props, ref) {

Popper.propTypes = {
/**
* This is the DOM element, or a function that returns the DOM element,
* This is the reference element, or a function that returns the reference element,
* that may be used to set the position of the popover.
* The return value will passed as the reference object of the Popper
* instance.
*
* The reference element should be an HTML Element instance or a referenceObject:
* https://popper.js.org/popper-documentation.html#referenceObject.
*/
anchorEl: chainPropTypes(PropTypes.oneOfType([PropTypes.object, PropTypes.func]), props => {
if (props.open) {
const resolvedAnchorEl = getAnchorEl(props.anchorEl);

if (resolvedAnchorEl instanceof Element) {
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
const box = resolvedAnchorEl.getBoundingClientRect();
if (
!resolvedAnchorEl ||
typeof resolvedAnchorEl.clientWidth !== 'number' ||
typeof resolvedAnchorEl.clientHeight !== 'number' ||
typeof resolvedAnchorEl.getBoundingClientRect !== 'function'
) {
return new Error(
[
'Material-UI: the `anchorEl` prop provided to the component is invalid.',
'It should be an HTML Element instance or a referenceObject:',
'https://popper.js.org/popper-documentation.html#referenceObject.',
].join('\n'),
);
}

const box = resolvedAnchorEl.getBoundingClientRect();

if (
process.env.NODE_ENV !== 'test' &&
box.top === 0 &&
box.left === 0 &&
box.right === 0 &&
box.bottom === 0
) {
return new Error(
[
'Material-UI: the `anchorEl` prop provided to the component is invalid.',
'The node element should be visible.',
].join('\n'),
);
}
} else {
if (
process.env.NODE_ENV !== 'test' &&
box.top === 0 &&
box.left === 0 &&
box.right === 0 &&
box.bottom === 0
) {
return new Error(
[
'Material-UI: the `anchorEl` prop provided to the component is invalid.',
`It should be an Element instance but it's \`${resolvedAnchorEl}\` instead.`,
'The reference element should be visible.',
].join('\n'),
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Popper/Popper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ describe('<Popper />', () => {
it('should warn if anchorEl is not valid', () => {
mount(<Popper {...defaultProps} open anchorEl={null} />);
assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(consoleErrorMock.args()[0][0], 'It should be an Element instance');
assert.include(consoleErrorMock.args()[0][0], 'It should be an HTML Element instance');
});

// it('should warn if anchorEl is not visible', () => {
Expand Down
2 changes: 1 addition & 1 deletion pages/api/popper.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Poppers rely on the 3rd party library [Popper.js](https://github.com/FezVrasta/p

| Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| <span class="prop-name">anchorEl</span> | <span class="prop-type">union:&nbsp;object&nbsp;&#124;<br>&nbsp;func<br></span> | | This is the DOM element, or a function that returns the DOM element, that may be used to set the position of the popover. The return value will passed as the reference object of the Popper instance. |
| <span class="prop-name">anchorEl</span> | <span class="prop-type">union:&nbsp;object&nbsp;&#124;<br>&nbsp;func<br></span> | | This is the reference element, or a function that returns the reference element, that may be used to set the position of the popover. The return value will passed as the reference object of the Popper instance.<br>The reference element should be an HTML Element instance or a referenceObject: https://popper.js.org/popper-documentation.html#referenceObject. |
| <span class="prop-name required">children&nbsp;*</span> | <span class="prop-type">union:&nbsp;node&nbsp;&#124;<br>&nbsp;func<br></span> | | Popper render function or node. |
| <span class="prop-name">container</span> | <span class="prop-type">union:&nbsp;object&nbsp;&#124;<br>&nbsp;func<br></span> | | A node, component instance, or function that returns either. The `container` will passed to the Modal component. By default, it uses the body of the anchorEl's top-level document object, so it's simply `document.body` most of the time. |
| <span class="prop-name">disablePortal</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Disable the portal behavior. The children stay within it's parent DOM hierarchy. |
Expand Down