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

[Tooltip] - add new prop: childrenTitle #44378

Open
yairEO opened this issue Nov 11, 2024 · 10 comments
Open

[Tooltip] - add new prop: childrenTitle #44378

yairEO opened this issue Nov 11, 2024 · 10 comments
Assignees
Labels
component: tooltip This is the name of the generic UI component, not the React module! new feature New feature or request port-to-base-ui PR to be included in the Base UI repository once the API changes are done

Comments

@yairEO
Copy link

yairEO commented Nov 11, 2024

Summary

It is helpful for the Tooltip component to optionally be able to use its children as the title.

I suggest a dedicated prop named childrenTitle (or maybe childrenAsTitle) which will be used if title prop is not provided/undefined/null.

Sometimes its not easy to use the same children also for the title attribute because it requires considerable code changes.

Imagine a component which returns some 10-20 lines of JSX. Now I want to wrap everything the component returns with a tooltip, because the element which rendered this component is truncating the content and I want the tooltip to show it as-it-is.

Such a scenario requires creating an extra component just so the title and the children could call it both, which can consume some unwanted effort.

Examples

No response

Motivation

No response

Search keywords: Tooltip, title, children

@yairEO yairEO added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 11, 2024
@zannager zannager added the component: tooltip This is the name of the generic UI component, not the React module! label Nov 11, 2024
@siriwatknp
Copy link
Member

Is this practical in terms of accessibility? Do you have any resource that we can look into?

@siriwatknp siriwatknp added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 12, 2024
@yairEO
Copy link
Author

yairEO commented Nov 12, 2024

@siriwatknp what do you mean accessibility? what resources? this suggestion is based on common-sense :)

Here's an example:

Assume the below <Foo/> component is heavily used in an app, lets say in table cells, and most or all places where this component is used are truncating their content, hence its likely <Foo/> cannot be fully-viewed by users.

Now we want to add a tooltip so every instance which implements <Foo/> automatically will have a fix for the above UX problem.

Now, assume we want to wrap everything within Foo with a Tooltip, we face a difficulty as to how should we re-structure the code so the content remains and also acts as the tooltip's title prop.

Most non-senior developers will struggle, waist time and most likely end up with some sort of an abomination where code is duplicated or just badly written.

const Foo = () => (
  <div>
    <h1 className="text-2xl font-bold">Main Title which can be very long</h1>
    <p className="text-gray-600">This is the subtitle text which can also be quite long</p>
  </div>
);

export default Foo;

Ideally, a single Boolean prop indicating to the Tooltip component to use its children as the title would save a lot of time:

const Foo = () => (
  <Tooltip childrenTitle>
    <div>
      <h1 className="text-2xl font-bold">Main Title which can be very long</h1>
      <p className="text-gray-600">This is the subtitle text which can also be quite long</p>
    </div>
  </Tooltip>
);

export default Foo;

Even better would be to have this the default behavior if no title prop exists at all:

<Tooltip>
   ...
</Tooltip>

This requires eliminating scenarios where the title is used but have an undefined or null value, which is a clear indication of the wish of the developers not to render a tooltip

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Nov 12, 2024
@siriwatknp
Copy link
Member

siriwatknp what do you mean accessibility?

I meant the screen reader.

Also, based on your example, it does not make much sense to me. You'd use a Tooltip when you want to displays information about the element but if your content is already showing those content, the Tooltip sounds excessive to me.

The title is required to be able to pass the correct aria-label and aria-labelledby to the children.

@siriwatknp
Copy link
Member

May be an opinion from Base UI team would confirm about your issue cc @michaldudak @mj12albert

@siriwatknp siriwatknp added status: waiting for author Issue with insufficient information 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 12, 2024
@yairEO
Copy link
Author

yairEO commented Nov 12, 2024

but if your content is already showing those content, the Tooltip sounds excessive to me.

As I've mentioned in my previous message, Foo's content is not fully visible because it depends where does is rendered in.

In this scenario, its rendered within table cells which truncates the content, hence <Foo/> is not fully visible.

Also, this is an extremely easy feature to implement.
I assume writing tests & documentation for it takes double than actually implementing it 😉

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Nov 12, 2024
@michaldudak
Copy link
Member

If I understand the problem correctly, we have a similar issue in Base UI: mui/base-ui#523, but with no ETA at the moment.

@siriwatknp
Copy link
Member

As I've mentioned in my previous message, Foo's content is not fully visible because it depends where does is rendered in.

In this scenario, its rendered within table cells which truncates the content, hence <Foo/> is not fully visible.

I see.

@siriwatknp siriwatknp added port-to-base-ui PR to be included in the Base UI repository once the API changes are done and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 13, 2024
@yairEO
Copy link
Author

yairEO commented Nov 13, 2024

@michaldudak - its not similar at all... I only gave an example of one scenario out of who knows how many.. BTW, I already created a variant Tooltip that perfectly achieves the aim of mui/base-ui#523. But this is really not related to what I am suggesting here.

My suggestion is extremely simple and easy to implement. a no-brainer.

@mnajdova mnajdova added this to the Material UI with Base UI milestone Nov 14, 2024
@michaldudak
Copy link
Member

Tooltips are often placed on interactive elements, such as icon buttons. Having the prop you're proposing enabled in such cases doesn't make sense. Since your case is so simple to implement, I'd suggest implementing it in a wrapper component in your application code instead of adding it to the library.

@yairEO
Copy link
Author

yairEO commented Nov 15, 2024

@michaldudak - a wrapper sounds logical to me.

tooltips are used in SaaS products for complex content rather than simply wrapping "icons and buttons".

That my be their "traditional" use-cases for simple websites and low-level work, but for those who build complex systems, tooltips are everywhere and can be anything. Can get VERY complex in all sorts of ways.

I get it that most people don't get to work on complex things and are using probably using MUI as-is for their websites and simple needs, and the few who does need more complexity can just create wrappers around MUI and enhance them, and only if that's not easily possible, suggest direct change to MUI team.
That is a good philosophy which I can understand and agree with. Makes sense.

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 port-to-base-ui PR to be included in the Base UI repository once the API changes are done
Projects
None yet
Development

No branches or pull requests

6 participants