feat: pass Google Analytics ID as Sphinx config parameter - #55
Conversation
There was a problem hiding this comment.
Pull request overview
Moves Google Analytics initialization into the document head while synchronizing consent handling and adding Sphinx configuration.
Changes:
- Adds SSR/head analytics bootstrap and consent-mode updates.
- Adds Sphinx analytics options and demo build flags.
- Updates documentation, tests, dependencies, and release configuration.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Reviewed change | Final review status |
|---|---|---|
sphinx-ui/README.md |
Documents analytics configuration. | No final comment |
sphinx-ui/react/tsdown.config.ts |
Removes build-time GA substitution. | No final comment |
sphinx-ui/react/src/injectNav.tsx |
Reads analytics settings from HTML. | No final comment |
sphinx-ui/react/src/globals.d.ts |
Removes obsolete GA declaration. | No final comment |
sphinx-ui/react/package.json |
Updates dependencies and tooling. | No final comment |
sphinx-ui/quantinuum_sphinx/theme.toml |
Adds analytics theme options. | No final comment |
sphinx-ui/quantinuum_sphinx/page.html |
Adds head bootstrap and data attributes. | Moderate, 2 votes: The initial page_view is queued while analytics is denied; seed the default with analyticsGranted or update before config.Moderate, 3 votes: Inline validation accepts incomplete consent; validate the complete schema or default to denied. |
sphinx-ui/build-demo.sh |
Adds analytics build options. | Moderate, 3 votes: The Sphinx -D keys do not override nested theme options, so the analytics flags and ID do not work; pass a complete html_theme_options value or use a temporary override. |
documentation-ui/src/custom/docs/components/gdpr/_components/CookieSettingsDialog/CookieSettingsDialog.tsx |
Adds callback typing. | No final comment |
documentation-ui/src/custom/docs/components/analytics/index.ts |
Exports the head component. | No final comment |
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsWithConsent.tsx |
Updates analytics documentation. | No final comment |
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsWithConsent.test.tsx |
Updates consent tests. | No final comment |
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsHead.tsx |
Adds SSR analytics bootstrap. | Moderate, 2 votes: Validation accepts incomplete consent and may grant analytics before the provider rejects it; validate the complete schema or deny incomplete values. This also applies on line 42. |
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsHead.test.tsx |
Tests head markup and CSP nonce handling. | No final comment |
documentation-ui/src/custom/docs/components/analytics/consent-mode.ts |
Exports shared helpers and adjusts defaults. | No final comment |
documentation-ui/src/custom/docs/components/analytics/consent-mode.test.ts |
Updates consent-mode tests. | No final comment |
documentation-ui/src/custom/docs/components/analytics/bootstrap.ts |
Updates fallback initialization. | Moderate, 2 votes: loadGoogleAnalytics queues config before updating the persisted grant, causing the first page_view to use denied consent; seed the default with analyticsGranted or update first. |
documentation-ui/src/custom/docs/components/analytics/bootstrap.test.ts |
Tests persisted consent handling. | No final comment |
documentation-ui/README.md |
Documents head integration. | No final comment |
documentation-ui/package.json |
Refreshes dependencies. | No final comment |
.github/workflows/release.yml |
Removes obsolete GA build configuration. | No final comment |
Suppressed comments (3)
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsHead.tsx:42
- The generated head script does not use
analyticsGranteduntil afterconfig, so a returning visitor's first automaticpage_viewis queued withanalytics_storagedenied. Apply the stored grant in the default state or move the update beforeconfig; otherwise the new parser-time integration does not provide a consented first hit.
window.gtag('consent', 'default', ${deniedConsent});
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsWithConsent.tsx:17
- This JSDoc says bootstrap seeds its default from the stored consent, but
bootstrapGoogleAnalyticsnow always callssetGoogleConsentDefault()with no override and sends the stored value only as an update. Describe the denied-default/update sequence so consumers do not infer that the first config uses the stored grant.
* GA is always loaded. Bootstrapping seeds the default from the valid stored
* decision. Later consent changes upgrade or downgrade GA accordingly.
sphinx-ui/build-demo.sh:45
analytics_idis interpolated intoSPHINXOPTS, which is expanded unquoted bydemo/Makefilewhen it invokes Sphinx. An input such asG-foo;...or$(...)therefore becomes shell syntax duringmake, rather than just an ID. Validate--analytics-idagainst the GA4 measurement-ID format before appending it.
if [[ -n "$analytics_id" ]]; then
sphinx_options+=" -D html_theme_options.analytics_id=$analytics_id"
fi
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsHead.test.tsx:31
- This test only searches the generated HTML and never executes the parser-time bootstrap. The new consent-cookie parsing, queued default/update/config commands, and dynamic script insertion therefore have no runtime coverage; a regression in any of those paths would still pass. Add a jsdom execution test covering no cookie, a valid Analytics grant, and an invalid/stale cookie.
it('renders an ordered parser-time bootstrap', () => {
const markup = renderToStaticMarkup(<GoogleAnalyticsHead gaId={TEST_GA_ID} />)
const consentDefaultIndex = markup.indexOf("gtag('consent', 'default'")
const consentUpdateIndex = markup.indexOf("gtag('consent', 'update'")
const configIndex = markup.indexOf("gtag('config', gaId)")
const loaderIndex = markup.indexOf("document.createElement('script')")
const appendIndex = markup.indexOf('document.head.appendChild(script)')
expect(consentDefaultIndex).toBeGreaterThanOrEqual(0)
expect(consentUpdateIndex).toBeGreaterThan(consentDefaultIndex)
expect(configIndex).toBeGreaterThan(consentUpdateIndex)
expect(loaderIndex).toBeGreaterThan(configIndex)
expect(appendIndex).toBeGreaterThan(loaderIndex)
expect(markup).toContain(`data-measurement-id="${TEST_GA_ID}"`)
expect(markup).toContain('https://www.googletagmanager.com/gtag/js?id=')
expect(markup).toContain('dateConsentWasGiven')
expect(markup).toContain('consentCategories.Essential')
expect(markup).toContain(String.raw`/^\d{4}-\d{2}-\d{2}T`)
expect(markup).not.toContain('wait_for_update')
documentation-ui/src/custom/docs/components/analytics/GoogleAnalyticsHead.tsx:46
- These tests only inspect the emitted JavaScript text; they never execute the bootstrap. Consequently, regressions in cookie decoding/version validation, consent command ordering at runtime, or dynamic script insertion can pass while this feature is broken. Add a jsdom/browser-level test covering absent, valid-grant, invalid, and obsolete cookies and the resulting dataLayer/script state.
var storedConsent = JSON.parse(decodeURIComponent(value));
var consentDate = storedConsent && storedConsent.dateConsentWasGiven;
var consentCategories = storedConsent && storedConsent.consentCategories;
analyticsGranted = storedConsent !== null
&& typeof storedConsent === 'object'
&& storedConsent.consentVersion === ${consentVersion}
&& typeof consentDate === 'string'
&& /^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{3}Z$/.test(consentDate)
&& !Number.isNaN(Date.parse(consentDate))
&& consentCategories !== null
&& typeof consentCategories === 'object'
&& typeof consentCategories.Essential === 'boolean'
&& typeof consentCategories[${analyticsCategoryName}] === 'boolean'
&& storedConsent.consentCategories[${analyticsCategoryName}] === true;
documentation-ui/src/custom/docs/components/analytics/bootstrap.ts:38
CookieConsentProvideraccepts a caller-supplied consentversion, but this bootstrap validates against the package constant instead. If a consumer usesversion={2}, a validAnalytics: truecookie is treated as obsolete, so the fallback queues analytics as denied (and may delete the cookie); the head variant has the same mismatch before hydration. Pass the provider's version through the analytics API or centralize both components on the same version.
const analyticsGranted =
isConsentSetInCookies(COOKIES_CONSENT_VERSION) &&
retrieveConsentCategoriesFromCookies()[CookieCategoryName.Analytics]
sphinx-ui/build-demo.sh:46
html_theme_optionsis a single dict-valued Sphinx config setting; dotted names such ashtml_theme_options.enable_analyticsare not recognized by Sphinx's-Doverride parser. As a result,--analytics(and the ID override below) leaves the theme option at its defaultfalse, so the documented demo opt-in does not enable analytics. Pass a completehtml_theme_optionsmapping or set the values through the democonf.pyinstead.
sphinx_options="-D html_theme_options.enable_analytics=true"
No description provided.