-
Notifications
You must be signed in to change notification settings - Fork 88
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
[WNMGDS-3205] Adds analytics tracking to theme & version dialogs #3424
[WNMGDS-3205] Adds analytics tracking to theme & version dialogs #3424
Conversation
…WNMGDS-3205-add-analytics-to-theme-switcher
@@ -15,6 +16,11 @@ export const ThemeVersionDialog = (props: ThemeVersionDialogProps) => { | |||
|
|||
function handleUpdate() { | |||
if (version !== props.version) { | |||
let filterCategoriesUsed = { theme: props.theme, version: version }; |
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.
This is super close! In trying this out, there's one case that we won't capture which is if people select the latest version from the dropdown. We know that the latest version is the one shown by default, but users might not get that, and it could be helpful to see when users happen to select the latest version. I think moving these lines outside of the if
statement to the beginning of the handle update would be a good way to handle this.
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.
Ah, I see what you mean. Will update!
@@ -16,6 +17,10 @@ export const ThemeVersionDialog = (props: ThemeVersionDialogProps) => { | |||
function handleUpdate() { | |||
if (theme !== props.theme) { | |||
setQueryParam('theme', theme, true); | |||
|
|||
let filterCategoriesUsed = { theme: theme }; |
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 we want to move these added lines to outside of the if check. We want to capture when users stay on the currently selected theme. Also, is there a way to derive the version here too? Maybe by looking at the url and by using the getVersionEquivalent
function?
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.
Love this idea! Instead of deriving the fromVersion
argument for getVersionEquivalent
from the URL, I think it might make more sense to expand the props on the theme dialog component so that it receives the version prop from the parent—just like the version dialog. Let me know what you think!
…o cover cases where version or theme remains the same.
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.
Pulled this down locally and was able to follow the steps to verify the payload data using the window.utag.link = console.log
step
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.
👌🏻
Summary
sendFilterAppliedEvent
and integrates it into the theme and version switcher dialogs on the docsite.Jira ticket
How to test
npm run start
window.utag.link = console.log
to be able to intercept the analytics events and see them printed to the console.For switching versions you should see something like this:
For switching themes you should see something like this:
Checklist
[WNMGDS-####] Title
or [NO-TICKET] if this is unticketed work.Type
(only one) label for this PR, if it is a breaking change, label should only beType: Breaking
Impacts
, multiple can be selected.