-
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
Stepper Tracking: Use same tracking values as in start #94706
base: trunk
Are you sure you want to change the base?
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: App Entrypoints (~99 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~1241 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. Async-loaded Components (~7 bytes added 📈 [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. |
@@ -18,16 +23,16 @@ export const useRecordSignupComplete = ( flow: string | null ) => { | |||
selectedDomain: ( select( ONBOARD_STORE ) as OnboardSelect ).getSelectedDomain(), | |||
}; | |||
}, [] ); | |||
const isNewishUser = isUserRegistrationDaysWithinRange( state, null, 0, 7 ); | |||
const dependencies = getSignupDependencyStore( state ); |
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.
Note for myself:
I need to figure why getSignupDependencyStore always returns an {}
a0974f7
to
fabcdf2
Compare
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 |
@@ -3,68 +3,81 @@ import { useSelect } from '@wordpress/data'; | |||
import { useCallback } from 'react'; | |||
import { USER_STORE, ONBOARD_STORE } from 'calypso/landing/stepper/stores'; | |||
import { SIGNUP_DOMAIN_ORIGIN, recordSignupComplete } from 'calypso/lib/analytics/signup'; | |||
import { useSelector } from 'calypso/state'; | |||
import isUserRegistrationDaysWithinRange from 'calypso/state/selectors/is-user-registration-days-within-range'; | |||
import { useSite } from './use-site'; | |||
import type { UserSelect, OnboardSelect } from '@automattic/data-stores'; | |||
|
|||
export const useRecordSignupComplete = ( flow: string | null ) => { | |||
const site = useSite(); |
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.
@escapemanuele is this where the issue stems? It looks like it grabs site ID/slug from URL params, which would obviously be undefined
unless carried along?
Isn't there a store/state slice that gets populated with site ID when one is created? We should probably look at create-site step and do that if not?
@@ -21,6 +24,9 @@ export function useCreateAccountMutation() { | |||
} ); | |||
}, | |||
onSuccess: ( data ) => { | |||
if ( 'isNewAccountCreated' in data && data.isNewAccountCreated ) { | |||
setIsNewUser( true ); |
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.
One thing that, I think, would be nice to make sure it is clear is that the user will be "newUser" just once. For instance, if you create the user, then land on the ___domain's step and refresh the page, the user is not new anymore.
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.
Some differences I noticed /setup
vs /start
/setup
includes a few extra props: starting_point
, theme
, intent
I went through /setup
with an existing user and the elapsed_time_since_start
is logged as null.
Fixes #94688
Proposed Changes
isNewUser
andisNew7DUserSite
values, these values are used to properly log thecalypso_signup_complete
tracking event.Why are these changes being made?
/start
logic.Testing Instructions
calypso_signup_complete
gets triggered correctly, with the blog_id