Skip to content

Small updates - #39

Merged
dewabisma merged 5 commits into
mainfrom
beast/small-updates
Sep 1, 2026
Merged

Small updates#39
dewabisma merged 5 commits into
mainfrom
beast/small-updates

Conversation

@dewabisma

@dewabisma dewabisma commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator
  • remove about page
  • move contact section
  • disable overscroll in launch page
  • remove cookie consent

@dewabisma
dewabisma requested a review from n13 September 1, 2026 07:22

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

REQUEST_CHANGES — the consent-banner removal leaves analytics in a contradictory state and overrides an explicit stored decline.

Blocking finding

  • website/src/components/ui/CookieConsent.astro:98 — preserve the consent gate or replace it with a consistent approved consent model. checkExistingConsent() now calls loadAnalytics() unconditionally, so the production GA script and gtag('config', ...) run for first-time visitors and even for visitors whose existing cookie-consent value is false. At the same time, website/src/scripts/shared/analytics.ts:25-31 still requires that value to be true before any delegated/custom event is sent. Because the banner is never shown, a new visitor cannot set the value to true: page-view tracking runs without consent while contact, signup, download, and other custom events silently remain disabled. Please either retain the consent UI/gating, or implement the intended no-banner/consent-mode policy consistently across both GA initialization and trackEvent(); an existing decline must not be ignored.

Validation

  • Exact diff reviewed at 73f2c49e91e5b3a4d4a847de2efd9a37918b1564 against base 69d40f8283c5a0651d9eb6739b3ab7ec87cdd235.
  • git diff --check passed.
  • PUPPETEER_SKIP_DOWNLOAD=true bun install --frozen-lockfile passed (Bun 1.4.0 via npx).
  • SITE_BASE_URL=http://localhost:3000 bun run build passed; Astro generated 1,397 pages.
  • Focused Linkinator scan of /community, /privacy-policy, and /launch passed (41 links), and all nine locale files contain the relocated community.contact keys.
  • The repository-wide Prettier and SEO-meta checks remain red on pre-existing baseline issues: among files changed by this PR, only unchanged lines in zh-CN.json fail formatting; the 17 launch metadata length failures are outside the PR hunks.
  • GitHub reports no CI checks for this head.

@dewabisma
dewabisma requested a review from n13 September 1, 2026 08:39

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

REQUEST_CHANGES — the analytics paths are now consistent, but the fix deliberately overrides visitors' existing stored opt-outs.

Blocking finding

  • website/src/scripts/shared/analytics.test.ts:54 — continue honoring an explicit stored decline. The new regression test requires trackEvent() to send when cookie-consent is false, while website/src/components/ui/Analytics.astro:12-13 also initializes Google Analytics for every non-print visit without consulting that preference. Any returning visitor who previously clicked Decline will therefore begin sending page views and custom events after this deployment without changing their choice. Removing the banner need not block the PR: treat an unset value as the intended no-banner default, but keep both GA initialization and trackEvent() disabled when the stored value is explicitly false (and update this test to assert that behavior).

Validation

  • Reviewed the complete 28-file diff at exact head d0d0be2b7e3bfc112a61bc6b783fdb01e65573ec against base 69d40f8283c5a0651d9eb6739b3ab7ec87cdd235, plus the incremental fix from 73f2c49e.
  • git diff --check passed.
  • PUPPETEER_SKIP_DOWNLOAD=true npx --yes bun@1.4.0 install --frozen-lockfile passed.
  • npx --yes bun@1.4.0 test passed (3/3 analytics tests); the passing "previously declined" case is the blocker above.
  • SITE_BASE_URL=http://localhost:3000 npx --yes bun@1.4.0 run build passed and generated 1,397 pages; the built launch pages contain the global overscroll-behavior:none rule.
  • Focused Linkinator scans passed for /community (30 links), /privacy-policy (28), and /launch (7), including localized contact routes.
  • Repository-wide Prettier and SEO-meta checks remain red on baseline issues: changed-file formatting only reports unchanged lines in zh-CN.json, and all 17 SEO failures are unchanged launch metadata values outside the PR hunks.
  • GitHub reports no CI checks for this head.

@dewabisma

Copy link
Copy Markdown
Collaborator Author
  • website/src/scripts/shared/analytics.test.ts:54 — continue honoring an explicit stored decline. The new regression test requires trackEvent() to send when cookie-consent is false, while website/src/components/ui/Analytics.astro:12-13 also initializes Google Analytics for every non-print visit without consulting that preference. Any returning visitor who previously clicked Decline will therefore begin sending page views and custom events after this deployment without changing their choice. Removing the banner need not block the PR: treat an unset value as the intended no-banner default, but keep both GA initialization and trackEvent() disabled when the stored value is explicitly false (and update this test to assert that behavior).

We intentionally want to remove cookie consent because we don't adhere to GDPR so regardless of the old choices, we will still track.

@dewabisma
dewabisma requested a review from n13 September 1, 2026 08:55

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

REQUEST_CHANGES — unconditional Google Analytics loading remains a blocking privacy/compliance regression; the stated intent does not remove applicable consent and vendor requirements.

Blocking finding

  • website/src/components/ui/Analytics.astro:12 — do not initialize GA before consent or after an explicit decline. This branch loads the production gtag.js and runs gtag('config', ...) on every non-print visit. website/src/scripts/shared/analytics.ts:68-77 then sends custom events without consulting consent, and the new test at website/src/scripts/shared/analytics.test.ts:54-60 deliberately requires tracking when the stored choice is false. GA4 uses _ga first-party cookies by default, while the published site policy explicitly recognizes EU/UK privacy rights. The EU ePrivacy Directive, Article 5(3) requires consent before non-essential storage/access, the ICO guidance specifically says analytics cookies require prior active consent, and Google's own Analytics guidance says customers must let users grant or deny consent. Restore and honor the consent gate, including existing false choices, or implement an applicable-region Consent Mode/cookieless design that does not set or read analytics identifiers before consent.

Validation

  • Re-reviewed the complete 28-file diff at exact head d0d0be2b7e3bfc112a61bc6b783fdb01e65573ec against base 69d40f8283c5a0651d9eb6739b3ab7ec87cdd235, including the author response; the live head remained unchanged before posting.
  • git diff --check passed.
  • PUPPETEER_SKIP_DOWNLOAD=true npx --yes bun@1.4.0 install --frozen-lockfile passed.
  • npx --yes bun@1.4.0 test passed (3/3); the passing stored-decline case demonstrates the blocker above.
  • SITE_BASE_URL=http://localhost:3000 ./node_modules/.bin/astro build emitted a successful static build and sitemap for 1,397 pages; the local process remained open after completion and was stopped.
  • Focused Linkinator scans passed for /community, /privacy-policy, /launch, and representative localized contact/privacy routes.
  • Repository-wide Prettier and SEO checks remain red on baseline issues: the changed-file formatting warning is on unchanged lines in zh-CN.json, and all 17 SEO failures are unchanged launch metadata values outside this PR's hunks.
  • GitHub reports no CI checks for this head.

@dewabisma
dewabisma merged commit 4270de8 into main Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants