-
Notifications
You must be signed in to change notification settings - Fork 0
fix(Codebytes): tracking tweaks for monolith #31
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
Changes from all commits
0b58775
716a28a
1ab8e74
a89f562
e8b7dc2
03a0b04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| import { UserClickData } from '@codecademy/tracking'; | ||
|
|
||
| import { trackUserClick } from '../libs/eventTracking'; | ||
|
|
||
| export type CodebyteOptions = { | ||
|
|
@@ -27,7 +29,16 @@ export const getOptions = (): CodebyteOptions => { | |
| }; | ||
| }; | ||
|
|
||
| export const trackClick = (target: string) => { | ||
| export const trackClick = ( | ||
| target: string, | ||
| trackingData?: Omit<UserClickData, 'target'> | ||
| ) => { | ||
| if (trackingData) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we make comment about tech debt ticket here/mention homepage?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 100% good call. just wanted to see if you and @BandanaKM were okay with this stopgap approach
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, think this works as a quick way to pass in the correct tracking data from monolith to avoid getting the weird values for we can definitely refactor this as a fast follow. ideally we can get rid of this file in the future and just have getOptions and all the relevant dependencies in the next.js version, since they were primarily intended for the query params if we do end up passing in an object later with the tracking functions, we can definitely do that |
||
| return trackUserClick({ | ||
| ...trackingData, | ||
| target, | ||
| }); | ||
| } | ||
| const options = getOptions(); | ||
| const page_name = options.renderMode | ||
| ? `${options.clientName}_${options.renderMode}` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| export * from './codeByteEditor'; | ||
| export { LanguageOption, LanguageOptions } from './consts'; | ||
| export * from './consts'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was blocking us from being able to import in the monolith for some reason, so we're just gonna do this to unblock |
||
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 catch