Light and dark variants for every ad, picked automatically - #211
Merged
Merged
Conversation
Every creative carried one palette, and the generator's prompt pushed it dark. On a black-on-white publisher page the unit rendered as a hole punched in the page. Nothing errored, so it was invisible until somebody ran the tag on a white site. Creatives now carry two palettes and the tag works out which to ask for: - ad.js walks up from the container for the first painted background, converts it to relative luminance and sends theme=light|dark. With nothing painted anywhere the answer is light, because that is what the browser paints. prefers-color-scheme deliberately does not decide it: a page with no CSS is white on a dark desktop too. data-theme on the unit overrides everything. - The generator asks the model for both trios and trusts neither: each is checked for real contrast against the background it will sit on (4.5:1 text, 3:1 CTA) and a failing trio is replaced by a derived one. - paletteFor() derives a missing variant on the fly, so a creative that predates this renders correctly without waiting for the backfill. - Slots get a default polarity for surfaces that cannot measure: a MOTD over curl, a feed spliced at build time, a page with JS blocked. Colour maths lives in lib/ads/theme.ts and is the single source for the renderer, the editor preview and the backfill, so a backfilled palette and a freshly generated one cannot disagree. Also: the editor's colour pickers gained an opacity slider (#rrggbbaa, with a checkerboard swatch), and inks that punch out of a chip now strip alpha so a translucent background cannot make a CTA label see-through. The hard-coded rgba(255,255,255,.08) hairline on every unit is now theme-aware. It was a white haze: invisible on a dark page, a grey smear on a light one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renders one creative in both polarities into a single page, so the pair can be eyeballed the way a publisher sees them rather than inferred from a hex in a diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#210 set border-radius to 0 on the five ad container rules, in exactly the lines this branch rewrote for light/dark. Resolved by keeping both: the containers stay square, and the palette and hairline stay theme-aware. Also squared off the three container radii in the React <AdPreview>, which #210 missed. That component exists to mirror renderCreativeHtml, so a rounded preview of a square unit is the drift it is there to prevent. Inner chrome (logo, monogram, CTA pill) keeps its radius, and so does the feed card, which mocks a reader's own sheet rather than an ad container in an iframe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan36 finding(s) HIGH/CRITICAL: 3 | MEDIUM: 24 | LOW: 9
Snippets are redacted; ThreatCrush never prints matched credential material. |
Contributor
Author
|
Merged #210 set One thing #210 missed, fixed here: the React Worth noting for later: the corner bleed #210 fixed was the host page showing through a rounded ad — which is the same root cause this PR addresses from the other end. Once the unit's background matches the page it sits on, a rounded corner would bleed a colour that matches instead of white. The radius could go back if you want it; that is a look decision, so I left #210's call alone.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Every creative carried one palette, and the generator's prompt pushed it dark. On a black-on-white publisher page (a plain blog with no CSS) the unit rendered as a 40px bar of near black, sitting at the bottom of a white page like a hole punched in it. Nothing errored and the tests passed, so it was invisible until somebody ran the tag on a white site.
Found by installing the
text_linkunit ondev.profullstack.com/~anthony/blog, which is deliberately black on white.What changed
Every creative now carries two palettes, and the tag works out which to ask for.
Detection (
app/ad.js/route.ts). Walks up from the ad container for the first element with a painted background, converts it to relative luminance, and sends&theme=light|dark. If nothing paints a background anywhere, the answer is light, because that is what the browser actually paints.prefers-color-schemedeliberately does not decide this: a page with no CSS is white on a dark desktop too.data-theme="light|dark"on the unit overrides everything, which is the knob a publisher reaches for when the guess is wrong.Generation (
lib/ads/creative.ts). The model is asked for both trios and trusted for neither: each is checked for real contrast against the background it will sit on (4.5:1 text, 3:1 for the CTA chip) and a failing trio is replaced by a derived one. A wrong light palette is worse than a computed one, because it ships an unreadable ad to a real publisher.Derivation (
lib/ads/theme.ts). Hue is preserved, lightness and a little saturation move. An accent picked to glow on near black is almost always too pale to read on near white. This is the single source of colour maths for the renderer, the editor preview and the backfill, so a backfilled palette and a freshly generated one cannot disagree.Fallback.
paletteFor()derives a missing variant on the fly, so a creative that predates this renders correctly without waiting for the backfill. Slots also get a default polarity for surfaces that cannot measure anything: a MOTD over curl, a feed spliced at build time, a page with JS blocked.Alpha in the editor. The colour pickers gained an opacity slider writing
#rrggbbaa(with a checkerboard swatch so translucent reads as translucent).hexToRgbanow multiplies alpha rather than replacing it, and inks that punch out of a chip go throughsolid()so a translucent background cannot make a CTA label see-through.Hairlines. The hard-coded
rgba(255,255,255,.08)border on every unit is now theme-aware. It was a white haze: invisible on a dark page and a grey smear on a light one.Already applied to production
20260819090000_ad_theme_variantsapplied.Three real bugs the backfill surfaced
Each is now a regression test:
#a3a3a3CTA came out reddish. Greys now stay grey and get contrast from lightness alone.radial-gradient(..., var(--accent), ...), var(--bg)was read as the accent. CSS only permits a colour on the final layer, and the base is what a reader sees behind text.prefers-color-schemeblocks flipped the verdict. A light site with a dark mode was being read as dark, because the override won as "last declaration". Those blocks describe a particular visitor, not the site, and are now stripped when deciding a default.Also added: CSS custom-property resolution, without which almost every modern stylesheet (
body{background:var(--bg)}) reported "no declared background".Heads-up before merge
114 creatives had a primary palette that was actually light. The backfill moved it into the light columns and derived a dark counterpart, which is correct once this deploys. Until it deploys, those 114 render in the derived dark palette, because the running code only reads
bg_color. It self-corrects on deploy, so this is worth shipping promptly rather than sitting on the branch.Verification
tsc --noEmitclean.tracker-geoneeds the GeoLite2 download, andads-feed-itemcannot resolve its deps becausepnpm installis broken at HEAD (@profullstack/autoblog#75e54afno longer resolves upstream, andfast-xml-parserwas never installed). Both fail onmastertoo.npm run lintis broken at HEAD independently of this change (next lintreads "lint" as a directory).🤖 Generated with Claude Code