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

Allow Tooltip to have string as child element #2724

Closed
eirikbacker opened this issue Nov 4, 2024 · 11 comments · Fixed by #2777
Closed

Allow Tooltip to have string as child element #2724

eirikbacker opened this issue Nov 4, 2024 · 11 comments · Fixed by #2777

Comments

@eirikbacker
Copy link
Contributor

No description provided.

@eirikbacker eirikbacker converted this from a draft issue Nov 4, 2024
@Barsnes
Copy link
Member

Barsnes commented Nov 13, 2024

We would need to add a wrapper element for our consumers in this case.
With our current implementation we need a ref to the element the tooltip should attach to. We could allow this:

<Tooltip>min streng</Tooltip>

but in that case, we would need to wrap it in something like a span. I believe this was discussed when the tooltip was first implemented, but we decided not to do this, as we want the consumer to make the choice on element here.

This is somewhat related to #2451.

I have started a PR for the issue above, and this can be lifted as a possible change there: #2777

@mimarz
Copy link
Collaborator

mimarz commented Nov 14, 2024

Whats the use-case for having it support just string?

Is the idea that they then attach the tooltip to an element using their own ref instead of us expecting children to be an element that supports ref?

@eirikbacker
Copy link
Contributor Author

Whats the use-case for having it support just string?

The string use case is how Mattilsynet is using Tooltip in most situations: we have so many complicated terms, or terms that can be interpreted in different ways, or used in different was across different countries, but we need to use them due to legal regulations. In these scenarios, we use tooltip like:

Please contact the <Tooltip content="…">freight forwarder</Tooltip> for information about the <Tooltip content="…">export certificate</Tooltip>.

@mimarz
Copy link
Collaborator

mimarz commented Nov 14, 2024

Whats the use-case for having it support just string?

The string use case is how Mattilsynet is using Tooltip in most situations: we have so many complicated terms, or terms that can be interpreted in different ways, or used in different was across different countries, but we need to use them due to legal regulations. In these scenarios, we use tooltip like:

Please contact the <Tooltip content="…">freight forwarder</Tooltip> for information about the <Tooltip content="…">export certificate</Tooltip>.

Wouldn't you want some styling to indicate there is a tooltip here? What if someone wants to use a strong element instead of span?

Do you suggest we support string in addition to element as children, so check on the children type?

@Barsnes
Copy link
Member

Barsnes commented Nov 14, 2024

Whats the use-case for having it support just string?

The string use case is how Mattilsynet is using Tooltip in most situations: we have so many complicated terms, or terms that can be interpreted in different ways, or used in different was across different countries, but we need to use them due to legal regulations. In these scenarios, we use tooltip like:

Please contact the <Tooltip content="…">freight forwarder</Tooltip> for information about the <Tooltip content="…">export certificate</Tooltip>.

Wouldn't you want some styling to indicate there is a tooltip here? What if someone wants to use a strong element instead of span?

Do you suggest we support string in addition to element as children, so check on the children type?

This is exactly how it has been implemented in #2777. If you send a string, it is wrapped in a span. If you send an element, that element is used

@eirikbacker
Copy link
Contributor Author

yes and yes 🙈 this is how @Barsnes has implemented, and we might want to add some default styling - good point. Dashed underline is a common convention, and design should probably have an opinion here, but bottom line is - I think it is very good if we support a default solution for Tooltip around text/string ☺️

@Barsnes
Copy link
Member

Barsnes commented Nov 14, 2024

yes and yes 🙈 this is how @Barsnes has implemented, and we might want to add some default styling - good point. Dashed underline is a common convention, and design should probably have an opinion here, but bottom line is - I think it is very good if we support a default solution for Tooltip around text/string ☺️

Yeah design can always come later, but it would also only be relevant if they don't send an element.

I think guidelines is the way to go for styling

@mimarz
Copy link
Collaborator

mimarz commented Nov 14, 2024

Yeah, a dashed underline also came to mind 😄

As long as users stil have an option to also define the element and alternative styling it should be good 👍

@mimarz
Copy link
Collaborator

mimarz commented Nov 14, 2024

Do we have an issue for this on design?

@eirikbacker
Copy link
Contributor Author

Do we have an issue for this on design?

Now we do :p #2781
But I suggest we can move ahead and merge this before it is solved in design? ☺️

@Barsnes
Copy link
Member

Barsnes commented Nov 14, 2024

Do we have an issue for this on design?

Now we do :p #2781 But I suggest we can move ahead and merge this before it is solved in design? ☺️

I agree, no need to be blocked by something that is easy to implement later on

@github-project-automation github-project-automation bot moved this from 📄 Todo to ✅ Done in Team Design System Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants