site-kit-wp
site-kit-wp copied to clipboard
Improve UX/UI on AdSense notifications
Bug Description
While testing the AdSense alerts in #7559 I spotted two UX/UI improvements that we should fix.
- The AdSense logo and text is rather small and not consistant with other notifications.
- 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
- Set up AdSense using an account that has a server alert (oi.ie has two which you can use to test this)
- Go to the Dashboard.
- The AdSense alert will appear (you might have to close other notifications that appear first)
- See UX/UI observations above.
Screenshots
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.jspassing the classgooglesitekit-adsense-alertto theBannerNotificationcomponent. - Create a new scss file
assets/sass/components/adsense/_googlesitekit-adsense-alerts.scssoveriding 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
AC 🌶️
@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:
Alternatively the AdSense logo could be included as the BannerNotification image.
What do you think?
@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.
The key point that is in ticket description but not covered in the AC is:
- The AdSense logo and text is rather small and not consistant with other notifications.
@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.
I see @kuasha420, the ticket is about resizing this svg:
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 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 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 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.
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:
Which addresses the AC and original issue. Moving to IBR.
Thanks, @benbowler. IB ✔️