feat: add GDPR cookie consent#161
Conversation
✅ Deploy Preview for alloradocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
No issues found across 2 files
Architecture diagram
sequenceDiagram
participant User as User Browser
participant App as Nextra Docs App
participant Consent as CookieConsent Component
participant Storage as localStorage
Note over User,Storage: GDPR Cookie Consent Flow
User->>App: Navigate to docs page
App->>Consent: Mount CookieConsent in footer
Consent->>Consent: useEffect - check visibility
Consent->>Storage: Check for STORAGE_KEY
alt Key not found
Storage-->>Consent: null
Consent->>Consent: setVisible(true)
Consent-->>User: Render consent dialog
User->>Consent: Click "Accept" or "Decline"
Consent->>Storage: setItem(STORAGE_KEY, choice)
Consent->>Consent: setVisible(false)
Consent-->>User: Remove dialog from DOM
else Key exists (already responded)
Storage-->>Consent: stored choice
Consent->>Consent: setVisible(false)
Consent-->>User: No dialog rendered
else localStorage unavailable/error
alt Error thrown
Consent->>Consent: setVisible(true) as fallback
end
end
clementupshot
left a comment
There was a problem hiding this comment.
Review summary
This PR adds a small GDPR-style cookie consent banner (components/CookieConsent.js, ~94 LOC) and wires it into the Nextra footer.component slot alongside the existing AiButton. The implementation itself is clean React (correct SSR/hydration pattern, useState + useEffect, try/catch around storage), but a repo-wide search reveals the most consequential issue: there is currently nothing for this banner to govern. No analytics, tracking scripts, third-party cookies, or HTTP cookies exist anywhere in the codebase, so the stored "accepted/declined" preference is read by zero callers. The banner records a decision but enforces nothing — which is more legally exposed than not having a banner at all, because it creates a consent audit trail for processing that doesn't happen.
The review focused on: GDPR semantic correctness, the consent state machine, accessibility (the component declares role="dialog"), private-mode storage failure paths, and visual/positioning interaction with the existing AiButton.
Verdict
- 🔴 High: 4 (theatrical banner; no withdrawal path; banner copy says "cookies" but site sets none; dark-pattern button styling)
- 🟡 Medium: 8 (private-mode infinite-banner loop, ARIA contract gaps, no public consent API, no policy link, hardcoded theme color, z-index/mobile interaction, no schema version, existence-only storage check)
- 🔵 Low: 5 (redundant SSR guard, silent catch, untyped choice, mobile overflow risk, missing
type="button") - ⚪ Info: 3 (positive — SSR pattern, Strict-Mode behavior, and codebase conventions are all correctly handled)
What's done well
useState(false)+useEffectis the correct SSR-safe pattern for client-only UI in Next.js / Nextra — no hydration mismatch.- React 18 Strict Mode double-invocation of
useEffectis handled gracefully because the read is idempotent. - The component follows existing conventions in the repo (inline styles,
.jsextension, default export,STORAGE_KEYnaming). - Default state is fail-closed (no stored value → banner shows).
- No new third-party dependencies — zero supply-chain impact.
Out-of-diff findings
🔴 High — No mechanism to withdraw consent (GDPR Art. 7(3))
GDPR Article 7(3) requires that "it shall be as easy to withdraw consent as to give it." Once a user clicks Accept or Decline, this PR provides no UI to change the choice — there is no settings page, no footer link, and no re-prompt. The only way for a user to re-trigger the banner is to clear localStorage from devtools, which is not "as easy."
Suggested fix: Add a "Cookie preferences" link in the site footer that clears localStorage.removeItem("allora-cookie-consent") and re-shows the banner. Or expose a <CookieSettings /> component that toggles consent on demand.
🟡 Medium — No public API for consent state
The component is a write-only sink: it has zero props, no callbacks, and the STORAGE_KEY constant is module-private. The moment any future code needs to gate on consent (the first analytics script, a third-party embed, etc.), there is no exported helper to read it without duplicating the magic string. Recommend exporting getConsent()/onConsentChange() (or moving the key + reader to lib/consent.ts) before this becomes a duplication trap.
🟡 Medium — No privacy/cookie policy page exists in the repo
The banner text invites the user to "accept or decline non-essential cookies" but offers no link to a policy explaining what those cookies are, who processes them, and for how long. GDPR Articles 12–13 require informed consent — a banner that asks for consent without offering the information needed to make an informed decision is not GDPR-compliant. Adding a pages/privacy.mdx (or external link) and linking it from the banner is the standard fix.
🔵 Low — Banner layout may overflow on narrow mobile viewports
With left: 20px, right: 20px, and maxWidth: 520px + a flex: '1 1 240px' text basis plus two buttons, the layout has been observed to tighten past horizontal capacity on viewports ≤ 360px (iPhone SE 1st gen, older Android). Worth a manual check; flexWrap: wrap is in place but the text+buttons can still overflow on very narrow widths.
Next actions
- Decide whether to delete the banner (no analytics → no consent UI needed and the banner is misleading) or land it together with (a) an actual analytics integration that respects the stored preference, (b) honest banner copy that matches what's actually being stored/used, (c) a withdrawal path, and (d) a privacy policy page.
- Either way: fix the silent
localStorage.setItemfailure path so private-mode users aren't trapped in an infinite-banner loop. - Bring the dialog up to ARIA spec: drop
aria-livefrom thedialogelement, add focus management on open, Escape-to-dismiss, and atype="button"on each<button>. - Symmetric Accept/Decline button styling (equal visual weight) and a theme-aware color (instead of hardcoded
#22c55e).
| @@ -0,0 +1,94 @@ | |||
| import React, { useEffect, useState } from "react"; | |||
|
|
|||
| const STORAGE_KEY = "allora-cookie-consent"; | |||
There was a problem hiding this comment.
[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.
| const [visible, setVisible] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| if (typeof window === "undefined") return; |
There was a problem hiding this comment.
[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.
| useEffect(() => { | ||
| if (typeof window === "undefined") return; | ||
| try { | ||
| if (!window.localStorage.getItem(STORAGE_KEY)) { |
There was a problem hiding this comment.
[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.
| const respond = (choice) => { | ||
| try { | ||
| window.localStorage.setItem(STORAGE_KEY, choice); | ||
| } catch (e) {} |
There was a problem hiding this comment.
[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;
// ...
};| return ( | ||
| <div | ||
| role="dialog" | ||
| aria-live="polite" |
There was a problem hiding this comment.
[medium] Dialog ARIA contract is incomplete and self-contradictory
Three issues on this element:
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.- No focus management. The ARIA Authoring Practices for
role="dialog"require focus to move to (or into) the dialog when it opens. Right now, whensetVisible(true)runs, focus stays wherever it was — keyboard users have no signal that a dialog appeared. - 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
keydownhandler.
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.
| color: "#fff", | ||
| borderRadius: "8px", | ||
| boxShadow: "0 4px 16px rgba(0,0,0,0.25)", | ||
| zIndex: 1001, |
There was a problem hiding this comment.
[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.
| justifyContent: "space-between", | ||
| }} | ||
| > | ||
| <span style={{ flex: "1 1 240px" }}> |
There was a problem hiding this comment.
[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:
- 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.
- The only thing stored doesn't need consent. The consent record stored in
localStorageis 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.
| </span> | ||
| <div style={{ display: "flex", gap: "8px" }}> | ||
| <button | ||
| onClick={() => respond("declined")} |
There was a problem hiding this comment.
[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.
| fontSize: "14px", | ||
| cursor: "pointer", | ||
| background: "#22c55e", | ||
| color: "#000", |
There was a problem hiding this comment.
[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.
Add minimal GDPR cookie consent popup