Skip to content

fix(Codebytes): tracking tweaks for monolith#31

Merged
borisonr merged 6 commits intomainfrom
rb-tracking-tweaks
Feb 3, 2022
Merged

fix(Codebytes): tracking tweaks for monolith#31
borisonr merged 6 commits intomainfrom
rb-tracking-tweaks

Conversation

@borisonr
Copy link
Copy Markdown
Contributor

@borisonr borisonr commented Feb 2, 2022

Overview

allow non iframe usages to pass in tracking data

this is necessary for importing and using the codebytes component in the monolith and having appropriate tracking data (since there won't be an iframe url to pull params from)

this is a stopgap for now and will be revisited in disc - 465 to think through a better solution

PR Checklist

  • Related to JIRA ticket: DISC-409
  • I have run this code to verify it works
  • This PR includes unit tests for the code change

Copy link
Copy Markdown

@zippyzow zippyzow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

trackClick('lang_select');
trackClick('lang_select', trackingData);
onLanguageChange?.(newText, newLanguage);
trackClick('lang_select');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

target: string,
trackingData: Omit<UserClickData, 'target'>
) => {
if (trackingData) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we make comment about tech debt ticket here/mention homepage?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 clientName in all of our userClick events in the monolith.

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

@borisonr borisonr changed the title trackign tweaks for monolith tracking tweaks for monolith Feb 3, 2022
@borisonr borisonr changed the title tracking tweaks for monolith fix(Codebytes): tracking tweaks for monolith Feb 3, 2022
@borisonr borisonr marked this pull request as ready for review February 3, 2022 18:29
@borisonr borisonr requested a review from a team as a code owner February 3, 2022 18:29
@borisonr borisonr requested review from a team, BandanaKM, cstatro, jakemhiller and thedoomshine and removed request for a team February 3, 2022 18:29
@@ -1,2 +1,2 @@
export * from './codeByteEditor';
export { LanguageOption, LanguageOptions } from './consts';
export * from './consts';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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

@codecademydev
Copy link
Copy Markdown
Contributor

📬Published Alpha Packages:

@codecademy/codebytes@0.6.5-alpha.03a0b0.0

@borisonr borisonr merged commit 4d309d0 into main Feb 3, 2022
@borisonr borisonr deleted the rb-tracking-tweaks branch February 3, 2022 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants