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

Add tooltip on hover of color presets #62201

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

amitraj2203
Copy link
Contributor

What?

Fix: #61743

Why?

To improve the user experience by displaying the title of each color preset when hovering over them in the site editor's global styles palette just like it happens with the theme colors.

How?

This PR adds tooltip to the color presets in the site editor's global styles palette to display the title of each preset when hovered over.

Testing Instructions

  1. Go to site editor > global styles > colors > palette
  2. Hover over a preset

Screenshots or screencast

Screen.Recording.2024-06-01.at.8.54.30.PM.mov

Copy link

github-actions bot commented Jun 1, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @sahiladit.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: sahiladit.

Co-authored-by: amitraj2203 <amitraj2203@git.wordpress.org>
Co-authored-by: scruffian <scruffian@git.wordpress.org>
Co-authored-by: bgardner <bgardner@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@amitraj2203 amitraj2203 added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Jun 12, 2024
@ndiego ndiego requested a review from scruffian June 19, 2024 13:17
@ndiego ndiego added [Type] Enhancement A suggestion for improvement. and removed [Type] Bug An existing feature does not function as intended labels Jun 19, 2024
{ () => <StylesPreviewColors /> }
</Variation>
<Tooltip key={ index } text={ variation?.title }>
<div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this extra div?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @scruffian, thanks for your review.

I tried it without the div, but it seems that the Tooltip doesn't seem to work then.

@scruffian
Copy link
Contributor

It would be good to do this for typography variations too, though happy for that to be a separate PR

@amitraj2203
Copy link
Contributor Author

It would be good to do this for typography variations too, though happy for that to be a separate PR

Thanks for suggestion. Addressed this in a separate PR.

@bgardner
Copy link

Confirming this works as expected and is a nice touch!

Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame that we need this extra div but I can't find a way round it.

Co-authored-by: Ben Dwyer <ben@scruffian.com>
@carolinan
Copy link
Contributor

Can you please include testing instructions and post the test results for users of screen readers and keyboard users.

@carolinan
Copy link
Contributor

Also does this affect the color palette presets in every part of the UI; or only in the Styles sidebar as in the clip?

@carolinan carolinan added the Needs Testing Needs further testing to be confirmed. label Jun 21, 2024
@amitraj2203
Copy link
Contributor Author

amitraj2203 commented Jun 21, 2024

Hi @scruffian & @carolinan, because of the extra div the focus is not working properly on the palletes. It requires two tab button clicks to focus the palette.
Can we move the Tooltip inside <Variation> that way it doesn't requies extra div and also with this change the tooltip will work on all the three - Styles, palettes and Typography. We can remove it specifically from Styles but still wants your confirmation.

This is how it's working now:

Screen.Recording.2024-06-21.at.1.23.19.PM.mov

And if we move the Tooltip inside <Variation> then this is how it will work:

Screen.Recording.2024-06-21.at.1.29.01.PM.mov

@scruffian
Copy link
Contributor

Can we move the Tooltip inside that way it doesn't requies extra div and also with this change the tooltip will work on all the three - Styles, palettes and Typography. We can remove it specifically from Styles but still wants your confirmation

I had considered this too, I think we should try it.

@amitraj2203
Copy link
Contributor Author

I had considered this too, I think we should try it.

Updated it. Here's a demo,

Screen.Recording.2024-06-21.at.3.14.57.PM.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Testing Needs further testing to be confirmed. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tooltip to hover for color presets
5 participants