-
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
Add recordTracksEvent for Editor NUX modal #46796
Conversation
Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com D51826-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 |
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
@@ -52,6 +53,9 @@ function WpcomNux() { | |||
if ( ! isWpcomNuxEnabled || isSPTOpen ) { | |||
return null; | |||
} | |||
recordTracksEvent( 'calypso_newsite_wpcomnux_view', { | |||
is_new_flow: isGutenboarding, |
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 think isGutenboarding
is defined a few lines beneath, so this will be undefined
@ramonjd I left instructions to reset nux stats in this PR description, but really that didn’t work for me most of the time. Do you see an error I made? Is there somewhere else where the nux status needs to be cleared? Or cache cleared? (I'll delete that part of the PR description if it is useless.) Going another route to fake the nux status: I set the relevant api endpoints to return hard coded values: Line 60 in 720b5da
and returned But still I was seeing the redux store return “false” wp-calypso/apps/editing-toolkit/editing-toolkit-plugin/wpcom-block-editor-nux/src/store.js Line 27 in bf02435
In the end, I did other hacky things to make dev-ing easier. Final testing should be done on new user. |
Rather than creating a new event for each page, we could fire one event and sent different data along with it. Similar to the example PR from @razvanpapadopol. For example
Not only is it tidier, but it makes data analysis easier if we're looking at and filtering data for one event, and not The
🤔 I think what you've done it fine. A wild guess is that something is cached. Not sure where though... maybe even in your browser if it has cached the response.
Don't fear the power of the hacky side! Thanks for writing up what you did. It will make testing a lot easier! 🙇 |
Yes! Actually I used Razvan's code! I just forgot to update the PR description to reflect that slides are tracked by passing the slide number and is_last_slide as data. Credit for Thanks everyone for helping me with this! |
Oh! I will look at the code next time too. Sorry! 😇 |
…ast_slide' props.
75a160f
to
9adc40b
Compare
✅ Event fire when wpcom Nux loads and first slide is shown:
and
✅ Navigating through the slide fires
✅ Navigating to last slide sets
✅ Closing Nux modal by either clicking on "Get started" or on the page body triggers
By the way, I think I found a way to test without worrying about user meta or attributes: clear WP_DATA_USER AND WP_LAUNCH as you mentions, then to set a default value of |
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.
Nice work!
LGTM
It's not related to this PR since it's happening on production too, but the modal isn't translated even though most of the strings exist, e.g., Just adding a note here because I noticed while testing. Have put it on the TODO list over at #37665 |
@@ -11,6 +11,7 @@ import { useDispatch, useSelect } from '@wordpress/data'; | |||
import { useEffect } from '@wordpress/element'; | |||
import { __ } from '@wordpress/i18n'; | |||
import { registerPlugin } from '@wordpress/plugins'; | |||
import { recordTracksEvent } from '@automattic/calypso-analytics'; |
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.
Does this work on Atomic sites? I have no idea, just thought it would be worth bringing up just in case something is weird there
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.
Hiya @noahtallen Why do you think this wouldn't work on Atomic sites? You probably have some valuable insight that I haven't yet mastered. 😄 Is there a known issue with analytics tracking on Atomic sites?
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.
Haha, I don’t have a specific reason. I’m just used to things not working exactly the same on atomic so it’s always worth checking! I mentioned this because it seems like the sort of library that could theoretically rely on a connection to wpcom or mission control, which is one of the areas that can be different between the two platforms!
@ramonjd and/or @noahtallen Looking for input on something I just learned: I just learned that the NUX modal can be triggered from the three vertical dot menu. Should we pass additional paramaters to indicate when/how the NUX modal is being viewed? This would only apply to the NuxPage tracking and we could track
I'll create a new issue if we need to be tracking additional data for the NuXPage tracking. |
TIL 🎉
@autumnfjeld Great point! I think it definitely makes sense to pass a parameter to tell us that it hasn't popped up after site creation. We already have This is just a suggestion, but we could add
|
Agreed on the tracking! Some more notes about it:
|
Added an issue #47257
Hmm, that sounds like it's a convenient bug? 😆 Takes the "N" out of NUX... |
I think it's deliberate, and it works out since you would probably close it right away, which disables it again :P |
@Automattic/ganon |
Changes proposed in this Pull Request
calypso_editor_wpcom_nux_open
calypso_editor_wpcom_nux_dismiss
calypso_editor_wpcom_nux_slide_view
is_last_slide
Testing instructions
sync Editing Toolkit to your sandbox
create new user & new site -or- see below for instructions to reset nux status for exisiting user
View NUX model opening in the Editor
Verify tracking network request (see screenshot) for each of the 3 tracking events. The last event will fire for each of the 4 slides with data to track slide number.
calypso_editor_wpcom_nux_open
calypso_editor_wpcom_nux_dismiss
calypso_editor_wpcom_nux_slide_view
(fired on each slide)- note the data passed for each slide:
slide_number
is_last_slide
.Hover over the network request to show the full tracking event label
🚧 This didn't work reliably every time for me. To try reset your nux status to
enabled
so that NUX dialog shows on existing account:reset nux status in your sandbox
clear local storage: both WP_DATA_USER AND WP_LAUNCH as shown in screenshot
Fixed #47257