-
Notifications
You must be signed in to change notification settings - Fork 2k
Connect Refresh: Updated Woo DNA JP flow to unified Login #104506
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
base: trunk
Are you sure you want to change the base?
Conversation
Link to live branch is being generated... |
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~7672 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~194 bytes removed 📉 [gzipped])
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 (~490 bytes removed 📉 [gzipped])
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. Generated by performance advisor bot at iscalypsofastyet.com. |
b40efa5
to
9967f1d
Compare
85fe68d
to
f50532a
Compare
75d5fbb
to
e1f964d
Compare
Hey @chriskmnds, thank you for the PR! I went through Woo and other pages and all looks good. I'll go through the code now, but before I do that I did some investigation around this I found this D150157-code with instructions to test. I followed it to install Woo Shipping. Activating it shows me the Connect button: Connect button takes me to this URL.
I would say, yes, this seems pretty active still. But does it need this custom styling, I doubt it, so it can be aligned with WooJPC, in my opinion.
Interestingly, this PR doesn't change much in the designs, I hope I didn't mess up something but this is the URL I used. |
Thanks @raicem!
Interesting! I think you just unearthed another OAuth Login screen that we haven't migrated. But this is also part of the Connect flow, so not sure how that works right now. The Woo DNA flow that I chased in this PR was defined when I think what I will try next is to include both the client ID I'll dig a little more and ping you back to give it another look. Thanks! 🙏 |
43e75f4
to
a00f355
Compare
f50532a
to
3c05d49
Compare
3c05d49
to
2df9e79
Compare
Gosh, this is old code #41798 |
Part of https://linear.app/a8c/issue/DOTCOM-13732/woo-dna-jp-flow
Proposed Changes
?woodna_service_name=1
exists in the URL.Questions
WooJPC
client Logins? (so we can more effectively remove all the plumbing instead)isJetpackWooDnaFlow
part ofisWoo
(fromgetWoo
) selector and avoid the extra property in the code?Media
Why are these changes being made?
Part of https://linear.app/a8c/issue/DOTCOM-13732/woo-dna-jp-flow
Testing Instructions
Pre-merge Checklist