Add @automattic/onboarding package to standardize stylesheets & reuse components. #44077

yansern · 2020-07-10T19:00:49Z

Changes proposed in this Pull Request

Created @automattic/onboarding package.
The intention of this package is to offer onboarding stylesheets & components that can be reused across Calypso, FSE & WP-Admin. Currently it has SCSS variables & mixins, <Title> and <Subtitle> component taken out from Gutenboarding.
Expanded @automattic/typography with font family & font size variables.
This is taken out from client/assets/stylesheets/shared/_typography.scss file so it can be used outside of Calypso.
Clean up all the SCSS variables & mixins, <Title> & <Subtitle> duplication across the calypso codebase (within the scope of onboarding & typography). This includes:
- Updating gutenboarding to use @automattic/onboarding.
- Updating @automattic/domain-picker to use @automattic/onboarding.
- Updating @automattic/plans-grid to use @automattic/onboarding.
- Updating magic-login, wp-login & signup stylesheet to use mixins from @automattic/onboarding.
- Updating wpcom-block-editor-nux plugin to import fonts & mixins from @automattic/typography &
  @automattic/onboarding.

There are a lot of files modified in this PR. However, they do very simple changes. To review this PR, it is easier to review by commits, as commits are grouped thematically. Most work revolves around importing, renaming variables and removing duplicated codes.

Usage Examples

For using onboarding/typography variables & mixins:

@import '~@automattic/onboarding/styles/variables';
@import '~@automattic/onboarding/styles/mixins';
@import '~@automattic/typography/styles/fonts';
@import '~@automattic/typography/styles/variables';

For using <Title> & <SubTitle> components:

@import { Title, SubTitle } from '@automattic/onboarding';

Testing instructions

Run yarn to rebuild all packages.
Run yarn start to boot up Calypso locally.
As long as there is no compilation errors, all variables & mixins have been imported successfully.
Test gutenboarding on /new, everything should look the same.
Look around Calypso, everything should look the same.

Note: Testing launch flow on FSE is on this PR #44087 which is based off this PR.

Fixes part of #43750

… Renamed sass folder to styles. Added variables file.

…/onboarding package.

…automattic/typography & @automattic/onboarding.

matticbot · 2020-07-10T19:00:51Z

Test live: https://calypso.live/?branch=add/onboarding-package

matticbot · 2020-07-10T19:01:48Z

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

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

name                 parsed_size           gzip_size
entry-gutenboarding        -12 B  (-0.0%)      +12 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.

matticbot · 2020-07-10T19:10:42Z

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D46237-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2

yansern · 2020-07-10T19:22:49Z

packages/onboarding/styles/variables.scss

+$onboarding-header-height: $header-height;
+$onboarding-footer-height: $header-height;


Take note that this was originally hardcoded to 64px. This is now inherited directly from core WP which has a value of 60px. This ensures header height consistency from gutenboarding to block editor.

yansern · 2020-07-10T19:32:38Z

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

@@ -1,5 +1,5 @@
-@import '../styles/placeholder.scss'; // Contains the placeholder mixin
-@import '../styles/mixins';
+@import '~@automattic/calypso-color-schemes';


Need to beware not to import this multiple times as this creates styling duplication.

Hm! Wouldn't it now be included multiple times by importing this to Gutenboarding, which also includes this package? 🤔

Should it be declared it as peerDependencies?

Wouldn't it now be included multiple times by importing this to Gutenboarding, which also includes this package?

I think this is happening right now. If you look at entry-gutenboarding.css file the :root selector where all the color declarations are repeated at least twice. Need to investigate how this is happening but not something to fix in this PR.

Should it be declared it as peerDependencies?

Not sure if sass-loader actually reads these things.

yansern · 2020-07-10T19:36:00Z

packages/plans-grid/src/plans-details/style.scss

-@import '../base-styles.scss';
-@import '../mixins.scss';
-@import '../variables.scss';
+@import '~@wordpress/base-styles/breakpoints';
+@import '~@wordpress/base-styles/mixins';
+@import '~@automattic/typography/styles/variables';


This is done to import only the specific files that has the specific variables & mixins needed instead of blindly importing everything. You'll see this in a few other files as well.

yansern · 2020-07-10T19:36:29Z

packages/typography/package.json

-	"main": "sass/fonts.scss",
+	"main": "styles/fonts.scss",


The sass folder has been renamed to styles folder for consistency.

…c/onboarding.

wp-desktop

WordPress Desktop CI Failure for job "wp-desktop-source".

@yansern please inspect this job's build steps for breaking changes at this link. For temporal failures, you may try to "Rerun Workflow from Failed".

Please also ensure this branch is rebased off latest Calypso.

Dependencies fixed.

wp-desktop

WordPress Desktop CI Failure for job "wp-desktop-source".

@yansern please inspect this job's build steps for breaking changes at this link. For temporal failures, you may try to "Rerun Workflow from Failed".

Please also ensure this branch is rebased off latest Calypso.

nsakaimbo

@yansern 👋 Hello! The last run wp-desktop-source job was failinging with the error:

@automattic/onboarding: info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
lerna ERR! yarn run prepare exited 2 in '@automattic/onboarding'
lerna WARN complete Waiting for 7 child processes to exit. CTRL-C to exit immediately.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 2.

The wp-desktop review would have automatically been dismissed once the job was re-ran and passed (looks like it was dismissed it manually, so it's unclear what the CI status was).

However for some reason this job is missing from the latest SHA for this PR - not sure why. Also - only 8 checks were performed (there's typically around 22-23 CI jobs that are ran against any given Calypso PR).

Can we make sure that the full suite of CI tests is reflected prior to getting this landed?

razvanpapadopol · 2020-07-13T15:02:04Z

There seems to be this error logged when Install dependencies step fail for wp-desktop-source:

@automattic/onboarding: src/action-buttons/index.tsx(7,25): error TS2307: Cannot find module '@automattic/react-i18n'.

only 8 checks were performed

Not sure why this would happen. Re-adding Needs Review tag in case this helps.

nsakaimbo · 2020-07-13T15:08:45Z

Perhaps a rebase might help kick-start things as well?

If it helps, you can now debug the wp-desktop build wholly from within Calypso by running yarn run build-desktop from Calypso root (the step that's failing is yarn run build-desktop:source (which attempts to install the dependencies). Let me know if you need any help trying to debug further.

nsakaimbo · 2020-07-13T15:11:11Z

@automattic/onboarding: src/action-buttons/index.tsx(7,25): error TS2307: Cannot find module '@automattic/react-i18n'

It looks like this file was renamed as part of this PR? 🤔

razvanpapadopol · 2020-07-13T15:24:19Z

@automattic/onboarding: src/action-buttons/index.tsx(7,25): error TS2307: Cannot find module '@automattic/react-i18n'

It looks like this file was renamed as part of this PR? 🤔

Yes, it was extracted from /client to @automattic/onboarding package 🔎

nsakaimbo · 2020-07-13T15:33:17Z

@razvanpapadopol The reason CI isn't running on this branch is that the root Calypso CI job ci/setup is failing (link) at the Install Dependencies step with the error:

@automattic/onboarding: src/action-buttons/index.tsx(7,25): error TS2307: Cannot find module '@automattic/react-i18n'.

@automattic/onboarding: error Command failed with exit code 2.
@automattic/onboarding: info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
lerna ERR! yarn run prepare exited 2 in '@automattic/onboarding'

So this error isn't specific to the wp-desktop-source job.

You can verify the Circle CI activity for this branch PR at this link.

nsakaimbo · 2020-07-13T15:37:36Z

Update: looks like the last CI job that was attempted (and failed) was 12 hours ago. Seems Calypso CI hasn't been triggered at all since then.

simison · 2020-07-13T15:45:25Z

packages/onboarding/package.json

+		"classnames": "^2.2.6"
+	},
+	"devDependencies": {
+		"@wordpress/base-styles": "^1.8.0",


Needs @automattic/typography too?

Good catch! Added to onboarding & plans-grid package. d4633a6

Shouldn't this be added also to FSE deps? I see @wordpress/base-styles there

wp-desktop ci passing, closing review

yansern · 2020-07-14T07:39:23Z

The wp-desktop review would have automatically been dismissed once the job was re-ran and passed (looks like it was dismissed it manually, so it's unclear what the CI status was).

Sorry, that was me. Wasn't sure why the desktop CI was relevant but later I saw that all tests were passing I dismissed it. Good to know that if it was really ALL tests were passing then it will be automatically dismissed.

Can we make sure that the full suite of CI tests is reflected prior to getting this landed?

Yup all good now with 67014.

razvanpapadopol

Great work building a solid foundation for the reusable onboarding components! 💯

yansern added 8 commits July 10, 2020 20:18

Added @automattic/onboarding.

b4fa7d2

Refactor calypso's typography.scss to @automattic/typography package.…

01b5b41

… Renamed sass folder to styles. Added variables file.

Updated gutenboarding to use @automattic/onboarding package.

5a90738

Updated @automattic/domain-picker to use @automattic/onboarding package.

2b9572d

Updated @automattic/plans-grid to use @automattic/onboarding package.

1802551

Updated magic-login, wp-login & signup to use mixins from @automattic…

d934fae

…/onboarding package.

Update plans grid typography import path.

5da7a27

Updated wpcom-block-editor-nux plugin to import fonts & mixins from @…

648a147

…automattic/typography & @automattic/onboarding.

yansern added the [Goal] New Onboarding previously called Gutenboarding label Jul 10, 2020

yansern requested a review from a team as a code owner July 10, 2020 19:00

yansern self-assigned this Jul 10, 2020

yansern mentioned this pull request Jul 10, 2020

[EPIC] Step-by-step Launch: missing features #43750

Open

41 tasks

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

Added yarn.lock file.

013042c

yansern commented Jul 10, 2020

View reviewed changes

yansern added 3 commits July 11, 2020 07:19

Updated site-launch plugin typography import path.

4e7b8e9

Move ActionButtons, BackButton, NextButton & SkipButton to @automatti…

ff0ba81

…c/onboarding.

Move onboarding-z-index() @automattic/onboarding.

57508c0

wp-desktop previously requested changes Jul 13, 2020

View reviewed changes

Added missing dependencies for action buttons.

26021c5

yansern force-pushed the add/onboarding-package branch from a8ffdbc to 26021c5 Compare July 13, 2020 03:51

yansern mentioned this pull request Jul 13, 2020

Gutenboarding: Update site launch styling. #44087

Merged

Maybe this will fix the failing build.

2dec924

wp-desktop previously requested changes Jul 13, 2020

View reviewed changes

nsakaimbo self-requested a review July 13, 2020 14:19

nsakaimbo requested changes Jul 13, 2020

View reviewed changes

razvanpapadopol added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jul 13, 2020

simison reviewed Jul 13, 2020

View reviewed changes

Add missing @automattic/typography dependency.

d4633a6

yansern requested a review from nsakaimbo July 14, 2020 08:18

razvanpapadopol approved these changes Jul 14, 2020

View reviewed changes

Add @automattic/typography to FSE.

5468f08

yansern merged commit a83f9ac into master Jul 14, 2020

yansern deleted the add/onboarding-package branch July 14, 2020 09:57

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

yansern mentioned this pull request Jul 14, 2020

Use styles folder instead. #44130

Merged

razvanpapadopol mentioned this pull request Jul 14, 2020

Gutenboarding: extract reusable UI components and styles to packages #42950

Closed

deBhal mentioned this pull request Jul 20, 2020

Update FSE plugin to version to 1.13 #44125

Merged

31 tasks

razvanpapadopol mentioned this pull request Jul 23, 2020

Gutenboarding: Modal-first mobile-first domain picker stylesheets. #41806

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add @automattic/onboarding package to standardize stylesheets & reuse components. #44077

Add @automattic/onboarding package to standardize stylesheets & reuse components. #44077

yansern commented Jul 10, 2020 •

edited

Loading

matticbot commented Jul 10, 2020

matticbot commented Jul 10, 2020 •

edited

Loading

matticbot commented Jul 10, 2020

yansern Jul 10, 2020 •

edited

Loading

yansern Jul 10, 2020

simison Jul 13, 2020

yansern Jul 14, 2020 •

edited

Loading

yansern Jul 10, 2020

yansern Jul 10, 2020

wp-desktop left a comment

wp-desktop left a comment

nsakaimbo left a comment

razvanpapadopol commented Jul 13, 2020 •

edited

Loading

nsakaimbo commented Jul 13, 2020

nsakaimbo commented Jul 13, 2020

razvanpapadopol commented Jul 13, 2020

nsakaimbo commented Jul 13, 2020

nsakaimbo commented Jul 13, 2020

simison Jul 13, 2020

yansern Jul 14, 2020

razvanpapadopol Jul 14, 2020

yansern Jul 14, 2020

yansern commented Jul 14, 2020

razvanpapadopol left a comment

		$onboarding-header-height: $header-height;
		$onboarding-footer-height: $header-height;

Add @automattic/onboarding package to standardize stylesheets & reuse components. #44077

Add @automattic/onboarding package to standardize stylesheets & reuse components. #44077

Conversation

yansern commented Jul 10, 2020 • edited Loading

Changes proposed in this Pull Request

Usage Examples

Testing instructions

matticbot commented Jul 10, 2020

matticbot commented Jul 10, 2020 • edited Loading

matticbot commented Jul 10, 2020

yansern Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yansern Jul 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wp-desktop left a comment

Choose a reason for hiding this comment

wp-desktop left a comment

Choose a reason for hiding this comment

nsakaimbo left a comment

Choose a reason for hiding this comment

razvanpapadopol commented Jul 13, 2020 • edited Loading

nsakaimbo commented Jul 13, 2020

nsakaimbo commented Jul 13, 2020

razvanpapadopol commented Jul 13, 2020

nsakaimbo commented Jul 13, 2020

nsakaimbo commented Jul 13, 2020

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

yansern commented Jul 14, 2020

razvanpapadopol left a comment

Choose a reason for hiding this comment

yansern commented Jul 10, 2020 •

edited

Loading

matticbot commented Jul 10, 2020 •

edited

Loading

yansern Jul 10, 2020 •

edited

Loading

yansern Jul 14, 2020 •

edited

Loading

razvanpapadopol commented Jul 13, 2020 •

edited

Loading