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 deep merge of PopperProps #19851

Merged
merged 8 commits into from
Feb 29, 2020
2 changes: 1 addition & 1 deletion docs/pages/api/tooltip.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi
| <span class="prop-name">onOpen</span> | <span class="prop-type">func</span> | | Callback fired when the component requests to be open.<br><br>**Signature:**<br>`function(event: object) => void`<br>*event:* The event source of the callback. |
| <span class="prop-name">open</span> | <span class="prop-type">bool</span> | | If `true`, the tooltip is shown. |
| <span class="prop-name">placement</span> | <span class="prop-type">'bottom-end'<br>&#124;&nbsp;'bottom-start'<br>&#124;&nbsp;'bottom'<br>&#124;&nbsp;'left-end'<br>&#124;&nbsp;'left-start'<br>&#124;&nbsp;'left'<br>&#124;&nbsp;'right-end'<br>&#124;&nbsp;'right-start'<br>&#124;&nbsp;'right'<br>&#124;&nbsp;'top-end'<br>&#124;&nbsp;'top-start'<br>&#124;&nbsp;'top'</span> | <span class="prop-default">'bottom'</span> | Tooltip placement. |
| <span class="prop-name">PopperProps</span> | <span class="prop-type">object</span> | | Props applied to the [`Popper`](/api/popper/) element. |
| <span class="prop-name">PopperProps</span> | <span class="prop-type">object</span> | <span class="prop-default">{}</span> | Props applied to the [`Popper`](/api/popper/) element. |
| <span class="prop-name required">title&nbsp;*</span> | <span class="prop-type">node</span> | | Tooltip title. Zero-length titles string are never displayed. |
| <span class="prop-name">TransitionComponent</span> | <span class="prop-type">elementType</span> | <span class="prop-default">Grow</span> | The component used for the transition. [Follow this guide](/components/transitions/#transitioncomponent-prop) to learn more about the requirements for this component. |
| <span class="prop-name">TransitionProps</span> | <span class="prop-type">object</span> | | Props applied to the [`Transition`](http://reactcommunity.org/react-transition-group/transition#Transition-props) element. |
Expand Down
30 changes: 16 additions & 14 deletions packages/material-ui/src/Tooltip/Tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react';
import * as ReactDOM from 'react-dom';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { elementAcceptingRef } from '@material-ui/utils';
import { deepmerge, elementAcceptingRef } from '@material-ui/utils';
import { fade } from '../styles/colorManipulator';
import withStyles from '../styles/withStyles';
import capitalize from '../utils/capitalize';
Expand Down Expand Up @@ -194,7 +194,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
onOpen,
open: openProp,
placement = 'bottom',
PopperProps,
PopperProps = {},
title,
TransitionComponent = Grow,
TransitionProps,
Expand Down Expand Up @@ -484,18 +484,21 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
}
}

// Avoid the creation of a new Popper.js instance at each render.
const popperOptions = React.useMemo(
() => ({
modifiers: {
arrow: {
enabled: Boolean(arrowRef),
element: arrowRef,
const mergedPopperProps = React.useMemo(() => {
return deepmerge(
{
popperOptions: {
modifiers: {
arrow: {
enabled: Boolean(arrowRef),
element: arrowRef,
},
},
},
},
}),
[arrowRef],
);
PopperProps,
);
}, [arrowRef, PopperProps]);

return (
<React.Fragment>
Expand All @@ -510,9 +513,8 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
open={childNode ? open : false}
id={childrenProps['aria-describedby']}
transition
popperOptions={popperOptions}
{...interactiveWrapperListeners}
{...PopperProps}
{...mergedPopperProps}
>
{({ placement: placementInner, TransitionProps: TransitionPropsInner }) => (
<TransitionComponent
Expand Down
32 changes: 32 additions & 0 deletions packages/material-ui/src/Tooltip/Tooltip.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,38 @@ describe('<Tooltip />', () => {
});
});

describe('prop: PopperProps', () => {
it('should pass PopperProps to Popper Component', () => {
const { getByTestId } = render(
<Tooltip {...defaultProps} open PopperProps={{ 'data-testid': 'popper' }} />,
);

expect(getByTestId('popper')).to.be.ok;
});

it('should merge popperOptions with arrow modifier', () => {
const popperRef = React.createRef();
render(
<Tooltip
{...defaultProps}
open
arrow
PopperProps={{
popperRef,
popperOptions: {
modifiers: {
arrow: {
foo: 'bar',
},
},
},
}}
/>,
);
expect(popperRef.current.modifiers.find(x => x.name === 'arrow').foo).to.equal('bar');
});
});

describe('forward', () => {
it('should forward props to the child element', () => {
const wrapper = mount(
Expand Down