google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 289 forks source link

Update the design of the Key Metrics setup CTA banner. #8895

Open techanvil opened 4 months ago

techanvil commented 4 months ago

Feature Description

The current Key Metrics setup CTA banner looks like this:

Screenshot 2024-07-25 at 19 20 32

But the dismiss CTA is in the bottom of the banner, which isn’t consistent with our new banners.

We have a new Figma design for the Key Metrics setup CTA banner:

image

This design should be applied to the banner in the plugin.


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

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

tofumatt commented 2 months ago

As mentioned in Slack, this needs mobile designs/guidance before we can move it to IB, so leaving it in AC for now.

tofumatt commented 2 months ago

Mobile designs are ready so moved this to IB 👍🏻

eugene-manuilov commented 2 months ago

IB ✔️

zutigrm commented 2 months ago

Parking it in the meantime until it gets a sprint number, since I picked it up by mistake

zutigrm commented 3 weeks ago

Hi @sigal-teller , since this banner has a bit longer content, and we aim to cover desktop layout for screens starting at 960px and over, currently it's existing desktop sizes are too crowded, causing the banner to grow in height, and hence the Graphic to look out of place as it doesn't fit correctly in that crowded space. At the moment it is showing tablet layout on these screens. Would it be possible to add new screen with adjusted desktop layout for 960px screen width?

Current desktop layout works fine for screens starting at 1280px and over so that style will remain for these screens, new adjustment would be only for specific screen sizes between 960px to bellow 1280px.

cc: @eugene-manuilov

sigal-teller commented 3 weeks ago

Hi @zutigrm I assume you meant a total screen width of 960px, which means that when deducting the WP bar, there is a net width of 800px for the dashboard. It is possible to have a version of the desktop design like here. In this width it looks better then the tablet design. By the way, why aren't we implementing the banner design that I created for ACR already? It is very similar only includes a slightly different illustration and additional explanatory text, which can actually fit for KMW as well. This new design for KMW was created before, but it was a long time ago, so Im wondering if we're implementing it now why not use the ACR banner which will replace it?

zutigrm commented 3 weeks ago

@sigal-teller Yes perfect, that's nice solution.

By the way, why aren't we implementing the banner design that I created for ACR already? It is very similar only includes a slightly different illustration and additional explanatory text

That's a good point, thanks. Since the text is not mentioning directly the ACR - because this banner will be shown to all users, not hidden behind feature flag, I will adjust the current implementation to include new graphic and text from the ACR figma file.

sigal-teller commented 3 weeks ago

@zutigrm ok great, so I'll create the design for this this width in the ACR Figma file as well

zutigrm commented 3 weeks ago

@sigal-teller awesome, thank you!

sigal-teller commented 3 weeks ago

@zutigrm Done.

mohitwp commented 4 days ago

QA Update ❌

@zutigrm

Issues -

  1. Additional space showing below CTA under viewports from 601px to 959px.
  2. Both KMW CTA and Analytics CTA are not showing correctly under tablets and mobile viewports.
  3. CTA's are not loading correctly.

Loading issue -

https://github.com/user-attachments/assets/d21c2285-f8e2-4bb2-902d-e7c4f094b77e https://github.com/user-attachments/assets/1ad52373-10be-4d21-ae19-f8609ff636fc

Design issues

![Image](https://github.com/user-attachments/assets/21dbfee2-ede8-4296-b18b-22ca665280bc) ![Image](https://github.com/user-attachments/assets/e8a4d2a7-c0ef-4efe-9239-c5a1ea45055b) ![Image](https://github.com/user-attachments/assets/75ba7d43-af3d-4407-ab45-37459af12bc0) ![Image](https://github.com/user-attachments/assets/bf91f186-704e-4e2d-b892-080821e626c6) ![Image](https://github.com/user-attachments/assets/b5cc8dd2-0b0f-4ffb-8203-368991e88aab) ![Image](https://github.com/user-attachments/assets/5f2f0a3a-9bdf-4ffa-9e69-783f9197809e) ![Image](https://github.com/user-attachments/assets/66b48d14-500e-471a-a1bb-1866ea24b709)
zutigrm commented 4 days ago

@mohitwp thanks for your observations. I updated the styles, regarding the spacing on tablet screens, there is an open issue for that #9106, I brought it up to the AC so we can work on it sooner. Couldn't include it as part of this issue, since it will needs some thoughts and digging during definition, since it is a grid style, affecting the overall display of KMW, but makes an empty space when banner is shown. There seems to be KMW that are just hidden with css, which might be causing this.

Regarding one screenshot - on 1024px for iPad PRO, I couldn't reproduce it on my end, but I decreased graphic a bit, let me know if it works on your end Image

mohitwp commented 2 days ago

QA Update ❌

@zutigrm I tested this on BrowserStack and found that the CTA banner image appears misaligned on Mac and iOS devices. However, it looks good on Windows and Android devices. The issue exists only on macOS and iPads across all browsers.

**Mac and iOS devices** ![Image](https://github.com/user-attachments/assets/c9aaa397-8c1d-49d6-b7ee-587d1e576155) ![Image](https://github.com/user-attachments/assets/e9694424-6732-4207-934b-8f00c5c01d84) ![Image](https://github.com/user-attachments/assets/0b1dd6dd-24a1-4cee-9494-9111b5680fad) ![Image](https://github.com/user-attachments/assets/8ba49c8b-288e-4d39-b585-79067755a48b)
zutigrm commented 2 days ago

@mohitwp Thanks, graphic has been fixed for mac OS and iOS

mohitwp commented 1 day ago

QA Update ✅

![Image](https://github.com/user-attachments/assets/374af761-cc11-4d6f-9717-cfeee383cf6c) ![Image](https://github.com/user-attachments/assets/22afa2ed-a846-4f77-9f50-b73f2918ca48) ![Image](https://github.com/user-attachments/assets/a37523d8-9830-4797-81a3-6246ee0c6da8) ![Image](https://github.com/user-attachments/assets/0ca3c878-3a3a-4450-a5fb-c547cc6f2687) ![Image](https://github.com/user-attachments/assets/53d9f5da-71e1-47ec-a441-9b03c7eefc95)

When 4 metrics are selected and analytics gets disconnect

![Image](https://github.com/user-attachments/assets/ca9d57b5-a508-41c0-bfee-179cd47265ac) ![Image](https://github.com/user-attachments/assets/1a40aca2-014b-434a-898e-49627d16e4cd) ![Image](https://github.com/user-attachments/assets/9732c2ca-03ad-4ab3-bd39-a2091ab5f8c1) ![Image](https://github.com/user-attachments/assets/ebc33627-9f61-457f-acaa-f8180ec4ff4a)

When 2 metrics selected and analytics gets disconnect

![Image](https://github.com/user-attachments/assets/233c5640-450d-4613-8ab3-dc281624bfae) ![Image](https://github.com/user-attachments/assets/493f571d-57f6-4918-af04-eb3ff62ea938)