-
Notifications
You must be signed in to change notification settings - Fork 13
docs(tooltips): update ux guidelines #1105
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
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Documentation. Coverage Reports: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@spike-rabbit @dauriamarco Please review as well 🙏
And, I think we should do this for v48 as it's requested by project(s). If something would require a breaking change, then we should do it for v49 but I don't think that'd be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @panch1739 for the update and for the research you did on the topic 🚀
Replying to @kfenner as well:
I’m currently updating my old tooltip PR #778, also as a follow-up to the discussion with Maxi here which should be directly related to this topic.
That PR will very likely be breaking, since the input triggers will be completely removed in order to make the tooltip compliant with the new guidelines.
I’ll keep you posted once the update is ready 👍
UPDATE: I've opened a new pull request to address the alignment: #1245
ec97969 to
30066e0
Compare
This PR updates the tooltip docs based on the latest Element decisions.
@kfenner This should only be merged when we implement the tooltips coherently in our components. Im unsure if we plan to do it for V49...change it if is not the case (L)