Skip to content

feat(surveys): fix dark theme + redesign closed survey error page#53813

Open
lucasheriques wants to merge 1 commit intomasterfrom
lucas/surveys-error-page-and-dark-theme-fix
Open

feat(surveys): fix dark theme + redesign closed survey error page#53813
lucasheriques wants to merge 1 commit intomasterfrom
lucas/surveys-error-page-and-dark-theme-fix

Conversation

@lucasheriques
Copy link
Copy Markdown
Contributor

Problem

Two real bugs on the hosted survey page + its error sibling looked wrong on dark themes.

Changes

  1. renderSurvey() doesn't call addSurveyCSSVariablesToElement — so none of the --ph-survey-* container variables were set by posthog-js on the API path. Customer appearance fields (`inputBackground`, `borderColor`, text colors, submit button colors, etc.) were silently ignored and dark-theme surveys kept black body text. Replicated the variable-setting logic locally in `applyPageAppearance` and set everything on `:root` so values cascade into the container.
  2. Corner branding pill was hardcoded white-translucent, which reads as a glaring badge on dark backgrounds. Now adapts to page luminance and flips to a dark translucent fill on dark themes. Same for the error page pill.
  3. `surveys/error.html` rewritten to match the hosted page aesthetic. Dropped the brand gradient stripe, logo, orange error circle, decorative dots, "Go Back" / "Try Again" buttons (the latter was misleading — reloading doesn't un-archive a survey), and dark mode. Template variables (`error_title`, `error_message`) untouched. Threads `survey.appearance` through when we have the survey object loaded so a respondent hitting a closed survey link still sees the customer's brand.

How did you test this code?

Authored by Claude in an interactive session with Lucas. Manually exercised on the dev server with a dark-theme customer survey (`backgroundColor: "#171717"`, explicit `textColor`, custom `inputBackground`). Verified that question text, inputs, buttons, borders, and the corner pill all respect the customer theme. Not tested on other browsers, real touch devices, or with screen readers.

Publish to changelog?

No

🤖 LLM context

The load-bearing finding: `posthog-js`'s `renderSurvey` at `surveys.tsx:377` only calls `Preact.render()` — it does not call `addSurveyCSSVariablesToElement`. That function is only wired into the automatic popup/widget display path. On the hosted survey page, every `var(--ph-survey-*, fallback)` in the template's CSS was silently resolving to the fallback, which is why customer appearance customization was being ignored for everything except `backgroundColor` (which we were already setting ourselves). The fix replicates `addSurveyCSSVariablesToElement`'s variable mapping locally. Fixing this upstream (have `renderSurvey` call the helper) would be a cleaner long-term path but affects every consumer of that API, so doing it here for now.

@graphite-app graphite-app bot added graphite-merge-queue stamphog Request AI review from stamphog labels Apr 9, 2026
@graphite-app
Copy link
Copy Markdown
Contributor

graphite-app bot commented Apr 9, 2026

Graphite Automations

"Add graphite merge queue" took an action on this PR • (04/09/26)

2 labels were added to this PR based on Lucas Faria's automation.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

UI/template-only redesign by the owning team author. The appearance data is safely injected via Django's json_script filter (XSS-safe) and the inline script carries a CSP nonce; CSS custom property injection via setProperty is constrained and cannot escalate to script execution. No data model, API contract, or backend logic changes of concern.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 9, 2026

Vulnerabilities

No security concerns identified. Customer appearance data is embedded in the template via Django's json_script filter (which HTML-escapes the JSON), and all values are consumed exclusively through style.setProperty for CSS custom properties — CSS custom properties cannot execute JavaScript, so there is no XSS vector even if appearance fields contain attacker-controlled strings.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: posthog/templates/surveys/public_survey.html
Line: 33

Comment:
**Unused CSS variable**

`--ph-survey-pill-border` is declared here but never referenced anywhere in the file — no `border` or `border-color` property on `.footer-branding` consumes it. Per the simplicity rules, this is a superfluous part.

```suggestion
            --ph-survey-pill-background: rgba(255, 255, 255, 0.7);
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: posthog/templates/surveys/error.html
Line: 135-151

Comment:
**`getLuminance` duplicated across templates**

This IIFE re-implements `getLuminance` identically to the copy in `public_survey.html` (lines 703-715), violating the OnceAndOnlyOnce rule. Both templates share the same logic for hex parsing and Rec. 709 coefficients. If a bug is found (e.g. the `#` guard doesn't handle `rgb()` values), it would need to be patched in two places. Consider extracting to a shared static JS file that both templates include, or at minimum leaving a cross-reference comment.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat(surveys): fix dark theme + redesign..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

🎭 Playwright report · View test results →

⚠️ 2 flaky tests:

  • Save view (chromium)
  • Materialize view pane (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this comment? Help fix flakies and failures and it'll disappear!

@tests-posthog
Copy link
Copy Markdown
Contributor

tests-posthog bot commented Apr 9, 2026

Query snapshots: Backend query snapshots updated

Changes: 1 snapshots (1 modified, 0 added, 0 deleted)

What this means:

  • Query snapshots have been automatically updated to match current output
  • These changes reflect modifications to database queries or schema

Next steps:

  • Review the query changes to ensure they're intentional
  • If unexpected, investigate what caused the query to change

Review snapshot changes →

@lucasheriques lucasheriques force-pushed the lucas/surveys-error-page-and-dark-theme-fix branch from 46ad403 to 4495f9e Compare April 9, 2026 03:03
Two real bugs on the hosted survey page surface (/external_surveys/<id>)
plus a redesign of its error sibling:

1. renderSurvey() does NOT call addSurveyCSSVariablesToElement, so none
   of the --ph-survey-* container variables were being set by posthog-js
   on the API path. Customer appearance fields (inputBackground, borderColor,
   text colors, etc.) were silently ignored and dark-theme surveys kept
   black body text. Fix: replicate addSurveyCSSVariablesToElement locally
   in applyPageAppearance and set everything on :root so values cascade.

2. Corner branding pill was hardcoded to rgba(255,255,255,0.7), which
   reads as a glaring white badge on dark backgrounds. Now adapts to
   page luminance and flips to a dark translucent fill on dark themes.

3. surveys/error.html (shown when a survey is archived / not running /
   invalid) rewritten to match the hosted page's understated aesthetic.
   Drops the brand gradient stripe, logo, orange error circle, decorative
   dots, Go Back / Try Again buttons, and dark mode. Kept the template
   variables untouched. Threads survey.appearance through when we have
   the survey object loaded so a respondent hitting a closed survey link
   still sees the customer's brand (background, text color, font).

Verified BREACH-safe changes — no reflected secrets, same @csrf_exempt view.
@lucasheriques lucasheriques force-pushed the lucas/surveys-error-page-and-dark-theme-fix branch from 4495f9e to a6b0072 Compare April 9, 2026 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

graphite-merge-queue stamphog Request AI review from stamphog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant