Get started page UX improvements: add customer quotes card #1449

ianlin · 2022-04-27T10:52:37Z

Changes proposed in this Pull Request:

Part of #1392, this PR adds the customer quotes card to the new get started page.

Design for desktop: eQ9O4m2flzrcAiDMXkOW0m-fi-7350%3A121231
Design for mobile: eQ9O4m2flzrcAiDMXkOW0m-fi-7476%3A123025

Screenshots:

Desktop

Mobile

Detailed test instructions:

Go to GLA's get started page: https://domain.test/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fstart
Check the fourth card of the get started page, the UI should align with the design.

Changelog entry

Update - Add the customer quotes card for UX improvements on get started page.

puntope · 2022-04-27T12:24:05Z

js/src/get-started-page/index.js

@@ -14,6 +15,7 @@ const GetStartedPage = () => {
 			<GetStartedWithVideoCard />
 			<BenefitsCard />
 			<FeaturesCard />
+			<CaseStudiesCard />


💅

CaseStudies? In this case I guess it will be more accurate to use CustomerQuotes or Testimonials.

Both terms of case studies and customer quotes appear in the design brief document in this post: pcTzPl-i7-p2#design-requirements. But I agree that customer quotes is more straightforward, updated in ca06c61, thanks!

puntope · 2022-04-27T12:26:30Z

js/src/get-started-page/case-studies-card/index.scss

+	// Adjust <CardHeader> imported from @wordpress/components.
+	// Repeat selector to make it higher priority.
+	.components-card__header.components-card__header {
+		flex-direction: column;
+		padding: $grid-unit * 5.75 $grid-unit * 14 $grid-unit-50;
+		text-align: center;
+
+		@media (max-width: $break-small) {
+			padding: $grid-unit-30;
+		}
+	}


💅 / 🚧

Inserting this selector inside .components-flex also works and we can avoid that double selector patch.

.components-flex { gap: $grid-unit * 3.5; padding: 0 $grid-unit * 7.75 $grid-unit * 5.75; &.components-card__header { flex-direction: column; ...

Thanks for it! Using this form is clearer, updated in c673732. Also updated for other cards in fd58db1.

Note that it works for CardHeader since it has .components-flex class, while CardBody doesn't have that class so in js/src/get-started-page/benefits-card/index.scss I still use the repeating selectors hack in order to override the CardBody's original styles.

puntope · 2022-04-27T12:45:12Z

js/src/get-started-page/case-studies-card/index.js

+				</Text>
+			</CardHeader>
+			<Flex gap={ 0 }>
+				<ContentBlock


💅 I'm not convinced neither by ContentBlock as name for this. Maybe TestimonialItem or Quote or Testimonial?

Thanks for the suggestion! Updated in 6404822.

puntope · 2022-04-27T12:46:47Z

js/src/get-started-page/case-studies-card/index.scss

+
+	.components-flex {
+		gap: $grid-unit * 3.5;
+		padding: 0 $grid-unit * 7.75 $grid-unit * 5.75;


💅 / ❓

Not sure if this is already discussed but maybe we can create some SCSS variables for the fonts and some spacing is being reused all over get-started-page directory.

Thanks for the suggestion! I added a commit 13e1ac1 to extract some shared variables for font sizes and line heights as they've got more common uses.

There are too many different sizes of the spacing that I've used in get started page based on the design, for example:

10px; 20px; 28px; 40px; 44px; 46px; 50px; 55px; 62px; 88px; 112px;

It's a bit tricky to make them as variables tho.

puntope

Looking good ✅ thanks for working on this.

I'd love to discuss about that "Repeat selector to make it higher priority." hack. I guess there here are more standard ways to do so.

Would be nice also to evaluate the use and creation of variables for the common css values.

Rest of my comments are just opinions about naming.

ianlin · 2022-04-28T07:34:06Z

Hey @puntope, thanks for the review. I pushed some commits based on your suggestions and it's ready for another round of review.

puntope

Thanks for the adjustments it looks accurate with design and cleaner now.

Sorry. I want to mention 2 details more before approve. 1 of them seems to be regarding previous PRs since its related to other section.

I saw that in my tablet in vertical some resolutions the image on benefits-card is not covering the container. Wrapping the image in flex and playing with object-fit: cover maybe solves it.

In the quotes, if one paragraph is higher than the other (because resolution or content length) it looks unbalanced

Maybe align-items: stretch ??

ianlin · 2022-04-29T05:16:04Z

Thanks @puntope! Miss the tablet's resolution, noted! I pushed two commits a369c68 and 0011c43 to address the problem.

puntope · 2022-04-29T07:04:12Z

commits a369c68

Thanks for the change, it is no space anymore in the container, however now the image gets stretched. Adding object-fit: cover to the img element may solve this issue.

and 0011c43

it works! 👌 Thanks

So it will work best on all resolutions of devices.

ianlin · 2022-04-29T10:20:13Z

@puntope

I did try object-fit: cover and it looks like it's being stretched, while it looks normal when object-fit: cover is not added.

With `object-fit: cover`

Without `object-fit: cover`

But I was using the dev tool's device toolbar to check the result, it might not be accurate as using the real tablet so I still added object-fit: cover in 8f321ed as it doesn't look harm if we apply it.

Thanks for the detailed review!

puntope

✅ APPROVED good work!

Thanks for all the adjustments. The comment about _shared.scss here #1460 (comment)

I guess will be addressed in future PR.

ianlin added 3 commits April 27, 2022 17:47

Add a quote image for get started case studies card

802dffe

Add title for case studies card in get started page

16adafc

Add contents to case studies card in get started page

6a955aa

ianlin added the type: enhancement The issue is a request for an enhancement. label Apr 27, 2022

ianlin requested a review from a team April 27, 2022 10:52

ianlin self-assigned this Apr 27, 2022

puntope reviewed Apr 27, 2022

View reviewed changes

ianlin added 4 commits April 28, 2022 12:58

Rename case studies to customer quotes in get started page

ca06c61

Refactor css to use more specific rules rather than repeating selectors

c673732

Rename ContentBlock to Quote in customer quotes card

6404822

Use shared variables for font sizes and line heights in get started page

13e1ac1

ianlin changed the title ~~Get started page UX improvements: add case studies card~~ Get started page UX improvements: add customer quotes card Apr 28, 2022

Refactor css to use more specific rules rather than repeating selectors

fd58db1

ianlin requested a review from puntope April 28, 2022 07:34

puntope reviewed Apr 28, 2022

View reviewed changes

ianlin added 2 commits April 29, 2022 13:06

Makes image in benefits card responsive in desktops/mobiles/tablets

a369c68

Make customer quotes look more balanced in different resolution

0011c43

ianlin mentioned this pull request Apr 29, 2022

Get started page UX improvements: add get started card #1460

Merged

Add object-fit: cover in features card's image

8f321ed

So it will work best on all resolutions of devices.

ianlin requested a review from puntope April 29, 2022 10:20

puntope approved these changes Apr 29, 2022

View reviewed changes

ianlin merged commit 4669394 into feature/1392-ux-improvements-on-get-started-page May 3, 2022

ianlin deleted the add/1392-case-studies-card branch May 3, 2022 02:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get started page UX improvements: add customer quotes card #1449

Get started page UX improvements: add customer quotes card #1449

ianlin commented Apr 27, 2022 •

edited

Loading

puntope Apr 27, 2022

ianlin Apr 28, 2022 •

edited

Loading

puntope Apr 27, 2022 •

edited

Loading

ianlin Apr 28, 2022

puntope Apr 27, 2022

ianlin Apr 28, 2022

puntope Apr 27, 2022

ianlin Apr 28, 2022

puntope left a comment

ianlin commented Apr 28, 2022

puntope left a comment

ianlin commented Apr 29, 2022

puntope commented Apr 29, 2022

ianlin commented Apr 29, 2022

puntope left a comment

Get started page UX improvements: add customer quotes card #1449

Get started page UX improvements: add customer quotes card #1449

Conversation

ianlin commented Apr 27, 2022 • edited Loading

Changes proposed in this Pull Request:

Screenshots:

Desktop

Mobile

Detailed test instructions:

Changelog entry

puntope Apr 27, 2022

Choose a reason for hiding this comment

ianlin Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

puntope Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

ianlin Apr 28, 2022

Choose a reason for hiding this comment

puntope Apr 27, 2022

Choose a reason for hiding this comment

ianlin Apr 28, 2022

Choose a reason for hiding this comment

puntope Apr 27, 2022

Choose a reason for hiding this comment

ianlin Apr 28, 2022

Choose a reason for hiding this comment

puntope left a comment

Choose a reason for hiding this comment

ianlin commented Apr 28, 2022

puntope left a comment

Choose a reason for hiding this comment

ianlin commented Apr 29, 2022

puntope commented Apr 29, 2022

ianlin commented Apr 29, 2022

With object-fit: cover

Without object-fit: cover

puntope left a comment

Choose a reason for hiding this comment

ianlin commented Apr 27, 2022 •

edited

Loading

ianlin Apr 28, 2022 •

edited

Loading

puntope Apr 27, 2022 •

edited

Loading

With `object-fit: cover`

Without `object-fit: cover`