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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tooltip] Don't show if title is undefined, null or false #29696

Closed
2 tasks done
YuriGor opened this issue Nov 16, 2021 · 4 comments 路 Fixed by #34289
Closed
2 tasks done

[Tooltip] Don't show if title is undefined, null or false #29696

YuriGor opened this issue Nov 16, 2021 · 4 comments 路 Fixed by #34289
Labels
component: tooltip This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@YuriGor
Copy link

YuriGor commented Nov 16, 2021

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 馃挕

Tooltip shouldn't be shown if title property is undefined, null or boolean false.
Currently only explicit empty string works and for null / undefined / false empty screwed tooltip is still show,

Examples 馃寛

https://codesandbox.io/s/basictooltip-material-demo-forked-zy0vq?file=/demo.js

image

Motivation 馃敠

It's a common approach in React to not render undefined / null / false element, same expected from tooltip.

@YuriGor YuriGor added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 16, 2021
@YuriGor YuriGor changed the title Don't show tooltip if title is undefined or null Don't show tooltip if title is undefined, null or false Nov 16, 2021
@michaldudak
Copy link
Member

It makes sense. However, there could be applications that rely on the existing behavior, so I'd propose waiting for v6 with such a change.

A workaround, for now, would be to fall back to an empty string if a value is nullish or false:

function BetterTooltip(props)  {
    return <Tooltip {...props} title={(props.title != null && props.title !== false) ? props.title : '')}  />
}

@michaldudak michaldudak added new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 16, 2021
@michaldudak michaldudak added this to the v6 milestone Nov 16, 2021
@michaldudak michaldudak added the component: tooltip This is the name of the generic UI component, not the React module! label Nov 16, 2021
@michaldudak michaldudak changed the title Don't show tooltip if title is undefined, null or false [Tooltip] Don't show if title is undefined, null or false Nov 16, 2021
@YuriGor
Copy link
Author

YuriGor commented Nov 16, 2021

Hi @michaldudak I can't imagine how any application could use such a weird empty rudiment of tooltip as you see on screenshot.

@michaldudak
Copy link
Member

Perhaps not exactly what's on the screenshot, but custom styled version. Your proposal, while being perfectly valid, touches the public API that should not change between major versions.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 30, 2022

This issue feels almost like a duplicate of #9714. On that older issue, the argument was made that not accepting, null, undefined, or false as valid title prop values would be preferable because it would force developers to be explicit about how they want to handle edge cases. e.g is the value undefined because the translation key for the title is missing? Since the tooltip title requires interaction to be shown, it's hard to spot for developers without the current invalid prop warning logic.

Now, if this issue is about keeping the prop-type error and keeping the TypeScript failure when a non-displayable title is provided, then 馃憤. Not showing the invalid title would lead to a better fallback UX for end-users and keep a good DX for developers when the title value goes wrong (can be identified before going to production).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tooltip This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
4 participants