-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Redirect user to home page when home/launchpad-first is enabled #98835
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~137 bytes added 📈 [gzipped])
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. Generated by performance advisor bot at iscalypsofastyet.com. |
There's an error at times in the launchpad, It's not related to this PR but just calling out. Screen.Recording.2025-01-23.at.1.27.57.PM.movThis PR works to redirect to launchpad my home. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking if this is the right solution.
In my head, redirecting within the Launchpad component would make sense, because then whenever they tried to enter any Launchpad flow step (after creating a site of course), they'd be sent home. This is especially true in cases where they click the onboarding link in emails or the help center. It's usually /setup/(flowname)/launchpad
.
Now, the site-setup
or onboarding
flows don't have a Launchpad, so it depends on the intent of the site (write
or build
in this case.) Maybe we could do this differently, and within the Launchpad step redirect the user home if:
- They have created the site through
onboarding
- Their intent is
write
orbuild
(we can use the same helper function) - The feature flag is on?
🤔
@@ -296,6 +297,10 @@ const siteSetupFlow: FlowV1 = { | |||
} | |||
|
|||
if ( isLaunchpadIntent( intent ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could write this as if ( isLaunchpadIntent( intent ) && ! config.isEnabled( 'home/launchpad-first' ) )
cd602bf
to
6768100
Compare
@@ -96,6 +97,11 @@ const Launchpad: Step = ( { navigation, flow }: LaunchpadProps ) => { | |||
return; | |||
} | |||
|
|||
if ( config.isEnabled( 'home/launchpad-first' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should also check if the site_creation_flow
is onboarding
because we want to gate the changes to that flow.
Like this. I think we can even extract that function to its own file and reuse it here 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, reused it here: 1189aac
Also enabled it on dev by default since we lose that flag when redirecting through pages
548865c
to
1189aac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
client/landing/stepper/declarative-flow/internals/steps-repository/launchpad/index.tsx
Outdated
Show resolved
Hide resolved
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 |
Also enabled it on dev by default since we lose that flag when redirecting through pages
Related to #98812
Proposed Changes
Why are these changes being made?
This is part of Launchpad In My Home project: pet6gk-1T7-p2
Testing Instructions
yarn start
command so your local development will use the"home/launchpad-first": true
flag valuePre-merge Checklist