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 ReactNode as title in Tooltip #3460

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

magnh
Copy link
Contributor

@magnh magnh commented May 23, 2024

What does this pull request change?

Add ReactNode to the title prop in the Tooltip component.

Why is this pull request needed?

The title is used directly in the JSX and passing custom components as title seems to work.

Not sure this is intended, up to the EDS team.

@magnh magnh self-assigned this May 23, 2024
@oddvernes
Copy link
Collaborator

I have to check a bit more if this should be allowed. This might get vetoed by our designers since tooltip should be strictly for short feedback etc and to use Popover for more complex formatted output

@magnh magnh force-pushed the chore/allow-react-node-title-in-tooltip branch from f41a5f7 to d275688 Compare May 23, 2024 14:04
@yusijs
Copy link
Contributor

yusijs commented May 23, 2024

Just to chime in:
We have often also wanted to have a bit more comples values in the tooltip. Supersimple example is just being able to make part of the text cursive/bold, e.g "You should not do xyz". Another example is just to have an extra line to improve readability by splitting the text naturally. We often fall back to the native title-attribute here, since it adheres to \n for linebreaks (in some browser/os-combos at least)

This has been requested by our designers several times, where the popover is far to intrusive but the tooltip is to limited.

@oddvernes
Copy link
Collaborator

This is great feedback and I definetly agree πŸ‘

@oddvernes
Copy link
Collaborator

Our guidelines HARD disagree on this πŸ˜‚
But who wrote this and why? we will probably never know 🀷
bilde

@torleifhalseth torleifhalseth removed their request for review May 27, 2024 09:24
@yusijs
Copy link
Contributor

yusijs commented May 27, 2024

We actually discussed this with a former EDS designer (Lucas) who said that it should be treated as guidelines and not rules. In this case it seems like it's expected as a rule. Imo, the result of such restrictions doesnt lead to a better / more consistent design across Equinor, but rather the opposite, since those who have the requirement will just roll their own variants.

I would be interested getting some perspectives from the EDS designers (@BirteThornquist / @benteljones?) on why this rule is in place, and their thoughts on the points I outlined above.

@benteljones
Copy link
Contributor

We're going to go through all the old guidelines and create new examples. Several of them are outdated. Although we want the tooltip to be primarily for short, one-line information, we need to facilitate a slightly more adapted context. As you point out @yusijs , it will not lead to a more consistent use, but rather the opposite if the restrictions don't cover what the users need.
We will take these needs with us into the work on an updated 2.0 component. Thank you for the important feedback. 🌸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants