Gutenboarding: Fix "Choose a domain I own" layout on mobile #49249

StefanNieuwenhuis · 2021-01-25T09:45:38Z

Changes proposed in this Pull Request

The copy of the "Choose a domain I own" row on the Domain Picker is unreadable on mobile devices because the CTA-link is on the same row. This PR contains a fix that stacks (i.e. placed on separate rows) both on mobile viewports.

Technical changes

Update HTML semantics (i.e. from <label/> to <div/>) to align with the solution provided in the /start flow.
Update flexbox alignment (mobile-first approach).
Overrule flexbox alignment on larger-than-mobile-viewports.
Update padding for <ArrowButton/>. We have to revisit this while working on Focused Launch: Replace "Back" buttons with ButtonArrow onboarding component #48568.

Expected Result

Mobile

Desktop

Testing instructions

Gutenboarding

Go to /new/domains-modal
Confirm that:
- Both copy & CTA-link are on the same row
- The complete row is clickable and clicking redirects you to the domain's transfer/mapping view
- Clicking the CTA-link redirects you to the domain's transfer/mapping view.
Open DevTools and switch to a mobile viewport (e.g. iPhone X).
Revisit /new/domains-modal.
Confirm that:
- Copy & CTA-link are stacked and readable
- The complete row is clickable and clicking redirects you to the domain's transfer/mapping view.
- Clicking the CTA-link redirects you to the domain's transfer/mapping view
Switch from English to a more verbose language (e.g French) and repeat steps 1-6.

Focused Launch

Go to /page/[UNLAUNCHED_SITE_ID].wordpress.com/home?flags=create/focused-launch-flow.
Click "Launch" to open the Focused Launch flow.
Click on "View All Domains".
Confirm that:
- Both copy & CTA-link are on the same row
- The complete row is clickable and clicking redirects you to the domain's transfer/mapping view.
- Clicking the CTA-link redirects you to the domain's transfer/mapping view.
Open DevTools and switch to a mobile viewport (e.g. iPhone X).
Revisit /page/[UNLAUNCHED_SITE_ID].wordpress.com/home?flags=create/focused-launch-flow.
Confirm that:
- Copy & CTA-link are stacked and readable
- The complete row is clickable and clicking redirects you to the domain's transfer/mapping view
- Clicking the CTA-link redirects you to the domain's transfer/mapping view.
Switch from English to a more verbose language (e.g French) and repeat steps 1-7.

Fixes #49171

matticbot · 2021-01-25T09:45:40Z

Test live: https://calypso.live/?branch=fix/gutenboarding/domain-i-own-mobile-layout

matticbot · 2021-01-25T09:54:35Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~2 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-gutenboarding        +15 B  (+0.0%)       +2 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

ciampo · 2021-01-26T10:52:07Z

packages/onboarding/src/action-buttons/style.scss

@@ -92,6 +92,9 @@ button.action_buttons__button.components-button {
 		font-weight: 500;
 		color: var( --mainColor );

+		// @TODO: We have to revisit the ArrowButton's padding when we pick up #48568 (https://github.com/Automattic/wp-calypso/issues/48568)
+		padding-left: 0;


Just leaving this comment here to mark the TODO item

ciampo

Thank you for improving the layout! It looks much better on mobile now :)

I left some comments, and here's some additional feedback from my first round of review:

Can we improve the vertical alignment of text and arrow?

Can we align the "Use a domain I own >" button to the right, similarly to the items above?

While following the testing instructions, whenever I click and get redirected, the query params in the "redirected" URL are different from the testing instructions (maybe you just forgot to update them?):
- From gutenboarding, I got redirected to /start/domains/use-your-domain?source=http://calypso.localhost:3000/new/domains
- From Focused Launch, I got redirected to https://wordpress.com/start/new-launch/domains-launch/use-your-domain?siteSlug=teststart366187286.wordpress.com&source=http://calypso.localhost:3000/page/teststart366187286.wordpress.com/home?flags=create/focused-launch-flow — note that I got redirected to wordpress.com, even though I was testing on my local machine, and that the source query param contains the full URL (and not just the path)

packages/domain-picker/src/domain-picker/style.scss

ciampo · 2021-01-26T11:40:50Z

packages/domain-picker/src/domain-picker/use-your-domain-item.tsx

-		<label className="domain-picker__suggestion-item contains-link">
+		/* eslint-disable jsx-a11y/click-events-have-key-events */
+		/* eslint-disable jsx-a11y/interactive-supports-focus */
+		<div role="button" className="domain-picker__suggestion-item type-link" onClick={ onClick }>


This looks very hacky (and it looked hacky as well prior to this PR).

Why are we not using a <button />? Is it because there's another <button /> nested inside?

In general, nesting interactive HTML elements within each other (e.g button inside a label, or button inside a button) goes against the HTML spec

We may want to consider that the rest of the domain suggestion items have a different wrapping HTML element based on a prop — should we have this component behave the same way, so that it's always aligned with the rest of the items?

StefanNieuwenhuis · 2021-01-27T08:53:12Z

Can we improve the vertical alignment of text and arrow?

Absolutely and thanks for addressing this 👍 I've updated the component by aligning the icon with the copy's baseline w/ a negative margin since the svg contains whitespace (5px) around the chevron's path.

Can we align the "Use a domain I own >" button to the right, similarly to the items above?

Sure! I've updated the component.

While following the testing instructions, whenever I click and get redirected, the query params in the "redirected" URL are different from the testing instructions (maybe you just forgot to update them?):

I've updated the testing instructions.

ciampo · 2021-01-27T11:24:37Z

I started testing and noticed that on mobile the "Use a domain I own" button doesn't align correctly to the left

Also, what do you think about #49249 (comment)?

ciampo

Code changes LGTM, tests as per instructions — although https://github.com/Automattic/wp-calypso/pull/49249/files#r564448003? It hasn't been addressed yet.

@StefanNieuwenhuis what do you think? I understand that it's outside the scope of this PR, but at the very minimum we should discuss it and/or open a separate issue about it

StefanNieuwenhuis · 2021-01-28T09:25:22Z

Code changes LGTM, tests as per instructions — although https://github.com/Automattic/wp-calypso/pull/49249/files#r564448003? It hasn't been addressed yet.

@StefanNieuwenhuis what do you think? I understand that it's outside the scope of this PR, but at the very minimum we should discuss it and/or open a separate issue about it

Thanks for reviewing & approving!
Regarding your comment, sorry, I missed it entirely! I've abstracted the wrapper into a separate file. @ciampo, could you have another look, please?

Be more explicit when it comes to `flexbox`. Co-authored-by: Marco Ciampini <marco.ciampini@automattic.com>

The current semantics was a bit hacky and unaligned with the other domain suggestions, which are wrapped in a (surprise, surprice) wrapper component. To wrap our component, the wrapper is abstracted from the suggestion-item and moved to a seperate file.

ciampo

LGTM!

Feel free to merge once these two comments (this and this) are addressed 🚢

ciampo · 2021-01-29T12:49:29Z

packages/domain-picker/src/domain-picker/suggestion-item-wrapper.tsx

+const WrappingComponent = React.forwardRef< HTMLButtonElement, WrappingComponentProps >(
+	( { type, disabled, ...props }, ref ) => {
+		if ( type === 'button' ) {
+			return <div { ...( props as React.HTMLAttributes< HTMLDivElement > ) } />;
+		}
+		return (
+			<button
+				ref={ ref }
+				disabled={ disabled }
+				{ ...( props as React.ButtonHTMLAttributes< HTMLButtonElement > ) }
+			/>
+		);
+	}
+);


I'm still not a fan of the markup that this component outputs. Also, it's confusing that when type === button, the HTML element returned is a div.

We should open a follow-up issue to investigate the semantics of the domain picker a bit better, given all of its variations, and come up with more accessible markup/semantics.

Created #49462

packages/domain-picker/src/domain-picker/use-your-domain-item.tsx

StefanNieuwenhuis added [Pri] High [Goal] New Onboarding previously called Gutenboarding labels Jan 25, 2021

StefanNieuwenhuis self-assigned this Jan 25, 2021

StefanNieuwenhuis requested a review from a team January 25, 2021 13:41

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 25, 2021

StefanNieuwenhuis marked this pull request as ready for review January 25, 2021 13:41

ciampo self-requested a review January 26, 2021 10:47

ciampo reviewed Jan 26, 2021

View reviewed changes

ciampo self-requested a review January 26, 2021 11:08

ciampo requested changes Jan 26, 2021

View reviewed changes

StefanNieuwenhuis requested a review from ciampo January 27, 2021 08:53

ciampo approved these changes Jan 27, 2021

View reviewed changes

ciampo added the [Status] Needs Rebase label Jan 29, 2021

StefanNieuwenhuis and others added 9 commits January 29, 2021 13:30

Initial commit

f391eb8

Update HTML semantics

8120d3f

Update flexbox alignment

ccde95e

Update the padding for the ArrowButton component

8af8d1b

Update vertical alignment on desktop

250bb0f

Update packages/domain-picker/src/domain-picker/style.scss

6b21f2e

Be more explicit when it comes to `flexbox`. Co-authored-by: Marco Ciampini <marco.ciampini@automattic.com>

Update button + chevron alignment

f804396

Update ArrowButton alignment

c91bc57

StefanNieuwenhuis force-pushed the fix/gutenboarding/domain-i-own-mobile-layout branch from 3534764 to 717ae15 Compare January 29, 2021 12:30

ciampo removed the [Status] Needs Rebase label Jan 29, 2021

ciampo approved these changes Jan 29, 2021

View reviewed changes

ciampo reviewed Jan 29, 2021

View reviewed changes

packages/domain-picker/src/domain-picker/use-your-domain-item.tsx Outdated Show resolved Hide resolved

StefanNieuwenhuis mentioned this pull request Jan 29, 2021

Focused Launch: Update domain-picker semantics #49462

Closed

Remove unused ref

f5fee46

StefanNieuwenhuis merged commit 4e311f0 into trunk Jan 29, 2021

StefanNieuwenhuis deleted the fix/gutenboarding/domain-i-own-mobile-layout branch January 29, 2021 13:43

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 29, 2021

yansern mentioned this pull request Jan 29, 2021

Editing Toolkit: update to 2.16 #49437

Merged

8 tasks

razvanpapadopol mentioned this pull request Mar 16, 2021

Domain picker: stop click event propagation on the "Use a domain I own" button #51074

Merged

razvanpapadopol mentioned this pull request Apr 14, 2021

Domain Picker: Improve semantics #51175

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gutenboarding: Fix "Choose a domain I own" layout on mobile #49249

Gutenboarding: Fix "Choose a domain I own" layout on mobile #49249

StefanNieuwenhuis commented Jan 25, 2021 •

edited by ciampo

matticbot commented Jan 25, 2021

matticbot commented Jan 25, 2021 •

edited

ciampo Jan 26, 2021

ciampo left a comment

ciampo Jan 26, 2021

StefanNieuwenhuis commented Jan 27, 2021 •

edited

ciampo commented Jan 27, 2021

ciampo left a comment

StefanNieuwenhuis commented Jan 28, 2021 •

edited

ciampo left a comment •

edited

ciampo Jan 29, 2021

StefanNieuwenhuis Jan 29, 2021

Gutenboarding: Fix "Choose a domain I own" layout on mobile #49249

Gutenboarding: Fix "Choose a domain I own" layout on mobile #49249

Conversation

StefanNieuwenhuis commented Jan 25, 2021 • edited by ciampo

Changes proposed in this Pull Request

Technical changes

Expected Result

Testing instructions

Gutenboarding

Focused Launch

matticbot commented Jan 25, 2021

matticbot commented Jan 25, 2021 • edited

ciampo Jan 26, 2021

Choose a reason for hiding this comment

ciampo left a comment

Choose a reason for hiding this comment

ciampo Jan 26, 2021

Choose a reason for hiding this comment

StefanNieuwenhuis commented Jan 27, 2021 • edited

ciampo commented Jan 27, 2021

ciampo left a comment

Choose a reason for hiding this comment

StefanNieuwenhuis commented Jan 28, 2021 • edited

ciampo left a comment • edited

Choose a reason for hiding this comment

ciampo Jan 29, 2021

Choose a reason for hiding this comment

StefanNieuwenhuis Jan 29, 2021

Choose a reason for hiding this comment

StefanNieuwenhuis commented Jan 25, 2021 •

edited by ciampo

matticbot commented Jan 25, 2021 •

edited

StefanNieuwenhuis commented Jan 27, 2021 •

edited

StefanNieuwenhuis commented Jan 28, 2021 •

edited

ciampo left a comment •

edited