mirror of
https://github.com/ZSeven-W/openpencil.git
synced 2026-05-31 19:04:29 +07:00
fix(ai): hasFill must return false for transparent fills so contrast can paint
Follow-up on ef92774. Splitting the stroke/fill schema normalizer by
node type resolved the text/icon_font case, but left a third broken
scenario Codex caught immediately: a path node used as an icon inside
a button.
The chain was:
1. AI emits stroke-style line icon:
type=path, fill=[{color:"none"}], stroke={thickness:2} (no stroke color)
2. normalizeStrokeFillSchema replaces "none" with the explicit
transparent hex:
fill=[{color:"#00000000"}]
This is correct for activity rings / chart lines — deleting the
field would let the renderer fall back to the default gray.
3. fixButtonForegroundContrast walks the button children:
if (hasFill(child)) skip ← hasFill sees the transparent entry
else if stroke exists but has no fill → paint stroke
else if no stroke → paint fill
The naive hasFill counted the transparent entry as "has fill" and
took the skip branch, so the button icon never got a visible
color and rendered invisible on canvas.
The right fix is in hasFill itself — the function's semantic contract
is "does this node have a VISIBLE fill?", and a fully-transparent fill
(either CSS keyword "transparent"/"none", or any 8-digit hex with 00
alpha) draws nothing. Widen the invisibility check so every downstream
heuristic that relies on hasFill gets the same truthful answer without
each call site having to guard itself.
Added 5 unit tests for hasFill (#00000000, generic 00-alpha hex,
"transparent" keyword, "none" keyword, plus a sanity case showing
partially-transparent #FF000080 still counts as visible) and 2
end-to-end tests for resolveTreePostPass's button contrast pass
against path icons whose fill is #00000000:
- stroke-style icon (stroke without color) → stroke color assigned
- fill-style icon (no stroke) → fill color assigned
apps/web role-resolver: 87 committed tests, all green.
This commit is contained in:
parent
ef92774700
commit
b7639dadde
2 changed files with 130 additions and 1 deletions
|
|
@ -65,6 +65,107 @@ describe('hasFill', () => {
|
|||
} as PenNode;
|
||||
expect(hasFill(node)).toBe(true);
|
||||
});
|
||||
|
||||
// Regression: button foreground contrast relies on hasFill to detect
|
||||
// "no visible fill here, please supply one". A path icon whose fill
|
||||
// was normalized to the explicit-transparent hex #00000000 (to
|
||||
// preserve hollow-shape intent on activity rings / chart lines) must
|
||||
// still be reported as "no fill" so the contrast pass can paint it.
|
||||
it('returns false for 8-digit transparent hex (#00000000)', () => {
|
||||
const node = {
|
||||
id: 'p', type: 'path', x: 0, y: 0, width: 24, height: 24,
|
||||
fill: [{ type: 'solid', color: '#00000000' }],
|
||||
} as PenNode;
|
||||
expect(hasFill(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for any 8-digit hex with 00 alpha', () => {
|
||||
const node = {
|
||||
id: 'p', type: 'path', x: 0, y: 0, width: 24, height: 24,
|
||||
fill: [{ type: 'solid', color: '#FF00FF00' }],
|
||||
} as PenNode;
|
||||
expect(hasFill(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for CSS keyword "transparent"', () => {
|
||||
const node = {
|
||||
id: 'p', type: 'path', x: 0, y: 0, width: 24, height: 24,
|
||||
fill: [{ type: 'solid', color: 'transparent' }],
|
||||
} as PenNode;
|
||||
expect(hasFill(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for CSS keyword "none"', () => {
|
||||
const node = {
|
||||
id: 'p', type: 'path', x: 0, y: 0, width: 24, height: 24,
|
||||
fill: [{ type: 'solid', color: 'none' }],
|
||||
} as PenNode;
|
||||
expect(hasFill(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true for a partially-transparent 8-digit hex (non-zero alpha)', () => {
|
||||
const node = {
|
||||
id: 'n', type: 'frame', x: 0, y: 0, width: 100, height: 100,
|
||||
fill: [{ type: 'solid', color: '#FF000080' }],
|
||||
} as PenNode;
|
||||
expect(hasFill(node)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveTreePostPass — button foreground contrast with transparent-hex path icon', () => {
|
||||
// The real failure the normalizeStrokeFillSchema fix had to address:
|
||||
// an AI-generated stroke-style line icon inside a button, where the
|
||||
// AI wrote `fill: [{color: "none"}]` and the normalizer substituted
|
||||
// `#00000000` to preserve hollow intent. The button contrast pass
|
||||
// must still see "no visible fill" and supply a visible stroke color.
|
||||
it('paints stroke on a path icon whose fill is 8-digit transparent hex', () => {
|
||||
const button: PenNode = {
|
||||
id: 'btn',
|
||||
type: 'frame',
|
||||
name: 'Icon Button',
|
||||
x: 0, y: 0, width: 44, height: 44,
|
||||
role: 'icon-button',
|
||||
fill: [{ type: 'solid', color: '#1E293B' }],
|
||||
children: [
|
||||
{
|
||||
id: 'p', type: 'path', name: 'Arrow', x: 0, y: 0, width: 24, height: 24,
|
||||
fill: [{ type: 'solid', color: '#00000000' }],
|
||||
stroke: { thickness: 2 },
|
||||
} as PenNode,
|
||||
],
|
||||
} as PenNode;
|
||||
const root: PenNode = {
|
||||
id: 'root', type: 'frame', name: 'Root', x: 0, y: 0, width: 375, height: 812,
|
||||
children: [button],
|
||||
} as PenNode;
|
||||
resolveTreePostPass(root, 375);
|
||||
const p = (root as any).children[0].children[0];
|
||||
expect(p.stroke.fill).toEqual([{ type: 'solid', color: '#FFFFFF' }]);
|
||||
});
|
||||
|
||||
it('paints fill on a path icon whose only fill is transparent and has no stroke', () => {
|
||||
const button: PenNode = {
|
||||
id: 'btn',
|
||||
type: 'frame',
|
||||
name: 'Icon Button',
|
||||
x: 0, y: 0, width: 44, height: 44,
|
||||
role: 'icon-button',
|
||||
fill: [{ type: 'solid', color: '#2563EB' }],
|
||||
children: [
|
||||
{
|
||||
id: 'p', type: 'path', name: 'Star', x: 0, y: 0, width: 24, height: 24,
|
||||
fill: [{ type: 'solid', color: '#00000000' }],
|
||||
} as PenNode,
|
||||
],
|
||||
} as PenNode;
|
||||
const root: PenNode = {
|
||||
id: 'root', type: 'frame', name: 'Root', x: 0, y: 0, width: 375, height: 812,
|
||||
children: [button],
|
||||
} as PenNode;
|
||||
resolveTreePostPass(root, 375);
|
||||
const p = (root as any).children[0].children[0];
|
||||
expect(p.fill).toEqual([{ type: 'solid', color: '#FFFFFF' }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveTreePostPass — button foreground contrast', () => {
|
||||
|
|
|
|||
|
|
@ -482,8 +482,36 @@ export function hexLuminance(hex: string): number {
|
|||
* Check if a node has a non-empty fill array.
|
||||
* Does NOT distinguish AI-explicit from role-default fills.
|
||||
*/
|
||||
/**
|
||||
* Returns true when a node has a VISIBLE fill.
|
||||
*
|
||||
* The subtle bit is the "visible" qualifier: an explicit-transparent hex
|
||||
* (`#00000000`), a fully-transparent 8-digit hex (any `#RRGGBB00`), or a
|
||||
* CSS keyword (`"transparent"` / `"none"`) all satisfy the naive
|
||||
* `fill.length > 0` test but draw nothing. Downstream heuristics like
|
||||
* `fixButtonForegroundContrast` ask this function "does this path icon
|
||||
* already have a color?" expecting a truthful answer. A normalized
|
||||
* transparent fill (the placeholder the stroke/fill schema normalizer
|
||||
* leaves on hollow shapes to stop the renderer from falling back to
|
||||
* the default gray) must be reported as "no visible fill" so the
|
||||
* contrast pass can paint in a foreground color.
|
||||
*/
|
||||
export function hasFill(node: PenNode): boolean {
|
||||
return 'fill' in node && Array.isArray(node.fill) && node.fill.length > 0;
|
||||
if (!('fill' in node) || !Array.isArray(node.fill) || node.fill.length === 0) return false;
|
||||
const first = node.fill[0];
|
||||
if (!first) return false;
|
||||
if (first.type === 'solid' && isInvisibleColor(first.color)) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
function isInvisibleColor(color: unknown): boolean {
|
||||
if (typeof color !== 'string') return false;
|
||||
const c = color.trim().toLowerCase();
|
||||
if (c === 'transparent' || c === 'none') return true;
|
||||
// 8-digit hex with 00 alpha (#RRGGBB00). Valid hex color literal, but
|
||||
// it draws nothing.
|
||||
if (/^#[0-9a-f]{6}00$/i.test(c)) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Reference in a new issue