Enhancement/8895 km setup cta banner

[zutigrm]

Summary

Addresses issue:

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

Relevant technical choices

PR Author Checklist

• My code is tested and passes existing unit tests.
• My code has an appropriate set of unit tests which all pass.
• My code is backward-compatible with WordPress 5.2 and PHP 7.4.
• My code follows the WordPress coding standards.
• My code has proper inline documentation.
• I have added a QA Brief on the issue linked above.
• I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

---

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

Code Reviewer Checklist

• Run the code.
• Ensure the acceptance criteria are satisfied.
• Reassess the implementation with the IB.
• Ensure no unrelated changes are included.
• Ensure CI checks pass.
• Check Storybook where applicable.
• Ensure there is a QA Brief.

Merge Reviewer Checklist

• Ensure the PR has the correct target branch.
• Double-check that the PR is okay to be merged.
• Ensure the corresponding issue has a ZenHub release assigned.
• Add a changelog message to the issue.

[github-actions]

Build files for 5cc6da8 have been deleted.

[zutigrm]

Failing jest test has an open issue #9316 and is not related to this change

[eugene-manuilov]

@zutigrm looks like the new design doesn't match how it looks in Figma. Could you please check it again and ensure that it looks correctly.

[zutigrm]

@eugene-manuilov hm, not sure what you mean, for example here is the screenshot from the desktop layout from figma design

Here is the screenshot of the KMW widget from the dashboard of the current branch:

Also for other screens, styling is following the designs from figma. The only difference I just noticed is the that your word is falling to the second line in figma, so I did quick adjustment for that.

Perhaps you checked different figma link?

[eugene-manuilov]

hm, not sure what you mean

@zutigrm take a look at the large version of the vrt screenshot (tests/backstop/reference/google-site-kit_KeyMetrics_SetupCTAWidget_0_document_2_large.png), it doesn't seem to be right, does it?

[zutigrm]

@eugene-manuilov Couldn't find which resolution large uses in VRT, but basically it is between tablet size 960px and less than 1280. The banner couldn't fit desktop layout on larger tablet sizes in this instance, so it starts from 1280px (which is the smallest available desktop size), but bellow that it is showing the tablet layout (big tablets in horizontal mode like iPad Air is 1180px, which is still not big enough to properly fit desktop layout).

The desktop layout is styled to match the real desktop screens from 1280px, everything bellow that is fitting the tablet layout (per figma design), as desktop layout due to the text falling in more lines expands the height and doesn't look eye pleasing on big tablet sizes like it looks on desktop screens.

Now I can try to implement some custom size bellow 1280px (didn't find any desktop screen bellow 1280px in real usage) to squeeze somehow desktop layout only for the VRT purposes, but that will not do much for real life users, since layout is already optimised for them. WDYT?

[eugene-manuilov]

... basically it is between tablet size 960px and less than 1280px ...

@zutigrm based on our scss settings we consider screen sizes as desktop ones for everything that is bigger than 960px:

site-kit-wp/assets/sass/config/_variables.scss

Lines 263 to 269 in 047b543

	// Width Variables (breakpoints)
	$width-xsmall: 450;
	$width-tablet: 600;
	$width-wpAdminBarTablet: 783;
	$width-desktop: 960;
	$width-xlarge: 1280;
	$width-xxlarge: 1440;

It means that CTA banner design should account for it and use the desktop version for everything that equals or bigger than 960px.

[zutigrm]

@eugene-manuilov Yes, that's where the difference lies, we do usually use 960px for desktop layout, but in this case, desktop layout for this banner can't fit on that small size. The sizes in the design and length of the text causes banner to be too crowded and increase the height, which also leaves the graphic in a position where it can fit that height and it looks unpolished. So for this reason I had to target desktop from 1280px breakpoint, which is smallest desktop size in real use, and tablet layout is shown for screens bellow that breakpoint

[zutigrm]

I included subtitle as component, to allow for conditional rendering, since in new design, subtitle is show only after KMW is setup

[zutigrm]

To adapt to the new design of conditionally displaying the subtitle, had to make small modification to widget logic, to make subtitle more flexible - allowing it to render component when passed

[eugene-manuilov]

The text should be aligned to center on the new version.

[zutigrm]

Thanks, it does looks better on this variations, missed it initially.

[zutigrm]

Thanks @eugene-manuilov PR updated

[eugene-manuilov]

Forgot to approve it before merging.