Studio: Add missing tooltip labels to buttons #648

ivan-ottinger · 2024-11-08T15:38:48Z

Fixes https://github.com/Automattic/dotcom-forge/issues/6623.

Proposed Changes

add new tooltip labels for the following buttons: Open settings, Start site, Starting & Stop site, "site screenshot" and "copy to clipboard" buttons
update aria-label for the following buttons: Account → Open settings; Profile link → Edit profile & Help → Get help

Start site	Stop site

Open settings	Site screenshot

Toggle sidebar	Copy to clipboard

Testing Instructions

Check out the PR and build the Studio app.
Navigate through the app and hover over the buttons where the tooltip labels have been added.
Tooltip labels should display correctly.

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors?

… button

src/components/user-settings.tsx

katinthehatsite

The changes look good to me and they worked as expected.

One thing is that I am not sure whether we should also add a tooltip for toggling the sidebar? What do you think?

Another small thing that I noticed is that when the site is starting (e.g. the green indicator is blinking), the tooltip still says Start site. Should we remove when this process is happening or display "Starting" instead?

wojtekn · 2024-11-11T11:12:27Z

In addition to the sidebar toggle mentioned by Kat, I would propose to add a few more:

full WP Admin and site URLs when the user hovers over 'WP admin', 'Open site', or site screenshot
'Copy' when a user hovers over links that can be copied to the clipboard in the Settings tab
settings link displayed for a user who is logged out

Also, do you think we should add tooltips for items that are already somehow descriptive e.g. tabs, buttons like 'Add site' or 'Export entire site' etc.?

…settings`

ivan-ottinger · 2024-11-11T15:58:17Z

Thank you for your review and feedback, @katinthehatsite and @wojtekn! 🙂

One thing is that I am not sure whether we should also add a tooltip for toggling the sidebar? What do you think?

Good point! I have added tooltip there as well.

Another small thing that I noticed is that when the site is starting (e.g. the green indicator is blinking), the tooltip still says Start site. Should we remove when this process is happening or display "Starting" instead?

Nice catch! I have added Starting tooltip when the site is starting.

full WP Admin and site URLs when the user hovers over 'WP admin', 'Open site', or site screenshot

Done:

'Copy' when a user hovers over links that can be copied to the clipboard in the Settings tab

Done:

(example showing tooltip of one of the buttons)

settings link displayed for a user who is logged out

Are you referring to the following Log in button?

If so, do you have any suggestions of the specific tooltip we could use? All the ideas coming to my mind do not seem right there - since the button already has the Log in label.

I would personally leave this button as-is for now - unless we come up with some specific tooltip text that fits there well. 🤔

Also, do you think we should add tooltips for items that are already somehow descriptive e.g. tabs, buttons like 'Add site' or 'Export entire site' etc.?

I would prefer if we keep the buttons that are descriptive enough without a tooltip. I think adding extra tooltips could make the interface too cluttered. But I am happy to discuss further.

wojtekn · 2024-11-12T08:15:51Z

If so, do you have any suggestions of the specific tooltip we could use? All the ideas coming to my mind do not seem right there - since the button already has the Log in label.

Currently, it says Log in, but it opens Settings window that let users log in. What if we added a tooltip for this one saying, 'Open settings' or 'Open settings to log in'?

I would prefer if we keep the buttons that are descriptive enough without a tooltip. I think adding extra tooltips could make the interface too cluttered. But I am happy to discuss further.

Sounds fair, let's leave those as is for now.

wojtekn

In general looks good, I spot one issue related to double labels.

wojtekn · 2024-11-12T08:20:41Z

src/components/header.tsx

 							disabled={ ! site.running }
 							className="[&.is-link]:text-a8c-gray-70 [&.is-link]:hover:text-a8c-blueberry !px-0 h-0 leading-4"
 							onClick={ () => getIpcApi().openSiteURL( site.id, '', { autoLogin: false } ) }
+							label={ sprintf(


Should we reuse those labels for tooltip content? I've tried VoiceOver with this PR, and in those cases, when the item is active, it reads both labels one by one:

That's a great catch, Wojtek! Thanks.

Turns out the issue is present there with existing buttons as well, e.g. More options or Close:

The root cause is in the way the Button, DropdownMenu and WPModal components (defined in @wordpress/components) are built: As soon as the label property is passed to them (which is used to trigger the Tooltip), these components add aria-label automatically as well. Because of that, VoiceOver reads both labels instead of just one.

I have tried a couple different solutions how to address this issue. The one that seems to be the cleanest is to rely on our custom Tooltip component instead. In that case we wouldn't rely on label prop which will prevent duplicate announcements in VoiceOver and other assistive technology apps.

I have applied the changes in the latest commit (333b600) so you can take a closer look.

However, further workarounds would be needed to address similar existing issue with the DropdownMenu and WPModal components that handle More options and Close buttons.

Other option could be to leave things as they are for now and accept the duplication and the way the WordPress components are built.

What do you think?

Just noting that I have made a couple more changes to the branch in the meantime.

Looks good, thanks for explaining the root case. I think we can leave the rest of components as is for now.

…omponent

wojtekn · 2024-11-13T07:43:48Z

Looks good. I've only notice small bug, where Studio displays tooltip for stopped site:

katinthehatsite · 2024-11-13T08:35:21Z

Everything looks good to me, nice work and congrats on your first Studio PR 😍

Just to note that it also happens for the Open site as well:

ivan-ottinger · 2024-11-13T09:14:15Z

Perfect! Thank you for catching that Wojtek and Kat!

I have addressed the issue in 1fcd845.

Now when the site URL is not available, the tooltips will have the following texts: Open WP admin and Open site.

katinthehatsite

Nice the changes look good to me now, thanks for moving the logic for the URL to a separate constant. It makes it much easier to read 👍

Nice work!!

wojtekn · 2024-11-13T10:22:28Z

@ivan-ottinger, It seems that after we Start and Stop the site, those two links still show the URL in the tooltip. Could we remove tooltip for those two when site is stopped, so it would be consistent with thumbnail behavior?

We will ultimately fix it when we implement https://github.com/Automattic/dotcom-forge/issues/9139/

ivan-ottinger · 2024-11-13T11:07:55Z

Nice the changes look good to me now, thanks for moving the logic for the URL to a separate constant. It makes it much easier to read 👍

Nice work!!

Thank you, Kat!

@ivan-ottinger, It seems that after we Start and Stop the site, those two links still show the URL in the tooltip. Could we remove tooltip for those two when site is stopped, so it would be consistent with thumbnail behavior?

We will ultimately fix it when we implement https://github.com/Automattic/dotcom-forge/issues/9139

Sounds good! I have applied the change in dbf2937. I have also kept the fallback tooltips there in case the site.url isn't available for some reason.

wojtekn · 2024-11-14T09:51:04Z

@ivan-ottinger I gave it a final test, and the labels look good now. However, I noticed the glitch with the help icon label:

it's misaligned vertically
when tooltip is shown, window gets scrollbar
when I move cursor right, it quickly changes between both states

Screen.Capture.on.2024-11-14.at.10-47-30.mp4

…` buttons

ivan-ottinger · 2024-11-14T15:12:54Z

@ivan-ottinger I gave it a final test, and the labels look good now. However, I noticed the glitch with the help icon label:

it's misaligned vertically

when tooltip is shown, window gets scrollbar

when I move cursor right, it quickly changes between both states

Screen.Capture.on.2024-11-14.at.10-47-30.mp4

Thanks for catching that, Wojtek. I did not run into the same issue in my tests before but can definitely reproduce it now after resizing the app window.

Our custom Tooltip component is relying on an inline wrapper div that is causing styling conflicts in certain situations. I have now also noticed a related tiny misalignment on the Edit profile button in the Settings section.

In our 1:1 we were discussing the possibility of scaling down the scope of this PR and not adding the tooltip to buttons where there's some issue.

Other option could be to add the tooltip to Get help and Edit profile buttons as well, but rely on the title attribute there instead:

→ I have made the change in the last commit (3114ccd) if you would like to try it out.

It does not look that fancy as our custom tooltip (or the other tooltip component included in WordPress), but it is more compatible and works well with accessibility tools.

I would be even willing to replace all tooltips we have with the title attribute until we create a tooltip solution that is compatible with all use cases.

Regardless, we can also remove tooltips from Get help and Edit profile buttons completely for now and leave only the tooltips that work well.

Do you have any preference here? 🤔

wojtekn · 2024-11-15T08:46:41Z

@ivan-ottinger thanks for exploring it more. I think it would be better to skip the problematic tooltip at this point instead of having two different types of tooltip.

I noticed one more case. Should we delete those for now, too?

ivan-ottinger · 2024-11-15T09:19:41Z

@ivan-ottinger thanks for exploring it more. I think it would be better to skip the problematic tooltip at this point instead of having two different types of tooltip.

I noticed one more case. Should we delete those for now, too?

Thanks, Wojtek. Sounds good to me. 👍🏼 I have removed the tooltips that were causing issues and updated the PR description accordingly.

ivan-ottinger · 2024-11-15T13:14:08Z

Just noting that I have created a separate task to add the remaining tooltips: https://github.com/Automattic/dotcom-forge/issues/9841.

ivan-ottinger added 7 commits November 8, 2024 16:25

Add Edit Profile button tooltip label

bb8eafa

Update aria-label of the Edit Profile button

2246bd3

Add Open Settings button tooltip label

9a0c413

Update aria-label of the Open Settings button

a986cde

Update aria-label of the Get help button

97c4840

Add Get help button tooltip label

2f5bd45

Add Stop site / Start site tooltip label to the Stop / Start site…

b73d29f

… button

ivan-ottinger self-assigned this Nov 8, 2024

ivan-ottinger added 2 commits November 8, 2024 16:56

Fix unit tests

cc7f35f

Fix linting

033dc7e

ivan-ottinger requested a review from a team November 8, 2024 16:18

katinthehatsite reviewed Nov 11, 2024

View reviewed changes

src/components/user-settings.tsx Outdated Show resolved Hide resolved

katinthehatsite reviewed Nov 11, 2024

View reviewed changes

ivan-ottinger added 11 commits November 11, 2024 15:53

Change Edit Profile to Edit profile and Open Settings to `Open …

791c4b0

…settings`

Add Toggle sidebar tooltip

47d8583

Add Starting tooltip text when a site is starting

8debf03

Add tooltip handling to the CopyTextButton component

f4b76c1

Add tooltips to "Copy to clipboard" buttons on the site Settings page

a833c94

Add tooltips to WP admin and Open site links

a1aaf56

Add tooltip to the site screenshot on the Overview tab

5081bbb

Fix linting errors

6388a10

Fix translators comments

ebfcb29

Update "Copy to clipboard" tooltip logic

92a323c

Fix unit tests

c913ba0

wojtekn reviewed Nov 12, 2024

View reviewed changes

ivan-ottinger added 2 commits November 12, 2024 10:00

Add tooltip to the Log in button when logged out

108abae

Use custom Tooltip component

333b600

ivan-ottinger added 3 commits November 12, 2024 14:48

Fix linting errors

172c198

Restore unintentionaly removed showTooltip prop from the Button c…

ba4bd1a

…omponent

Refactor Button component return value

ebf326c

Handle header tooltip texts for stopped sites

1fcd845

katinthehatsite approved these changes Nov 13, 2024

View reviewed changes

Apply tooltip to Header links only when the site is running

dbf2937

Use title instead of tooltipText for Get help and `Edit profile…

3114ccd

…` buttons

ivan-ottinger added 4 commits November 15, 2024 09:51

Remove Edit profile tooltip

4713347

Remove Get help tooltip

fe706dc

Remove tooltips from Header links

9f74b5c

Remove url from StoppedSiteDetails interface

0688a7c

wojtekn approved these changes Nov 15, 2024

View reviewed changes

ivan-ottinger merged commit a3be065 into trunk Nov 15, 2024
14 checks passed

ivan-ottinger deleted the add/missing-button-labels branch November 15, 2024 12:52

This was referenced Nov 18, 2024

Studio: Improve offline mode for Sync tab #666

Merged

Studio: Offline icon in header is mis-aligned #668

Closed

Studio: Fix Toggle sidebar tooltip #669

Merged

Studio: Add missing tooltip labels to buttons #648

Studio: Add missing tooltip labels to buttons #648

Uh oh!

Conversation

ivan-ottinger commented Nov 8, 2024 • edited by wojtekn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Testing Instructions

Pre-merge Checklist

Uh oh!

Uh oh!

katinthehatsite left a comment

Choose a reason for hiding this comment

Uh oh!

wojtekn commented Nov 11, 2024

Uh oh!

ivan-ottinger commented Nov 11, 2024

Uh oh!

wojtekn commented Nov 12, 2024

Uh oh!

wojtekn left a comment

Choose a reason for hiding this comment

Uh oh!

wojtekn Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

ivan-ottinger Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

ivan-ottinger Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

wojtekn Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

wojtekn commented Nov 13, 2024

Uh oh!

katinthehatsite commented Nov 13, 2024

Uh oh!

ivan-ottinger commented Nov 13, 2024

Uh oh!

katinthehatsite left a comment

Choose a reason for hiding this comment

Uh oh!

wojtekn commented Nov 13, 2024

Uh oh!

ivan-ottinger commented Nov 13, 2024

Uh oh!

wojtekn commented Nov 14, 2024

Uh oh!

ivan-ottinger commented Nov 14, 2024

Uh oh!

wojtekn commented Nov 15, 2024

Uh oh!

ivan-ottinger commented Nov 15, 2024

Uh oh!

Uh oh!

ivan-ottinger commented Nov 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ivan-ottinger commented Nov 8, 2024 •

edited by wojtekn

Loading