Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Focused Launch: fix domain picker not showing in summary page in Calypso #47141

Merged

Conversation

razvanpapadopol
Copy link

@razvanpapadopol razvanpapadopol commented Nov 5, 2020

Changes proposed in this Pull Request

  • Update use-domain-search hook to get site title from useTitle hook.
  • Use React context to provide siteId and update all hooks to get the correct value always from context.
  • Extract sitePrimaryDomain, siteSubdomain, hasPaidDomain to useSiteDomains hook.
  • Domain picker should update domainSearch state using initialDomainSearch prop when showSearchField prop is true.

Testing instructions

Check the changes for calypso-dev version

  • Visit the page /page/[SITE_ID]/home?flags=create/focused-launch-flow-calypso on this PR's calypso.live site.
  • Domain suggestions should appear in the summary step.
  • Edit site title and advance to domain selection step. Domain suggestions should be updated.

Compare the in-editor versions (local vs production)

No change is expected here

  • Make sure you have an active connection to your sandbox, and that your sandbox is clean and up to date
  • In two separate terminal windows, from the root of the project, run:
    • yarn start
    • cd apps/editing-toolkit && yarn dev --sync
  • Sandbox an unlaunched WordPress site created via /start by adding it to your host file
  • Open console and run wp.data.dispatch('automattic/launch' ).openFocusedLaunch()

Test Gutenboarding Step-by-step launch flow for regressions

  • Create a site via /new and go to the editor.
  • Sandbox it as described above.
  • Press Launch button and go through the step-by-step flow to launch the site.

Fixes #47113
Fixes #47139
Fixes #46956
Fixes #47137
Fixes #47093

@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

matticbot commented Nov 5, 2020

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

App Entrypoints (~128 bytes removed 📉 [gzipped])

name                 parsed_size           gzip_size
entry-gutenboarding       -508 B  (-0.0%)     -128 B  (-0.0%)

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

Sections (~299 bytes removed 📉 [gzipped])

name              parsed_size           gzip_size
gutenberg-editor       -613 B  (-0.1%)     -263 B  (-0.2%)
signup                 -123 B  (-0.0%)       -9 B  (-0.0%)
jetpack-connect        -123 B  (-0.0%)       -9 B  (-0.0%)
checkout               -123 B  (-0.0%)       -9 B  (-0.0%)
accept-invite          -123 B  (-0.0%)       -9 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 (~19666 bytes removed 📉 [gzipped])

name                                            parsed_size            gzip_size
async-load-calypso-blocks-editor-launch-modal      -81416 B  (-34.1%)   -19657 B  (-31.9%)
async-load-design-wordpress-components-gallery       -123 B   (-0.0%)       -9 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.

@razvanpapadopol 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] In Progress labels Nov 5, 2020
@razvanpapadopol razvanpapadopol requested a review from a team November 5, 2020 14:43
Razvan Papadopol added 3 commits November 5, 2020 16:50
- Use React context to provide siteId and update hook to get the correct value always from context.
- Remove @wordpress/core-data dependency from editing-toolkit plugin.
@razvanpapadopol razvanpapadopol marked this pull request as ready for review November 5, 2020 14:50
@razvanpapadopol razvanpapadopol force-pushed the fix/focused-launch-domain-picker-not-working-in-calypso branch from c1046f5 to e159544 Compare November 5, 2020 15:23
@matticbot
Copy link
Contributor

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

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

…rch prop when showSearchField prop is true.
@ciampo
Copy link
Contributor

ciampo commented Nov 5, 2020

Code LGTM, and the changes test correctly. Thank you

We just need to solve conflicts before being able to merge

Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-tested after solving conflicts, everything still works as expected.

Great job on this PR, it solves at once many small-medium issues

@ciampo ciampo merged commit 158f409 into master Nov 6, 2020
@ciampo ciampo deleted the fix/focused-launch-domain-picker-not-working-in-calypso branch November 6, 2020 09:26
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment