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

[Tooltip] Convert to function component #15291

Merged
merged 12 commits into from
Apr 15, 2019
Merged

[Tooltip] Convert to function component #15291

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2de6ce0
[Tooltip] Convert to function component
joshwooding Apr 7, 2019
3df2d60
Remove unwrap usage
joshwooding Apr 9, 2019
ba36c8b
Remove ref function
joshwooding Apr 9, 2019
514b322
I think I've done this right
joshwooding Apr 9, 2019
52f8ff6
Add forceUpdate on children mount
joshwooding Apr 9, 2019
4fcfd13
Code review
joshwooding Apr 10, 2019
bd66482
Code review
joshwooding Apr 12, 2019
345458b
Remove mountRef
joshwooding Apr 12, 2019
6626831
Fix mistake
joshwooding Apr 14, 2019
f74dcbf
Re-order
joshwooding Apr 14, 2019
541fc2f
Boolean fun
joshwooding Apr 14, 2019
f62e92a
Merge branch 'next' into tooltip-function-component
oliviertassinari Apr 15, 2019
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
31 changes: 16 additions & 15 deletions packages/material-ui/src/Tooltip/Tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,22 @@ function Tooltip(props) {
const handleRef = useForkRef(children.ref, handleOwnRef);

React.useEffect(() => {
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
if (childNode) {
warning(
!childNode.disabled ||
(childNode.disabled && isControlled) ||
(childNode.disabled && title === '') ||
childNode.tagName.toLowerCase() !== 'button',
[
'Material-UI: you are providing a disabled `button` child to the Tooltip component.',
'A disabled element does not fire events.',
"Tooltip needs to listen to the child element's events to display the title.",
'',
'Place a `div` container on top of the element.',
].join('\n'),
);
}
warning(
!(
Copy link
Member

@oliviertassinari oliviertassinari Apr 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 That's is, maybe we can write our own warning method that uses warn internally but reverse the condition logic. It's more intuitive. Maybe we don't need a different if branch. What we need is a direct condition logic, not a reversed one.

childNode &&
childNode.disabled &&
!isControlled &&
title !== '' &&
childNode.tagName.toLowerCase() === 'button'
),
[
'Material-UI: you are providing a disabled `button` child to the Tooltip component.',
'A disabled element does not fire events.',
"Tooltip needs to listen to the child element's events to display the title.",
'',
'Place a `div` container on top of the element.',
].join('\n'),
);
}, [isControlled, title, childNode]);

React.useEffect(() => {
Expand Down