Skip to content

feat: add GDPR cookie consent #161

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.

Sign up for GitHub

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

Open
Kouteki wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add GDPR cookie consent #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions components/CookieConsent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import React, { useEffect, useState } from "react";

const STORAGE_KEY = "allora-cookie-consent";
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[medium] No schema version on the stored consent value

The stored value is a bare string ("accepted" / "declined"). Any future change — adding granular categories (analytics/functional/marketing), moving to IAB TCF v2, distinguishing per-region — will need to migrate or invalidate existing entries, and the existence-only check at line 11 (!getItem(STORAGE_KEY)) means there's no way to detect a legacy entry and re-prompt.

Suggested fix: Store an object with a version tag, e.g.:

const STORAGE_KEY = "allora-cookie-consent";
const SCHEMA_VERSION = 1;
// write:
window.localStorage.setItem(STORAGE_KEY, JSON.stringify({ v: SCHEMA_VERSION, choice }));
// read:
const raw = window.localStorage.getItem(STORAGE_KEY);
const entry = raw ? JSON.parse(raw) : null;
const valid = entry && entry.v === SCHEMA_VERSION && (entry.choice === "accepted" || entry.choice === "declined");
if (!valid) setVisible(true);

Why: Forward-compatibility — when this banner inevitably evolves, the schema check lets the new version re-prompt users with the old entry rather than silently treating it as valid.


function CookieConsent() {
const [visible, setVisible] = useState(false);

useEffect(() => {
if (typeof window === "undefined") return;
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[low] Redundant typeof window === "undefined" guard inside useEffect

useEffect only fires on the client — it is never invoked during server rendering. This guard is dead code (3 dimensions independently flagged it).

Suggested fix:

useEffect(() => {
  try {
    if (!window.localStorage.getItem(STORAGE_KEY)) {
      setVisible(true);
    }
  } catch (e) {
    setVisible(true);
  }
}, []);

Why: Removes a no-op check and tightens the effect body.

try {
if (!window.localStorage.getItem(STORAGE_KEY)) {
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[high] Consent banner is write-only — nothing reads the stored preference

4 review dimensions independently flagged this. A repo-wide grep for gtag, google-analytics, googletagmanager, posthog, mixpanel, segment, _paq, fbq, document.cookie, and <script src= returns zero matches. There is no _app.tsx or _document.tsx injecting analytics, and next.config.js does not configure any tracking. The only data-transmitting feature in the site is the AiButton/chat component, and it fires on explicit user interaction — not passive tracking.

This means the stored "accepted" / "declined" value is never read by any consumer. The Decline button is a no-op from a GDPR enforcement standpoint, and the Accept button merely silences a banner that wasn't gating anything in the first place. Recording a consent decision without any processing for it to govern arguably creates more legal exposure than not having a banner at all, because it produces a misleading audit trail.

Suggested resolution: Either delete this PR (the site has nothing that needs consent), or land it together with (a) the analytics/tracking integration that the banner is supposed to govern, and (b) gating code that actually checks the stored value before initialising those scripts. Recommend keeping the work on a branch until the second half is ready.

Why: A consent banner with no enforcement is theatrical compliance — under GDPR scrutiny it can be worse than no banner.

[medium] Existence-only check passes any truthy value

Separately, !window.localStorage.getItem(STORAGE_KEY) only detects "never asked" vs "any value at all". A stale value from a future schema ("v2:accepted"), a manually-edited entry, or even a corrupted blob silently bypasses the banner. Combine with the schema-version suggestion above and validate the read against the expected shape.

setVisible(true);
}
} catch (e) {
setVisible(true);
}
}, []);

const respond = (choice) => {
try {
window.localStorage.setItem(STORAGE_KEY, choice);
} catch (e) {}
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[medium] Silent localStorage.setItem failure creates infinite banner loop

4 dimensions agreed on this path: when localStorage.setItem throws (Safari ITP edge cases, content blockers, quota exceeded, some embedded browsers), the empty catch (e) {} swallows the error and setVisible(false) runs anyway. On the next mount, getItem returns null (or throws), the fallback branch shows the banner again, and the cycle repeats every page load. The user can never permanently dismiss it.

The consent record is also lost — the user thinks they declined but the preference is gone.

Suggested fix: Only hide the banner if persistence succeeded; otherwise fall back to an in-memory flag so at least the current session is quiet:

let sessionConsent = null; // module scope, outside the component

const respond = (choice) => {
  let persisted = false;
  try {
    window.localStorage.setItem(STORAGE_KEY, choice);
    persisted = true;
  } catch (e) {
    // Storage blocked (private mode, quota, ITP). Track for this session only.
    sessionConsent = choice;
    // Optional: surface a one-time message that the choice could not be saved.
  }
  setVisible(false);
};

And in the read effect, also consult sessionConsent before defaulting to setVisible(true).

Why: Eliminates the private-mode infinite-banner regression while keeping the silent-failure case observable internally.

[low] Empty catch (e) {} provides no observability

Even if the silent failure stays, the unused binding can use the bare form catch {} (ES2019+) and a short comment explaining the intent. Right now there is no log, no metric, and no way for the developer to know the write failed.

[low] respond(choice) accepts free-form string with no validation

The only callers pass "declined" or "accepted", but the function will happily persist any string a future caller passes (e.g. "maybe", "", "yes"). Combined with the existence-only read check, a malformed value silently bypasses the banner forever. Constrain at the boundary:

const respond = (choice) => {
  if (choice !== "accepted" && choice !== "declined") return;
  // ...
};

setVisible(false);
};

if (!visible) return null;

return (
<div
role="dialog"
aria-live="polite"
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[medium] Dialog ARIA contract is incomplete and self-contradictory

Three issues on this element:

  1. role="dialog" + aria-live="polite" is contradictory. Live regions announce updates as they happen; dialogs request focus and follow modal/inert semantics. Combining the two confuses screen readers — VoiceOver in particular will announce the dialog twice (once as a region, once as a dialog) and may skip the announcement entirely on subsequent renders.
  2. No focus management. The ARIA Authoring Practices for role="dialog" require focus to move to (or into) the dialog when it opens. Right now, when setVisible(true) runs, focus stays wherever it was — keyboard users have no signal that a dialog appeared.
  3. No Escape-to-dismiss. WCAG 2.1 SC 2.1.2 (no keyboard trap) is satisfied because the banner is non-modal, but the dialog contract typically expects Escape to act as a Decline shortcut. There is no keydown handler.

Suggested fix:

<div
  role="dialog"
  aria-modal="false"
  aria-labelledby="cookie-consent-title"
  aria-describedby="cookie-consent-desc"
  // drop aria-live entirely
  style={...}
>
  <span id="cookie-consent-title" style={visuallyHidden}>Cookie consent</span>
  <span id="cookie-consent-desc" style={{ flex: "1 1 240px" }}>...</span>
  ...
</div>

Add a useEffect that focuses the first button when visible flips to true, and a keydown handler that triggers respond("declined") on Escape (Decline is the GDPR-safe default).

Why: Brings the dialog up to ARIA spec, makes the banner discoverable to keyboard / screen-reader users, and resolves the role-vs-live-region conflict.

aria-label="Cookie consent"
style={{
position: "fixed",
left: "20px",
right: "20px",
bottom: "20px",
maxWidth: "520px",
margin: "0 auto",
padding: "16px 20px",
background: "#1f1f1f",
color: "#fff",
borderRadius: "8px",
boxShadow: "0 4px 16px rgba(0,0,0,0.25)",
zIndex: 1001,
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[medium] z-index 1001 over AiButton (1000) makes AiButton click target unreachable on mobile

The banner is position: fixed; left: 20px; right: 20px; bottom: 20px; maxWidth: 520px; margin: 0 auto; zIndex: 1001. The existing AiButton container (modified in theme.config.tsx) is position: fixed; right: 20px; bottom: 20px; zIndex: 1000. On any viewport narrower than ~560px (the banner's effective full width after the 20px insets), the banner spans the full width at the bottom and covers the AiButton with a higher z-index. The user cannot reach the AiButton without first dismissing the consent banner.

On viewports ≥ 560px the banner is centered (margin: 0 auto) at up to 520px wide; the AiButton anchored to right: 20px is partially obscured by the right edge of the banner when the viewport is between ~560px and ~580px wide.

Suggested fix: Move the banner above the AiButton vertically rather than overlapping (e.g. bottom: 88px on mobile so the banner sits above the AiButton), or temporarily hide the AiButton container while the banner is visible by exporting a consent state and letting the footer conditionally render the AiButton.

Why: Two stacked fixed-position UI elements in the same corner is a known interaction trap — the higher-z one blocks the lower one's click target.

fontSize: "14px",
lineHeight: 1.4,
display: "flex",
flexWrap: "wrap",
gap: "12px",
alignItems: "center",
justifyContent: "space-between",
}}
>
<span style={{ flex: "1 1 240px" }}>
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[high] Banner says "We use cookies" but the site sets no cookies

The copy declares cookie usage and asks the user to accept or decline "non-essential cookies." The PR adds no cookies — it adds localStorage usage. A repo-wide search for document.cookie, set-cookie, and any cookie-setting library returns zero matches. The only persistent client storage introduced by this PR is the consent record itself, which under GDPR / ePrivacy is strictly-necessary storage (storing the user's preference about non-essential storage is itself necessary) and does not require consent to begin with.

This creates two distinct problems:

  1. Factually incorrect copy. Telling users "we use cookies" when the site sets no cookies misrepresents data processing — under GDPR Article 12 the notice must be "clear and plain." A regulator looking at this banner would conclude the site is dishonest about its processing.
  2. The only thing stored doesn't need consent. The consent record stored in localStorage is strictly-necessary; asking permission for it (and recording the answer to that question in the same storage) is paradoxical.

Suggested fix: If the long-term plan is to add analytics that DO require consent, change the copy to be accurate about the future state ("This site may use analytics to help us understand how the docs are used.") and ship the banner together with the actual analytics integration. If the long-term plan is NO analytics, remove the banner entirely.

Why: A consent banner that lies about what it consents to is itself a GDPR compliance issue, not a fix for one.

We use cookies to improve your experience. You can accept or decline
non-essential cookies.
</span>
<div style={{ display: "flex", gap: "8px" }}>
<button
onClick={() => respond("declined")}
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[low] <button> elements lack explicit type="button"

Both buttons (Decline at line 61, Accept at line 75) omit the type attribute. In the current placement this is safe — the buttons are not inside a <form> — but if a future maintainer ever wraps the banner in a form (or if Nextra's MDX rendering injects one), the default type="submit" will cause unexpected form submissions on click.

Suggested fix:

<button type="button" onClick={() => respond("declined")} ...>Decline</button>
<button type="button" onClick={() => respond("accepted")} ...>Accept</button>

Why: One-line defensive fix; eliminates a fragility that will never trigger as the code stands today but is a recognised React best practice.

style={{
padding: "8px 14px",
fontSize: "14px",
cursor: "pointer",
background: "transparent",
color: "#fff",
border: "1px solid #888",
borderRadius: "4px",
}}
>
Decline
</button>
<button
onClick={() => respond("accepted")}
style={{
padding: "8px 14px",
fontSize: "14px",
cursor: "pointer",
background: "#22c55e",
color: "#000",
Copy link
Copy Markdown
Contributor

@clementupshot clementupshot May 18, 2026

Choose a reason for hiding this comment

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

[high] Asymmetric Accept/Decline styling is a recognised GDPR dark pattern

The Decline button (line 60-72) is a transparent ghost button with a gray border; the Accept button (line 74-86) is a bright green (#22c55e), bold, borderless filled button. EDPB Guidelines 03/2022 on dark patterns in social media (widely applied to all websites) and CNIL's published enforcement actions specifically flag this pattern — visually nudging users toward Accept by making Decline less prominent invalidates the "freely given" element of consent. Real fines have been issued for this specific UI shape.

Suggested fix: Give both buttons identical visual weight — same background treatment (both filled or both ghost), same font weight, same border treatment. If you must visually distinguish, the safer pattern is to make the Decline button the primary one (since under GDPR Decline is the default-safe choice).

// Both buttons styled identically; user is not nudged either direction.
const buttonStyle = {
  padding: "8px 14px",
  fontSize: "14px",
  cursor: "pointer",
  background: "transparent",
  color: "#fff",
  border: "1px solid #888",
  borderRadius: "4px",
  fontWeight: 500,
};

Why: Symmetric styling is a documented prerequisite for GDPR-valid consent. The bright-green Accept / gray-ghost Decline pairing is the canonical example of a non-compliant dark pattern.

[medium] Hardcoded #22c55e ignores the Nextra theme's primaryHue

Separately, theme.config.tsx already declares primaryHue: { dark: 144.71, light: 145.41 } and primarySaturation: { dark: 75.56, light: 55.78 }. Hardcoding a green hex value here means the banner will drift away from the theme on any future palette change, and it will not respect the light/dark theme toggle. If the dark-pattern fix above keeps any distinct "primary" treatment, that color should come from the theme tokens, not a hardcoded hex.

border: "none",
borderRadius: "4px",
fontWeight: 600,
}}
>
Accept
</button>
</div>
</div>
);
}

export default CookieConsent;
24 changes: 14 additions & 10 deletions theme.config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useRouter } from 'next/router'
import Link from 'next/link'
import { DocsThemeConfig, useConfig } from 'nextra-theme-docs'
import AiButton from './components/AiButton.js'
import CookieConsent from './components/CookieConsent.js'

const config: DocsThemeConfig = {
useNextSeoProps() {
Expand Down Expand Up @@ -68,16 +69,19 @@ const config: DocsThemeConfig = {
// fixed-position container.
footer: {
component: () => (
<div
style={{
position: "fixed",
right: "20px",
bottom: "20px",
zIndex: 1000,
}}
>
<AiButton />
</div>
<>
<div
style={{
position: "fixed",
right: "20px",
bottom: "20px",
zIndex: 1000,
}}
>
<AiButton />
</div>
<CookieConsent />
</>
),
},
sidebar: {
Expand Down
Loading