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] Add undefined, null or false in title #34289

Merged
merged 8 commits into from Sep 26, 2022
Merged

[Tooltip] Add undefined, null or false in title #34289

merged 8 commits into from Sep 26, 2022

Conversation

abhinav-22-tech
Copy link
Contributor

@abhinav-22-tech abhinav-22-tech commented Sep 12, 2022 •
edited

Closes: #29696

@mui-bot
Copy link

mui-bot commented Sep 12, 2022 •
edited

Details of bundle changes

Generated by 🚫 dangerJS against 5cb8a7a

@abhinav-22-tech
Copy link
Contributor Author

abhinav-22-tech commented Sep 12, 2022 •
edited

@siriwatknp
On the !title empty string, false and null work fine but for undefined it is not.
Demo: https://codesandbox.io/s/gracious-blackburn-p9j2f2?file=/demo.tsx

@abhinav-22-tech abhinav-22-tech marked this pull request as ready for review September 12, 2022 14:44
@michaldudak michaldudak added new feature New feature or request component: tooltip This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Sep 15, 2022
siriwatknp
siriwatknp reviewed Sep 16, 2022
View reviewed changes
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

@abhinav-22-tech One last comment, can you add tests to check the false, null, and undefined

@abhinav-22-tech abhinav-22-tech marked this pull request as draft September 17, 2022 09:40
@abhinav-22-tech
Copy link
Contributor Author

@siriwatknp I am confused why it shows
Warning: Failed prop type: The prop `title` is marked as required in `ForwardRef(Tooltip)`, but its value is `undefined`.

I think it's due to open. Could you please help me to figure this out?

@abhinav-22-tech @heyfirst
Update packages/mui-material/src/Tooltip/Tooltip.js
f21c0d9 
Co-authored-by: First Sutham <kanisorns.k@gmail.com>
Signed-off-by: Abhinav Jain <66718489+abhinav-22-tech@users.noreply.github.com>
@abhinav-22-tech
Copy link
Contributor Author

Thanks @heyfirst

@abhinav-22-tech abhinav-22-tech marked this pull request as ready for review September 20, 2022 05:46
siriwatknp
siriwatknp approved these changes Sep 20, 2022
View reviewed changes
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

Thanks, both 👍 @abhinav-22-tech @heyfirst. Removed the NonNullable<> and tts ignore. I think this is good to go!

@siriwatknp siriwatknp merged commit 44642d2 into mui:master Sep 26, 2022
alexfauquette pushed a commit to alexfauquette/material-ui that referenced this pull request Oct 14, 2022
@abhinav-22-tech @alexfauquette
[Tooltip] Add undefined, null or false in title (mui#34289)
2d7b909
@oliviertassinari oliviertassinari mentioned this pull request Oct 22, 2022
Merged
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
@abhinav-22-tech
[Tooltip] Add undefined, null or false in title (mui#34289)
1da4a2b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari left review comments

@heyfirst heyfirst heyfirst left review comments

@siriwatknp siriwatknp siriwatknp approved these changes

@michaldudak michaldudak Awaiting requested review from michaldudak

Assignees
No one assigned
Labels
component: tooltip This is the name of the generic UI component, not the React module! new feature New feature or request package: material-ui Specific to @mui/material
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Tooltip] Don't show if title is undefined, null or false
6 participants
@abhinav-22-tech @mui-bot @oliviertassinari @michaldudak @heyfirst @siriwatknp