Skip to content

Commit

Permalink
[Tooltip] Improve docs and warning for custom children (#22775)
Browse files Browse the repository at this point in the history
Co-authored-by: Matt <github@nospam.33m.co>
Co-authored-by: brorlarsnicklas <nicsi059@student.liu.se>
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
  • Loading branch information
4 people committed Oct 5, 2020
1 parent c01ce01 commit fc37528
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 8 deletions.
10 changes: 5 additions & 5 deletions docs/src/pages/guides/composition/composition.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,18 +168,18 @@ React in your console similar to:

> Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?
Be aware that you will still get this warning for `lazy` and `memo` components if their
wrapped component can't hold a ref.

Note that you will still get this warning for `lazy` and `memo` components if their wrapped component can't hold a ref.
In some instances an additional warning is issued to help with debugging, similar to:

> Invalid prop `component` supplied to `ComponentName`. Expected an element type that can hold a ref.
Only the two most common use cases are covered. For more information see [this section in the official React docs](https://reactjs.org/docs/forwarding-refs.html).

```diff
-const MyButton = props => <div role="button" {...props} />;
+const MyButton = React.forwardRef((props, ref) => <div role="button" {...props} ref={ref} />);
-const MyButton = () => <div role="button" />;
+const MyButton = React.forwardRef((props, ref) =>
+ <div role="button" {...props} ref={ref} />);

<Button component={MyButton} />;
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('<SpeedDialAction />', () => {
inheritComponent: Tooltip,
mount,
refInstanceof: window.HTMLButtonElement,
skip: ['componentProp'],
skip: ['componentProp', 'reactTestRenderer'],
}),
);

Expand Down
16 changes: 16 additions & 0 deletions packages/material-ui/src/Tooltip/Tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,22 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
ref: handleRef,
};

if (process.env.NODE_ENV !== 'production') {
childrenProps['data-mui-internal-clone-element'] = true;

// eslint-disable-next-line react-hooks/rules-of-hooks
React.useEffect(() => {
if (childNode && !childNode.getAttribute('data-mui-internal-clone-element')) {
console.error(
[
'Material-UI: The `children` component of the Tooltip is not forwarding its props correctly.',
'Please make sure that props are spread on the same element that the ref is applied to.',
].join('\n'),
);
}
}, [childNode]);
}

const interactiveWrapperListeners = {};

if (!disableTouchListener) {
Expand Down
19 changes: 17 additions & 2 deletions packages/material-ui/src/Tooltip/Tooltip.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -878,9 +878,10 @@ describe('<Tooltip />', () => {
// Tooltip should not assume that event handlers of children are attached to the
// outermost host
const TextField = React.forwardRef(function TextField(props, ref) {
const { onFocus, ...other } = props;
return (
<div ref={ref}>
<input type="text" {...props} />
<div ref={ref} {...other}>
<input type="text" onFocus={onFocus} />
</div>
);
});
Expand Down Expand Up @@ -917,6 +918,20 @@ describe('<Tooltip />', () => {
'Material-UI: A component is changing the uncontrolled open state of Tooltip to be controlled.',
);
});

it('should warn when not forwarding props', () => {
const BrokenButton = React.forwardRef((props, ref) => <button ref={ref}>Hello World</button>);

expect(() => {
render(
<Tooltip title="Hello World">
<BrokenButton />
</Tooltip>,
);
}).toErrorDev(
'The `children` component of the Tooltip is not forwarding its props correctly.',
);
});
});

it('should use the same popper.js instance between two renders', () => {
Expand Down

0 comments on commit fc37528

Please sign in to comment.