site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Published 20 hours ago • verified publisher icon google

Minor UX bug on AdSense Settings When Publisher ID loading

Open wpdarren opened this issue 2 years ago • 3 comments

Bug Description

Minor UX bug when you go to AdSense settings. If you click on the Check your site status link while the Publisher ID is loading (this can take a couple of seconds), the settings page reloads rather than to AdSense sites page. It's only when the Publisher ID has loaded that the link works as expected.

Not a huge issue, but wondering if it's possible to improve the UX here?

image.png

Steps to reproduce

  1. Go to 'Site Kit Settings'
  2. Click on 'AdSense'
  3. Click the 'Check your site status' straightaway and before Publisher ID loads.
  4. See error

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The link to Check your site status should be unclickable until the URL for it is available (initially it is undefined but this isn't accounted for)

Implementation Brief

  • In assets/js/modules/adsense/components/settings/SettingsView.js:
    • If siteStatusURL is undefined, add the 'disabled' prop to the <Link> component so it is not clickable until it is loaded.

Test Coverage

  • No changes expected

QA Brief

Changelog entry

wpdarren avatar Jun 22 '22 10:06 wpdarren

@aaemnnosttv The AC suggests rendering the <Link> but making it "Visually Hidden". Does this mean wrapping the entire component within the <VisuallyHidden> component? I feel this bug can be resolved easily if we just disable the link until the URL loads which is what I have suggested in the IB for now. In my testing, the loading is super quick on most sites. Or should we just show a loading bar for the whole component like we do for other settings views? This will avoid any colour changes or link appearing flickers?

jimmymadon avatar Aug 06 '22 11:08 jimmymadon

IB ✔️

eugene-manuilov avatar Aug 08 '22 12:08 eugene-manuilov

The AC suggests rendering the <Link> but making it "Visually Hidden". Does this mean wrapping the entire component within the <VisuallyHidden> component? I feel this bug can be resolved easily if we just disable the link until the URL loads which is what I have suggested in the IB for now. In my testing, the loading is super quick on most sites. Or should we just show a loading bar for the whole component like we do for other settings views? This will avoid any colour changes or link appearing flickers?

@jimmymadon VisuallyHidden is for screen readers so that wouldn't work as was intended here. The idea was to use visibility: hidden to hide the link until it is ready but also to not create a shift in the process by not rendering it at all. Using disabled should work well here. I forgot that our Link component has that. Using disabled and removing that later is more consistent with other usage we have so +1 for that. I'll update the AC.

Edit: please also remember to populate the test coverage section, even when there are no changes to make there 🙂

aaemnnosttv avatar Aug 09 '22 02:08 aaemnnosttv

QA Update: ✅

Verified:

  • The link is 'disabled'/'unclickable' when viewing AdSense settings while the Publisher ID is loading.
  • Ran through a smoke test various links within the plugin - especially links with the "external" image/icon.

https://user-images.githubusercontent.com/73545194/189014803-bbd66921-195f-4e25-af90-bf79080abbab.mp4

wpdarren avatar Sep 08 '22 01:09 wpdarren