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

Improve UX/UI on AdSense notifications

Open wpdarren opened this issue 2 years ago • 11 comments
trafficstars

Bug Description

While testing the AdSense alerts in #7559 I spotted two UX/UI improvements that we should fix.

  1. The AdSense logo and text is rather small and not consistant with other notifications.
  2. The SVG does not align very well with the text and CTA.

Would be good to tidy up these when we can.

Steps to reproduce

  1. Set up AdSense using an account that has a server alert (oi.ie has two which you can use to test this)
  2. Go to the Dashboard.
  3. The AdSense alert will appear (you might have to close other notifications that appear first)
  4. See UX/UI observations above.

Screenshots

image

Additional Context

This issue appears on all supported browsers.


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

Acceptance criteria

  • The SVG Icon that appears in the AdSense alert should be reduced in size to align with the text and CTA on its left, and have the unnecessary white spaces around it reduced.

Implementation Brief

  • Update assets/js/components/notifications/AdSenseAlerts.js passing the class googlesitekit-adsense-alert to the BannerNotification component.
  • Create a new scss file assets/sass/components/adsense/_googlesitekit-adsense-alerts.scss overiding the following notification styles for the AdSense notifications:
	.googlesitekit-adsense-alert {

		&.googlesitekit-publisher-win .googlesitekit-publisher-win__module-name {
			font-size: $fs-title-md;
		}

		.googlesitekit-publisher-win__image-small svg {
			width: 68px;
		}
	}

Test Coverage

  • Update VRTs. No further testing required.

QA Brief

Changelog entry

wpdarren avatar Sep 29 '23 04:09 wpdarren

AC 🌶️

eugene-manuilov avatar Feb 21 '24 14:02 eugene-manuilov

@eugene-manuilov @wpdarren I've been looking at this between other tickets and reviewing other banners in storybook and Figma. I don't think the AC matches the original ticket description which talks about increasing the font size of the banner title not reducing the icon size. To make it fully consistent with other site banners I think the AdSense logo should be removed completely and the banner be rephrased a little, this is what I think it should look like to match other banners using the BannerNotification component:

Screenshot 2024-03-05 at 09 35 01

Alternatively the AdSense logo could be included as the BannerNotification image.

What do you think?

benbowler avatar Mar 05 '24 09:03 benbowler

@benbowler i think the original idea for this task is not to make that banner look like other banners, but to fix style issues of that banner. But I can't say that what you suggest is wrong or bad. We need to ask @aaemnnosttv if we want to make those changes.

eugene-manuilov avatar Mar 05 '24 13:03 eugene-manuilov

The key point that is in ticket description but not covered in the AC is:

  1. The AdSense logo and text is rather small and not consistant with other notifications.

benbowler avatar Mar 05 '24 13:03 benbowler

@benbowler we discussed this in one of our ac sync. the plan here is to get a quick win by ensuring this is not ao ugly anymore. Feel free to file a follow up issue to suggest improvement.

kuasha420 avatar Mar 05 '24 13:03 kuasha420

I see @kuasha420, the ticket is about resizing this svg:

271471934-f9266e56-686f-48c5-b0c5-2c880344db2b

I read the AC as reducing the AdSense logo SVG which is already too small IMO. I will do a limited IB to reduce this SVGs dimensions.

benbowler avatar Mar 06 '24 11:03 benbowler

@benbowler There are two issues with this notification. 1) The AdSense title, i.e logo, and text are small. 2) the SVG you highlighted above must be smaller and more aligned with the copy and CTA. Generally, it needs tidying up because it looks a bit all over the place at the moment 😄

wpdarren avatar Mar 06 '24 11:03 wpdarren

@wpdarren do you have a storybook story or Figma design for another similar banner that I could use as the basis for choosing a font size/style for the title?

benbowler avatar Mar 06 '24 11:03 benbowler

@benbowler These notifications appear from the AdSense API and are a little different than normal notifications, so I am not sure we have anything to compare with. My thoughts are that we should chat with Sigal about this in the UX/UI channel and get her opinion on how she would like it to look.

wpdarren avatar Mar 06 '24 11:03 wpdarren

I started a low priority thread in Slack about refactoring these in future, if there is a response there we can create new tickets. In the short term I've created an IB that updates notifications to look like this:

Screenshot 2024-03-18 at 13 51 04

Which addresses the AC and original issue. Moving to IBR.

benbowler avatar Mar 18 '24 13:03 benbowler

Thanks, @benbowler. IB ✔️

eugene-manuilov avatar Mar 18 '24 16:03 eugene-manuilov