fix(ai): button contrast resolves \$color refs via built-in semantic palette
Codex flagged the previous fix (1c08ac3f, "skip on unresolved ref") as too conservative: a sub-agent's button with `\$color-accent` bg and a text child WITHOUT a fill ended up with no fill at all when doc.variables hadn't been seeded yet — text falls back to black, which is invisible on a dark accent button. Better fix: extend `resolveColorMaybeRef` with a step-2 fallback into the built-in semantic palette (`getSemanticPaletteHex`). Every core token (`color-accent`, `color-bg`, `color-text-primary`, etc.) has a known hex for both `Light` and `Dark` modes, so the cascade now is: 1. Doc-seeded variables (user's palette). 2. Built-in semantic palette for the requested mode. 3. Original ref string (unresolvable) — caller bails. In the food-app + GPT-5.5 path, step 1 already worked because `seedDocVariablesFromStyleGuide` runs before the sub-agents. The fallback is for paths that bypass seeding (test fixtures, external MCP callers, mid-flight states), not the common case. Skip-on-NaN behavior stays — it now only triggers for genuinely unknown tokens (`\$color-foobar` or similar), where any guess is worse than leaving the model's existing fill alone. Tests: - New: `\$color-accent` ref resolves to #2563EB via semantic palette even with no doc.variables, contrast pass picks white fg correctly for the unfilled text child. - New: `\$color-mystery-token` (not in palette) — step-2 misses, contrast pass skips, existing text fill survives. - Replaces the prior "skips on unresolved" test which over-asserted the conservative path.
This commit is contained in:
Fini
parent
705673eb8e
commit
0bbdb0bf35
|
|
@ -2164,17 +2164,20 @@ describe('resolveTreePostPass — icon_font contrast override', () => {
|
|||
expect(ico.fill?.[0]?.color).toBe('#DC2626');
|
||||
});
|
||||
|
||||
it('skips contrast pass when bg ref does not resolve (no white-on-unknown)', () => {
|
||||
// Regression: my fix from `ddf6580f` originally treated a NaN
|
||||
// luminance (unresolvable ref) as "dark bg" and painted white
|
||||
// text. If the doc's `$color-accent` later resolves to a LIGHT
|
||||
// hex (e.g. the user's chosen palette has accent=#FFE4B5), the
|
||||
// white text becomes invisible on the light bg. Codex flagged
|
||||
// this on stop-time review — the safer default is to skip the
|
||||
// contrast pass entirely when we can't resolve the ref, leaving
|
||||
// text/icon with whatever fill they already had. A later
|
||||
// post-pass invocation (after variables are seeded) re-runs and
|
||||
// applies contrast cleanly with a real hex luminance.
|
||||
it('resolves \\$color-accent ref via semantic palette when doc.variables is unseeded', () => {
|
||||
// Regression chain:
|
||||
// ddf6580f — treated NaN luminance as "dark bg" → white text
|
||||
// on whatever the user's palette later resolved to,
|
||||
// risking white-on-light invisibility.
|
||||
// 1c08ac3f — flipped to "skip pass on NaN" → text without fill
|
||||
// stayed with no fill, defaulting to black, risking
|
||||
// black-on-dark invisibility on accent buttons.
|
||||
// THIS COMMIT — `resolveColorMaybeRef` now cascades through the
|
||||
// built-in semantic palette as a step-2 fallback, so
|
||||
// `\$color-accent` always resolves to a known hex
|
||||
// (`#2563EB` light, `#60A5FA` dark) even with no
|
||||
// doc.variables. The contrast pass then runs with a
|
||||
// real luminance and picks the right fg.
|
||||
const button: PenNode = {
|
||||
id: 'btn',
|
||||
type: 'frame',
|
||||
|
|
@ -2183,7 +2186,7 @@ describe('resolveTreePostPass — icon_font contrast override', () => {
|
|||
width: 120,
|
||||
height: 44,
|
||||
role: 'button',
|
||||
// Unseeded ref — the doc has no variables yet.
|
||||
// Unseeded ref — the doc has no variables.
|
||||
fill: [{ type: 'solid', color: '$color-accent' }],
|
||||
children: [
|
||||
{
|
||||
|
|
@ -2194,7 +2197,51 @@ describe('resolveTreePostPass — icon_font contrast override', () => {
|
|||
width: 80,
|
||||
height: 20,
|
||||
content: 'Sign In',
|
||||
// Model's default — dark text on UNKNOWN bg. Survive untouched.
|
||||
// No fill emitted — the contrast pass has to supply one.
|
||||
} as PenNode,
|
||||
],
|
||||
} as PenNode;
|
||||
const root: PenNode = {
|
||||
id: 'root',
|
||||
type: 'frame',
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 375,
|
||||
height: 812,
|
||||
children: [button],
|
||||
} as PenNode;
|
||||
dispatchPostPass(root);
|
||||
const txt = ((root as { children: PenNode[] }).children[0] as { children: PenNode[] })
|
||||
.children[0] as PenNode & { fill?: Array<{ color?: string }> };
|
||||
// \$color-accent (light mode default) is #2563EB blue, lum ≈ 0.27
|
||||
// → contrast pass picks white fg.
|
||||
expect(txt.fill?.[0]?.color).toBe('#FFFFFF');
|
||||
});
|
||||
|
||||
it('skips contrast on unknown ref tokens (preserves existing text fill)', () => {
|
||||
// Step-2 fallback only handles tokens in the built-in semantic
|
||||
// palette. A made-up ref like `\$color-mystery` cascades all the
|
||||
// way through and returns the original string. The luminance
|
||||
// check then bails (NaN) and we leave the existing text fill
|
||||
// alone rather than guess and risk invisibility.
|
||||
const button: PenNode = {
|
||||
id: 'btn',
|
||||
type: 'frame',
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 120,
|
||||
height: 44,
|
||||
role: 'button',
|
||||
fill: [{ type: 'solid', color: '$color-mystery-token' }],
|
||||
children: [
|
||||
{
|
||||
id: 'txt',
|
||||
type: 'text',
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 80,
|
||||
height: 20,
|
||||
content: 'Sign In',
|
||||
fill: [{ type: 'solid', color: '#0F172A' }],
|
||||
} as PenNode,
|
||||
],
|
||||
|
|
@ -2211,8 +2258,6 @@ describe('resolveTreePostPass — icon_font contrast override', () => {
|
|||
dispatchPostPass(root);
|
||||
const txt = ((root as { children: PenNode[] }).children[0] as { children: PenNode[] })
|
||||
.children[0] as PenNode & { fill?: Array<{ color?: string }> };
|
||||
// Original fill survives — contrast pass bailed because we
|
||||
// can't safely choose a color against an unresolved bg.
|
||||
expect(txt.fill?.[0]?.color).toBe('#0F172A');
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,13 @@
|
|||
import type { PenNode, FrameNode, SizingBehavior } from '@/types/pen';
|
||||
import type { PathNode } from '@/types/pen';
|
||||
import type { PenFill, PenStroke, PenEffect, SolidFill } from '@/types/styles';
|
||||
import { resolveColorRef, getDefaultTheme } from '@zseven-w/pen-core';
|
||||
import {
|
||||
resolveColorRef,
|
||||
getDefaultTheme,
|
||||
getSemanticPaletteHex,
|
||||
SEMANTIC_PALETTE_THEME_LIGHT,
|
||||
SEMANTIC_PALETTE_THEME_DARK,
|
||||
} from '@zseven-w/pen-core';
|
||||
import { useDocumentStore } from '@/stores/document-store';
|
||||
import {
|
||||
toSizeNumber,
|
||||
|
|
@ -20,16 +26,52 @@ import { resolveIconPathBySemanticName } from './icon-resolver';
|
|||
* doc store directly because the role resolver runs without an
|
||||
* explicit variables param threaded through every helper.
|
||||
*/
|
||||
function resolveColorMaybeRef(color: string | undefined): string | undefined {
|
||||
/**
|
||||
* Resolve a color string that may be a `$color-*` variable ref into the
|
||||
* concrete hex it points at on the active theme. Returns the original
|
||||
* string when it isn't a ref, or `undefined` when input is undefined.
|
||||
*
|
||||
* Resolution cascade (each step's miss falls through to the next):
|
||||
* 1. Doc-seeded variables (the user's chosen palette, if any).
|
||||
* 2. The built-in `getSemanticPaletteHex` map for the detected
|
||||
* `Light` / `Dark` mode. This fallback covers the case where a
|
||||
* sub-agent emits `$color-accent` BEFORE
|
||||
* `seedDocVariablesFromStyleGuide` runs (or when seeding fails)
|
||||
* — every semantic token has a known light + dark hex baked in.
|
||||
* 3. Return the original ref string. The caller (typically a
|
||||
* luminance check) treats this as "unresolvable" and bails.
|
||||
*
|
||||
* Without step 2 the contrast pass would either skip the button
|
||||
* entirely (text/icon stays at the model's default — usually a dark
|
||||
* hex that's invisible on a dark accent bg) or pick the wrong fg via
|
||||
* a NaN-luminance default. The cascade keeps a best-effort hex
|
||||
* available even when the doc state is mid-flight.
|
||||
*/
|
||||
function resolveColorMaybeRef(
|
||||
color: string | undefined,
|
||||
themeHint?: 'light' | 'dark',
|
||||
): string | undefined {
|
||||
if (color === undefined) return undefined;
|
||||
if (!color.startsWith('$')) return color;
|
||||
const doc = useDocumentStore.getState().document;
|
||||
|
||||
// Step 1: doc-seeded variables.
|
||||
const variables = doc.variables;
|
||||
if (!variables || Object.keys(variables).length === 0) return color;
|
||||
const themes = doc.themes;
|
||||
const activeTheme = themes ? getDefaultTheme(themes) : undefined;
|
||||
const resolved = resolveColorRef(color, variables, activeTheme);
|
||||
return typeof resolved === 'string' ? resolved : color;
|
||||
if (variables && Object.keys(variables).length > 0) {
|
||||
const themes = doc.themes;
|
||||
const activeTheme = themes ? getDefaultTheme(themes) : undefined;
|
||||
const resolved = resolveColorRef(color, variables, activeTheme);
|
||||
if (typeof resolved === 'string' && !resolved.startsWith('$')) return resolved;
|
||||
}
|
||||
|
||||
// Step 2: built-in semantic palette for the requested or default mode.
|
||||
const tokenName = color.slice(1); // strip leading '$'
|
||||
const mode = themeHint === 'dark' ? SEMANTIC_PALETTE_THEME_DARK : SEMANTIC_PALETTE_THEME_LIGHT;
|
||||
const paletteHex = getSemanticPaletteHex(mode);
|
||||
if (typeof paletteHex[tokenName] === 'string') return paletteHex[tokenName];
|
||||
|
||||
// Step 3: unresolvable; let the caller decide.
|
||||
return color;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in a new issue