Bulk Domain Transfer: Improve domain form #79300

escapemanuele · 2023-07-12T13:38:02Z

Changes

Column for price
Fixed authorization code error message
Changed copy
Popover
Total price

Testing

Live link
Reach setup/domain-transfer/domains
Test the step

github-actions · 2023-07-12T13:38:14Z

	https://calypso.live?image=registry.a8c.com/calypso/app:build-73691

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-73691&env=jetpack

matticbot · 2023-07-12T13:40:01Z

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

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

name           parsed_size           gzip_size
entry-stepper       +233 B  (+0.0%)      +55 B  (+0.0%)

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

Sections (~52 bytes added 📈 [gzipped])

name             parsed_size           gzip_size
subscribers           +233 B  (+0.1%)      +52 B  (+0.0%)
sites-dashboard       +233 B  (+0.1%)      +52 B  (+0.0%)
settings              +233 B  (+0.0%)      +52 B  (+0.0%)
hosting               +233 B  (+0.0%)      +52 B  (+0.0%)
home                  +233 B  (+0.0%)      +52 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~52 bytes added 📈 [gzipped])

name               parsed_size           gzip_size
async-load-design       +233 B  (+0.0%)      +52 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.

alshakero · 2023-07-13T15:23:27Z

...per/declarative-flow/internals/steps-repository/domain-transfer-domains/domain-code-pair.tsx

+		return <div className="domains__domain-price-number disabled">$0</div>;
+	}
+
+	if ( ! saleCost && saleCost !== 0 ) {


Suggested change

if ( ! saleCost && saleCost !== 0 ) {

if ( saleCost === 0 ) {

I thought about this, but what about sales that are not zero but half the price? 🤔

You're right. Bad suggestion.

alshakero · 2023-07-13T15:24:30Z

...per/declarative-flow/internals/steps-repository/domain-transfer-domains/domain-code-pair.tsx

+	const { __ } = useI18n();
+
+	if ( ! rawPrice ) {
+		return <div className="domains__domain-price-number disabled">$0</div>;


We should probably call formatCurrency on the zero.

alshakero

Amazing work. This has been much harder than it seems. Left some comments.

alshakero · 2023-07-13T16:37:55Z

...ding/stepper/declarative-flow/internals/steps-repository/domain-transfer-domains/domains.tsx

+	const totalPrice = Object.values( domainsState ).reduce( ( total, currentDomain ) => {
+		if ( currentDomain.saleCost ) {
+			return total + currentDomain.saleCost;
+		}
+
+		return total + currentDomain.rawPrice;
+	}, 0 );


This probably should be a function outside the component, takes the state and gives a formatted price with a correct currency. I.e shouldn't be locked to USD.

alshakero · 2023-07-13T16:39:15Z

...per/declarative-flow/internals/steps-repository/domain-transfer-domains/domain-code-pair.tsx

+	if ( ! rawPrice ) {
+		return (
+			<div className="domains__domain-price-number disabled">
+				{ formatCurrency( 0, currencyCode, { stripZeros: true } ) }


I just realized that we don't need currency for the zero, 0 is enough, but not $0.

Ok! Do you mean just having
<div className="domains__domain-price-number disabled">0</div>
right?

alshakero · 2023-07-13T16:39:56Z

...ding/stepper/declarative-flow/internals/steps-repository/domain-transfer-domains/domains.tsx

@@ -107,6 +129,9 @@ const Domains: React.FC< Props > = ( { onSubmit } ) => {
 			domain: '',
 			auth: '',
 			valid: false,
+			rawPrice: 0,
+			saleCost: undefined,
+			currencyCode: 'USD',


Should be undefined.

alshakero · 2023-07-13T16:40:20Z

...ding/stepper/declarative-flow/internals/steps-repository/domain-transfer-domains/domains.tsx

@@ -143,6 +168,10 @@ const Domains: React.FC< Props > = ( { onSubmit } ) => {
 					{ __( 'Add another domain' ) }
 				</Button>
 			) }
+			<div className="bulk-domain-transfer__total-price">
+				<div>{ __( 'Total' ) }</div>
+				<div>{ formatCurrency( totalPrice, 'USD', { stripZeros: true } ) }</div>


Should be covered by the function I mention above.

escapemanuele · 2023-07-13T17:05:38Z

Thank you @alshakero, I think I addressed all your feedback!

alshakero

Pushed some changes and it looks great now!

alshakero

Pushed some changes and it looks great now!

a8ci18n · 2023-07-13T19:20:04Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/8549986

Hi @escapemanuele, could you please edit the description of this PR and add a screenshot for our translators? Ideally it'd include all of the following strings:

Unique code proving ownership, needed for secure domain transfer between registrars.
Add your domains

Thank you in advance!

cometgrrl · 2023-07-13T19:26:08Z

Minor copy feedback:
“Add as little or as many”
I think “few” goes with “many” (silly, but English is :weird: like that)
Text wrapping is not great. Maybe putting the second sentence on a new line?

cometgrrl · 2023-07-13T19:26:25Z

From the checkout screen, the back button used to go to /setup/domain-transfer/domains, and now it goes to /start/domains, which seems wrong.

john-legg · 2023-07-13T22:03:42Z

This is great work! While I was testing, I noticed that the width of the Price column is a bit wide. The link to the designs doesn't seem to work for me, so I don't know if this is intentional or not, but I just wanted to point it out. It's more noticeable when there's multiple domain rows.

escapemanuele · 2023-07-14T09:33:43Z

Hey @john-legg, thanks for the feedback. I gave the price column some space to accomodate for this:

The First year free is taking some space.
This is from the designs:

I'll try to remove some space 😄

karenroldan · 2023-07-14T12:03:18Z

Hi, @escapemanuele!

issue (ux,non-blocking): There is a dollar sign on the price per domain based on Figma. But, right now, it only shows 0

escapemanuele · 2023-07-14T13:37:44Z

Thank you @karenroldan, we'll try to address this!

a8ci18n · 2023-07-20T19:41:09Z

Translation for this Pull Request has now been finished.

escapemanuele marked this pull request as ready for review July 12, 2023 17:58

escapemanuele requested a review from a team July 13, 2023 10:45

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

escapemanuele added this to In progress in Vertex Team Board via automation Jul 13, 2023

escapemanuele self-assigned this Jul 13, 2023

escapemanuele mentioned this pull request Jul 13, 2023

Bulk domain transfer: Update/add copies #79348

Merged

alshakero reviewed Jul 13, 2023

View reviewed changes

escapemanuele added 11 commits July 13, 2023 18:27

First html

ee4422f

Total with same spacing

d0199a9

Fix error message

4874a2d

Fix validate css

4ff2837

Fix mobile and copy

bcc13d8

Fix popover

a657468

Fix popover on mobile

43f5df0

Add types and prices

e1b5850

Total price

5851147

Fix total when removing domain

913d149

Use onChange

7672fde

escapemanuele force-pushed the bulk-domain/domain-form-pricing branch from 8fa0f93 to 7672fde Compare July 13, 2023 16:29

escapemanuele requested a review from alshakero July 13, 2023 16:29

escapemanuele moved this from In progress to Needs review in Vertex Team Board Jul 13, 2023

alshakero requested changes Jul 13, 2023

View reviewed changes

Address feedback

6586267

escapemanuele requested a review from alshakero July 13, 2023 17:05

alshakero added 3 commits July 13, 2023 19:45

Show price when you have it

2922ded

Types

30c32d2

Handle zero

6ec0147

Minor accessibility issue

f370944

alshakero approved these changes Jul 13, 2023

View reviewed changes

escapemanuele merged commit 33e74ea into trunk Jul 13, 2023
18 checks passed

Vertex Team Board automation moved this from Needs review to Done Jul 13, 2023

escapemanuele deleted the bulk-domain/domain-form-pricing branch July 13, 2023 18:56

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 13, 2023

escapemanuele mentioned this pull request Jul 14, 2023

Bulk Domain Transfer: Addressing feedback #79410

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bulk Domain Transfer: Improve domain form #79300

Bulk Domain Transfer: Improve domain form #79300

escapemanuele commented Jul 12, 2023 •

edited

Loading

github-actions bot commented Jul 12, 2023 •

edited by matticbot

Loading

matticbot commented Jul 12, 2023 •

edited

Loading

alshakero Jul 13, 2023

escapemanuele Jul 13, 2023

alshakero Jul 13, 2023

alshakero Jul 13, 2023

alshakero left a comment

alshakero Jul 13, 2023

alshakero Jul 13, 2023

escapemanuele Jul 13, 2023

alshakero Jul 13, 2023

alshakero Jul 13, 2023

alshakero Jul 13, 2023

escapemanuele commented Jul 13, 2023

alshakero left a comment

alshakero left a comment

a8ci18n commented Jul 13, 2023 •

edited

Loading

cometgrrl commented Jul 13, 2023

cometgrrl commented Jul 13, 2023

john-legg commented Jul 13, 2023

escapemanuele commented Jul 14, 2023 •

edited

Loading

karenroldan commented Jul 14, 2023

escapemanuele commented Jul 14, 2023

a8ci18n commented Jul 20, 2023

	if ( ! saleCost && saleCost !== 0 ) {
	if ( saleCost === 0 ) {

Bulk Domain Transfer: Improve domain form #79300

Bulk Domain Transfer: Improve domain form #79300

Conversation

escapemanuele commented Jul 12, 2023 • edited Loading

Changes

Testing

github-actions bot commented Jul 12, 2023 • edited by matticbot Loading

matticbot commented Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alshakero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

escapemanuele commented Jul 13, 2023

alshakero left a comment

Choose a reason for hiding this comment

alshakero left a comment

Choose a reason for hiding this comment

a8ci18n commented Jul 13, 2023 • edited Loading

cometgrrl commented Jul 13, 2023

cometgrrl commented Jul 13, 2023

john-legg commented Jul 13, 2023

escapemanuele commented Jul 14, 2023 • edited Loading

karenroldan commented Jul 14, 2023

escapemanuele commented Jul 14, 2023

a8ci18n commented Jul 20, 2023

escapemanuele commented Jul 12, 2023 •

edited

Loading

github-actions bot commented Jul 12, 2023 •

edited by matticbot

Loading

matticbot commented Jul 12, 2023 •

edited

Loading

a8ci18n commented Jul 13, 2023 •

edited

Loading

escapemanuele commented Jul 14, 2023 •

edited

Loading