-
Notifications
You must be signed in to change notification settings - Fork 246
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
docs: update to latest hugo-theme-techdoc #1843
Conversation
be63dbb
to
ff7a35a
Compare
✔️ Deploy Preview for brigade-docs ready! 🔨 Explore the source changes: 2efb18b 🔍 Inspect the deploy log: https://app.netlify.com/sites/brigade-docs/deploys/62182d21c49429000868490f 😎 Browse the preview: https://deploy-preview-1843--brigade-docs.netlify.app |
ff7a35a
to
15ca3a4
Compare
15ca3a4
to
0baf949
Compare
{{- partial "custom-css.html" . -}} | ||
{{- template "_internal/opengraph.html" . -}} | ||
{{- template "_internal/twitter_cards.html" . -}} | ||
{{- template "_internal/schema.html" . -}} |
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.
This file is copied from the theme and minimally modified to include include our favicon and custom stylesheet. Hugo favors this file over the original in themes/...
2c0f6f8
to
f6e0407
Compare
@@ -1,30 +1,30 @@ | |||
{{$currentNode := .}} |
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.
This file is copied from the theme and minimally modified to favor using each page's titleLink
over its title
when generating the menu. Hugo favors this file over the original in themes/...
@@ -94,3 +68,6 @@ | |||
}); | |||
}; | |||
</script> | |||
<!-- END: Google Analytics --> | |||
{{- end -}} | |||
{{- end -}} |
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.
This file is copied from the theme and minimally modified to require users consent to the use of cookies before Google Analytics is activated. Hugo favors this file over the original in themes/...
This is consistent with the behavior prior to this PR. The only difference is that the hack is now part of our site code rather than a modification to the theme.
@@ -0,0 +1 @@ | |||
<!-- This is deliberately empty to override something we don't want from the theme. --> |
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.
This overrides the file of the same name found in themes/...
The original uses a <noscript>
element to activate Google Analytics in an invisible <iframe> when JS is disabled in the user's browser.
We don't want that because we don't want to activate Google Analytics unless the user explicitly consents to the use of tracking cookies, and we cannot gather that consent without JS enabled, so we simply have to live without GA in this case.
d1b0b84
to
35e3d23
Compare
Signed-off-by: Kent Rancourt <kent.rancourt@microsoft.com>
35e3d23
to
2efb18b
Compare
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.
Rather than going through all of the changed files in detail, I checked out the deploy preview and all looks great to me.
It sounds like this changeset gets us up-to-date and the internal doc workings vastly cleaned up, which all sounds excellent.
Noticed some notes around Google Analytics -- I'll defer to @krancour to make sure all is working as expected.
Otherwise, LGTM from me!
@vdice it is for sure working identically to before. Using the browser's developer tools, I was able to compare before/after and verify that Google Analytics is now incorporated into the site only after a user explicitly consents to cookies -- same as before. The only thing that's up in the air is that I'm missing access to view the collected data, but we'll work that out. |
Signed-off-by: Kent Rancourt <kent.rancourt@microsoft.com>
Fixes #1385
This rips out our Frankenstein customizations to the Hugo Techdoc theme and replaces it with the latest version using a submodule. This is apparently how themes are meant to be used.
I've updated our customizations to work with recent changes to the theme. #1385 is fixed in the process. Those customizations are all applied outside the
themes/
folder as is recommended.