-
Notifications
You must be signed in to change notification settings - Fork 8
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
Adds dap #792
Adds dap #792
Conversation
Co-authored-by: Curtis Mitchell <[email protected]>
Co-authored-by: Curtis Mitchell <[email protected]>
Co-authored-by: Curtis Mitchell <[email protected]>
Co-authored-by: Curtis Mitchell <[email protected]>
Co-authored-by: Curtis Mitchell <[email protected]>
Co-authored-by: Curtis Mitchell <[email protected]>
Adds DAP to site
_includes/scripts.html
Outdated
<script src="{{ site.baseurl }}/assets/js/styleguide.js"></script> | ||
<script async id="_fed_an_ua_tag" src="https://dap.digitalgov.gov/Universal-Federated-Analytics-Min.js?agency=DOC&subagency=CEN" type="text/javascript"></script> |
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 this should be moved to head.html
so that it's loaded within the head
HTML tags.
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.
@curt-mitch-census I think I fixed this issue and would appreciate input!
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.
@sadiejay Sorry, I should have been more specific: you were right to move the DAP script to the head.html
, according to their suggested documentation. However, I think you can keep the styleguide.js
file within the scripts.html
file along with the uswds.min.js
file to keep the non-analytics JavaScript files in one place. My apologies for the mixup!
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.
@curt-mitch-census
Thank you for catching this! Will move these to head!
Co-authored-by: Curtis Mitchell <[email protected]>
Co-authored-by: Curtis Mitchell <[email protected]>
Co-authored-by: Curtis Mitchell <[email protected]>
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 left some small comments on the location of the script
tag for the styleguide.js
file, otherwise this LGTM! 👍
No description provided.