fix(ai): button contrast skips on unresolved \$color refs (no invisible fg)
Codex flagged: the previous version (ddf6580f) treated a NaN luminance — what `hexLuminance` returns when the bg color is still a `\$color-accent` ref because the doc's variables haven't been seeded — as a dark bg and painted white text. If the user's palette later resolves \$color-accent to a LIGHT hex (e.g. cream #FFE4B5), the white text becomes invisible on the light bg. Same risk in the inverted direction with the original code, which defaulted to dark text on unknown bg. Either guess can ship a visually broken button. Skip the contrast pass entirely when we can't resolve the bg ref to a hex — text / icon retain whatever fill they already carry, which is at least visible (the model's default text color, usually dark). A later post-pass invocation, after variables get seeded, re-runs and applies contrast cleanly with a real luminance value. `needsContrastOverride` already returns false when either luminance is non-finite, so the icon-override branch was already safe; only the text branch and the (now-removed) NaN→white default needed the fix. New regression test in role-resolver.test.ts covers the unseeded-ref case: explicit dark text fill on an unresolvable accent button survives untouched.
This commit is contained in:
parent
ecd8d2df1a
commit
705673eb8e
|
|
@ -2164,6 +2164,58 @@ 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.
|
||||
const button: PenNode = {
|
||||
id: 'btn',
|
||||
type: 'frame',
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 120,
|
||||
height: 44,
|
||||
role: 'button',
|
||||
// Unseeded ref — the doc has no variables yet.
|
||||
fill: [{ type: 'solid', color: '$color-accent' }],
|
||||
children: [
|
||||
{
|
||||
id: 'txt',
|
||||
type: 'text',
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 80,
|
||||
height: 20,
|
||||
content: 'Sign In',
|
||||
// Model's default — dark text on UNKNOWN bg. Survive untouched.
|
||||
fill: [{ type: 'solid', color: '#0F172A' }],
|
||||
} 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 }> };
|
||||
// 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');
|
||||
});
|
||||
|
||||
it('still fills unfilled icon_font (no regression on the original branch)', () => {
|
||||
const button: PenNode = {
|
||||
id: 'btn',
|
||||
|
|
|
|||
|
|
@ -763,20 +763,28 @@ function fixButtonForegroundContrast(parent: FrameNode): void {
|
|||
if (!bgColorRaw) return;
|
||||
// The model emits accent-colored buttons as `$color-accent`, not
|
||||
// hex. Without resolving the ref the luminance check sees a literal
|
||||
// `$color-...` string, parseInt returns NaN, `NaN < 0.5` is false,
|
||||
// and the dark-fg branch wins — so on an orange accent button the
|
||||
// contrast pass paints text dark-on-orange. Use the resolved hex
|
||||
// (or the literal when not a ref) for luminance.
|
||||
// `$color-...` string, parseInt returns NaN, and `NaN < 0.5` is
|
||||
// false — so the original code always picked the dark fg branch on
|
||||
// unresolved refs (visible bug: dark text on orange accent).
|
||||
// Resolve the ref via the doc's variables before deciding contrast.
|
||||
const bgColor = resolveColorMaybeRef(bgColorRaw);
|
||||
if (!bgColor) return;
|
||||
|
||||
const lum = hexLuminance(bgColor);
|
||||
// Treat unparseable luminance (NaN — e.g. when the variable was not
|
||||
// seeded and resolution returned the original ref) as a dark bg so
|
||||
// the contrast pass at least paints visible white text instead of
|
||||
// dark-on-unknown. Better default than the previous silent NaN<0.5
|
||||
// branch which always picked the dark fg.
|
||||
const fgColor = !Number.isFinite(lum) || lum < 0.5 ? '#FFFFFF' : '#0F172A';
|
||||
// When luminance is unparseable (the ref still didn't resolve to a
|
||||
// hex — e.g. doc variables haven't been seeded yet, or the ref
|
||||
// points at a missing token), we cannot pick a contrast color
|
||||
// safely. Painting white risks invisible-on-light bg; painting
|
||||
// dark risks invisible-on-dark bg. Either guess can ship a
|
||||
// visually broken button. The least-bad option is to skip the
|
||||
// contrast pass entirely on this button — text/icon retain
|
||||
// whatever fill they already had, which is at least *something*
|
||||
// visible (the model's default text color, usually a dark hex). A
|
||||
// later post-pass run AFTER variables get seeded will re-resolve
|
||||
// and apply contrast cleanly.
|
||||
if (!Number.isFinite(lum)) return;
|
||||
|
||||
const fgColor = lum < 0.5 ? '#FFFFFF' : '#0F172A';
|
||||
const fgFill: PenFill[] = [{ type: 'solid', color: fgColor }];
|
||||
|
||||
if (!('children' in parent) || !Array.isArray(parent.children)) return;
|
||||
|
|
|
|||
Loading…
Reference in a new issue