Conversation
Member
|
also tagging @gifale95 |
mschrimpf
approved these changes
Mar 13, 2026
Member
mschrimpf
left a comment
mschrimpf
left a comment
There was a problem hiding this comment.
looks good, but please move javascript outside of the html
Comment on lines
+321
to
+353
| var bergLink = document.getElementById('berg-link'); | ||
| if (bergLink) { | ||
| bergLink.addEventListener('click', function() { | ||
| gtag('event', 'click', { | ||
| event_category: 'BERG', | ||
| event_label: 'main_link', | ||
| transport_type: 'beacon' | ||
| }); | ||
| }); | ||
| } | ||
| document.addEventListener('click', function(e) { | ||
| var link = e.target.closest('.tooltip-multiline a'); | ||
| if (!link) return; | ||
| var href = link.getAttribute('href'); | ||
| var label = 'unknown'; | ||
| if (href.indexOf('gifale95.github.io/BERG') !== -1) label = 'tooltip_berg'; | ||
| else if (href.indexOf('readthedocs') !== -1) label = 'tooltip_documentation'; | ||
| else if (href.indexOf('drive.google.com') !== -1) label = 'tooltip_tutorial'; | ||
| gtag('event', 'click', { | ||
| event_category: 'BERG', | ||
| event_label: label, | ||
| transport_type: 'beacon' | ||
| }); | ||
| }); | ||
| document.querySelectorAll('.berg-box-link').forEach(function(link) { | ||
| link.addEventListener('click', function() { | ||
| gtag('event', 'click', { | ||
| event_category: 'BERG', | ||
| event_label: this.getAttribute('data-berg-label'), | ||
| transport_type: 'beacon' | ||
| }); | ||
| }); | ||
| }); |
Member
There was a problem hiding this comment.
why not put this in the javascript? bad practice imo to mix this in the html.
Contributor
Author
There was a problem hiding this comment.
You're right. Didn't want to create a single javascript file for berg tracking. I'll make a generic link-tracking utility.
Member
There was a problem hiding this comment.
it's totally fine to put this in a single javascript file too. The website is set up to compress all javascript files into a single one before deployment, so no need to worry about network bandwidth or so. (Only worry about writing clean code ;-))
…erty ID (sunset July 2024)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Domain specific links redirect to appropriate external links. Includes both box and tooltip option. Tooltip option currently disabled. To re-enable tooltip, remove comment wrapper and rename tooltip attribute. See inline comment in line 266 (model.html).