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

Import flows: Refactoring: useQuery instead of redux; update CTA text... #85784

Merged
merged 11 commits into from
Jan 12, 2024

Conversation

bogiii
Copy link
Contributor

@bogiii bogiii commented Dec 26, 2023

Related to #85711

Proposed Changes

  • Simplified Import Ready components: Got rid of connect redux wrapper
  • Ready components: Updated logic to allow direct step access
  • Replaced redux with useQuery approach for analyzing entered url
  • Renamed obsolete "Start building" CTA with "Go to goals"

Testing Instructions

  • Go to /setup/import-focused?siteSlug={SLUG}
    • or /setup/site-setup?siteSlug={SLUG}
    • or /setup/import-hosted-site
  • Pass through the flow, and try different import options: import everything, content only for different platforms
  • Check if everything works as before

Note: For easier review, follow commit list (e2e tests are green 👌)

Now, the import-ready preview screen is not redirected back to the capture screen if you press refresh. It will stay on the same page; the "from" query param is included.
This is the required changes for future moves related to the task: #85711

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • https://wpcalypso.wordpress.com/devdocs/docs/testing/index.md 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)?

@bogiii bogiii self-assigned this Dec 26, 2023
Copy link

github-actions bot commented Dec 26, 2023

@matticbot
Copy link
Contributor

matticbot commented Dec 26, 2023

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

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

name                   parsed_size           gzip_size
entry-subscriptions         -126 B  (-0.0%)      -40 B  (-0.0%)
entry-stepper               -126 B  (-0.0%)      -45 B  (-0.0%)
entry-main                  -126 B  (-0.0%)      -40 B  (-0.0%)
entry-login                 -126 B  (-0.0%)      -40 B  (-0.0%)
entry-domains-landing       -126 B  (-0.0%)      -40 B  (-0.0%)
entry-browsehappy           -126 B  (-0.1%)      -40 B  (-0.1%)

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

Sections (~2877 bytes added 📈 [gzipped])

name                       parsed_size           gzip_size
with-theme-assembler-flow      -1188 B  (-2.5%)      -39 B  (-0.4%)
update-options-flow            -1188 B  (-4.0%)      -40 B  (-0.9%)
update-design-flow             -1188 B  (-0.1%)      -47 B  (-0.0%)
free-post-setup-flow           -1188 B  (-4.1%)      -40 B  (-1.0%)
free-flow                      -1188 B  (-3.2%)      -67 B  (-1.0%)
assembler-first-flow           -1188 B  (-2.2%)      -81 B  (-0.7%)
ai-assembler-flow              -1188 B  (-2.1%)      -94 B  (-0.8%)
site-setup-flow                -1151 B  (-2.2%)      -48 B  (-0.5%)
import-hosted-site-flow         +911 B  (+0.1%)     +748 B  (+0.3%)
import-flow                     +911 B  (+0.0%)     +298 B  (+0.0%)
site-profiler                   +254 B  (+0.7%)      +57 B  (+0.6%)
trial-wooexpress-flow            -98 B  (-0.1%)      -30 B  (-0.1%)
tailored-ecommerce-flow          -98 B  (-0.0%)      -30 B  (-0.0%)
link-in-bio-tld-flow             -98 B  (-0.0%)      -25 B  (-0.0%)
copy-site-flow                   -98 B  (-0.0%)      -25 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.

@bogiii bogiii changed the title Tools/import page update; replace it with import onboarding flow Import flows: Refactoring: useQuery instead of redux; update CTA text... Dec 28, 2023
@bogiii bogiii marked this pull request as ready for review December 28, 2023 10:42
@bogiii bogiii requested a review from a team as a code owner December 28, 2023 10:42
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 28, 2023
Copy link
Contributor

@ouikhuan ouikhuan left a comment

Choose a reason for hiding this comment

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

Test various flows and all works as expected, great job!

Now, the import-ready preview screen is not redirected back to the capture screen if you press refresh. It will stay on the same page;

I'm unsure how to test this though, for me the import-ready preview screen seems not existing in the flow anymore (Same behavior on production). Only see it flickering between the steps and land on the next steps.

Screen.Recording.2023-12-29.at.3.57.49.PM.mov

@bogiii
Copy link
Contributor Author

bogiii commented Dec 29, 2023

I'm unsure how to test this though, for me the import-ready preview screen seems not existing in the flow anymore (Same behavior on production). Only see it flickering between the steps and land on the next steps.

We have recently removed that screen from the WordPress flow, and if you want to test it. You can try different source platforms, for example: "medium," "wix," or "Squarespace."

@ouikhuan
Copy link
Contributor

ouikhuan commented Jan 2, 2024

We have recently removed that screen from the WordPress flow, and if you want to test it. You can try different source platforms, for example: "medium," "wix," or "Squarespace."

Thanks for the explanation! Tried to test the wix following the instructions but when I refresh and click back button it still goes back to the capture screen. Am I missing something here?

Screen.Recording.2024-01-02.at.11.07.08.AM.mov

Removed dependency of the capture step, which, before this change, provided the urlData. Now, the fetching urlData logic is there.
This allows us to get cached analyzed data for the same URL between steps.
We need this to keep only siteSlug queryParam for the 'import' step.
This allows us to have direct access support of ready screens, getting rid of urlData dependency from the capture screen.
@bogiii
Copy link
Contributor Author

bogiii commented Jan 11, 2024

Thanks for the explanation! Tried to test the wix following the instructions but when I refresh and click back button it still goes back to the capture screen. Am I missing something here?

From my point of view, this is expected behavior.

@ouikhuan
Copy link
Contributor

From my point of view, this is expected behavior.

Ok, in this case, I have no other questions here 👍

@matticbot
Copy link
Contributor

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/tools-import on your sandbox.

@bogiii bogiii merged commit 08075be into trunk Jan 12, 2024
11 checks passed
@bogiii bogiii deleted the update/tools-import branch January 12, 2024 09:44
@github-actions 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 Jan 12, 2024
TimBroddin pushed a commit that referenced this pull request Jan 18, 2024
…... (#85784)

* Rename obsolete "Start building" to "Back to goals" button text

* Update generateStepPath helper method supporting goals step

* Ready steps:  Update logic to allow direct step access
Removed dependency of the capture step, which, before this change, provided the urlData. Now, the fetching urlData logic is there.

* Ready component: Update effect dependencies

* useAnalyzeUrlQuery: Update stale time to 5 seconds
This allows us to get cached analyzed data for the same URL between steps.

* site-setup flow: Match goToStep logic with import-focused-flow
We need this to keep only siteSlug queryParam for the 'import' step.

* Capture step: Update logic proving fromUrl as query param
This allows us to have direct access support of ready screens, getting rid of urlData dependency from the capture screen.

* Capture screen: Get rid of redux connect data; simplify component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants