Fixing duplicate override and using global overrides on Button for Domain Transfer and Link in Bio

[paulopmt1]

Follow-up to #79692 and #79708

Implementing Louis' suggestion that deal with the root cause: avoids an override of an override by removing @wordpress/components/build-style/style from client/landing/stepper/declarative-flow/internals/global.scss

More context: p1689919222911329-slack-C05CT832K2T

20230726133833876.mp4

Proposed Changes

• Removes unnecessary override at @wordpress/components/build-style/style from client/landing/stepper/declarative-flow/internals/global.scss
• Link in bio is using global overrides instead of local
• Domain transfer is using global overrides instead of local

Testing Instructions

Go to /setup/domain-transfer.

• Check the first two steps (the second one, we need to remove the disabled attribute to see it)
• Check the second step disable button state has the correct style
• The buttons should have blue-50 and should have Calypso hover state.

Go to /setup/link-in-bio.

• Check the first and second steps
• The buttons should have blue-50 and should have Calypso hover state.

Go to /setup/newsletter/newsletterSetup.

• Check the steps
• The buttons should have blue-50 and should have Calypso hover state.

Pre-merge Checklist

• Has the general commit checklist been followed? (PCYsg-hS-p2)
• Have you written new tests for your changes?
• Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
• Have you checked for TypeScript, React or other console errors?
• Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
• Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
• For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

[github-actions]

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

[matticbot]

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

Sections (~40 bytes added 📈 [gzipped])

Details

name                         parsed_size           gzip_size
newsletter-post-setup-flow         +42 B  (+0.0%)      +14 B  (+0.0%)
link-in-bio-tld-flow               +42 B  (+0.0%)      +15 B  (+0.0%)
link-in-bio-post-setup-flow        +42 B  (+0.0%)      +14 B  (+0.0%)
free-post-setup-flow               +42 B  (+0.0%)      +14 B  (+0.0%)
free-flow                          +42 B  (+0.0%)      +11 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.

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.

[agrullon95]

I see the button primary color appears as #117ac9; instead of #0675C4.
It's due to this CSS in /client/landing/stepper/declarative-flow/internals/global.scss

Line 163:

/**
 	 * Step Container
 	 */
	.step-container {
		--color-accent: #117ac9;
		--color-accent-60: #0e64a5;

		...
	}

I'm checking if we can remove this and instead rely on the :root { ... changes you committed to this branch.

[paulopmt1]

I see the button primary color appears as #117ac9; instead of #0675C4. It's due to this CSS in /client/landing/stepper/declarative-flow/internals/global.scss

Line 163:

/**
 	 * Step Container
 	 */
	.step-container {
		--color-accent: #117ac9;
		--color-accent-60: #0e64a5;

		...
	}

I'm checking if we can remove this and instead rely on the :root { ... changes you committed to this branch.

Good catch @agrullon95! Since I'm adding it to the body, it seems safe to remove it from there. Will do more tests here to ensure that's the case.

[agrullon95]

@paulopmt1 - I noticed another difference in the newsletter/LIB flow "disabled" button styling vs production.
Using setup/newsletter/newsletterSetup as an example, on production the color of the button text is #fff (on hover as well), but with these changes, the color of the button text is #a7aaad without hover and then set to #fff on hover.

Before (production):

After:

I think we should update the button text color to match production (#fff) for all buttons ( at least in stepper for now). This can be set in _wp-components-overrides.scss.

[paulopmt1]

@paulopmt1 - I noticed another difference in the newsletter/LIB flow "disabled" button styling vs production. Using setup/newsletter/newsletterSetup as an example, on production the color of the button text is #fff (on hover as well), but with these changes, the color of the button text is #a7aaad without hover and then set to #fff on hover.

Before (production):

I think we should update the button text color to match production (#fff) for all buttons ( at least in stepper for now). This can be set in _wp-components-overrides.scss.

Thanks for catching this one as well!

This happens because the font color expects a gray background for the disabled state. I'm leaning toward making this disable state equal to the domain transfer:

How do you see it @nuriapenya?

[paulopmt1]

Hey @agrullon95, for this iteration, I added back some overriding so we can deliver an improvement while getting a new design view about it. So it's ready for a new review.

[lsl]

This is what I was after thanks! There are a few minor things we should fix but I'll try to split these concerns off into another PR today so we can ship the main things asap.

[nuriapenya]

@paulopmt1 , @lsl as shared on Slack, I would do the following:

• Right now, align all the disabled states to match the Newsletter one (that means adapting the transfer flow)
• Later, in the future, revise these disabled buttons to potentially make them enabled. So they force error messages

Correct styles:

background-color: var(--studio-blue-20);
color: #fff;

[lsl]

Right now, align all the disabled states to match the Newsletter one (that means adapting the transfer flow)

Sounds good

Later, in the future, revise these disabled buttons to potentially make them enabled. So they force error messages

We use disabled states sometimes while waiting for network requests to complete. Might need to keep the disabled blue around for those use cases while also moving to using error messages more consistently.

Issue for this here: https://github.com/Automattic/dotcom-forge/issues/3205 not sure when if/we'll be able to prioritize it but it sounds like a good improvement.

[agrullon95]

Confirmed the enabled/disabled state styling for buttons in newsletter, link-in-bio, and domain-transfer flows are matching and displaying correctly.

[nuriapenya]

Amazing, everything looks great 💎 thanks everyone!