Add files for Amplitude events in the navbar Create menu #56319
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| import $ from 'jquery'; | ||
|
|
||
| $(document).ready(function () { | ||
| console.log('User header event logger'); |
There was a problem hiding this comment.
Wait, won't this log to the console on every page? (that's not a big deal tbh, but curious because the description says it doesn't?)
There was a problem hiding this comment.
Oh it won't log on any page because the haml view user_header_event_logger isn't in the user header (that will be added in the next PR). Currently, this is just a pair of haml/js files that aren't being used anywhere yet.
bethanyaconnor
approved these changes
Feb 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My first attempt at adding click events to the "Create" menu in the navbar seemed to work locally, but once it merged there were HoneyBadger errors on staging relating to
webpack_asset_name. Since the navbar is used all across our site, I was afraid it would break something so I rushed to revert. However, after having a conversation with Elijah, it seems like it's possible that if I had let it play out after it finished building, it would've used the newly-built package and worked.So, following Elijah's advice, I'm using this PR to create the files themselves without actually implementing them into the user navbar so that if anything goes wrong it won't cause errors on our pages. Currently, I'm just having it console log a message so that there's something in the
jsfile (even though it won't be shown anywhere). Once I can confirm this is working, I'll set up the 2nd PR which will actually implement the event logger.Links
Jira ticket: here
Previous attempt: here
Revert of attempt: here
Slack conversation with Elijah: here
Testing story
While the event logger is not being implemented anywhere yet, I was able to confirm that adding it into the
user_header.hamlfile showed the console log I would expect locally (I then deleted that line before committing this):Deployment strategy
I'm going to merge this after a DTP so I can monitor #infra-staging and HoneyBadger for errors. Then, if anything breaks there's plenty of time to debug and/or revert.
Follow-up work
Actually implement the event logger into the user navbar.