fix(ai): split fill normalization by shape vs foreground node type

Follow-up on 239f635. The transparent-hex replacement for illegal CSS
keyword fills (none / transparent) was applied uniformly to every node
type, but text.fill and icon_font.fill are FOREGROUND colors, not
shape backgrounds:

    text.fill = [{color: "none"}]        → AI meant "default text color"
    ↓ previous normalizer ↓
    text.fill = [{color: "#00000000"}]   → invisible text
    ↓ canvas render ↓
    text is transparent — content lost

Split the behavior by node type:

  FOREGROUND types (text, icon_font)
    fill is the foreground color. An illegal "none" almost certainly
    means "I forgot to pick a color" — delete the field so downstream
    layers (role defaults, button contrast heuristic, style inheritance)
    can populate something visible. Setting it transparent here would
    freeze the content invisible.

  SHAPE types (frame, rectangle, ellipse, path, group, ...)
    fill is a background. "no fill" is a valid hollow-shape intent
    (activity rings, chart line backdrops). Keep the explicit
    #00000000 replacement so canvas-object-factory does not fall back
    to the default gray.

Added 6 TDD cases to normalize-stroke-fill-schema.test.ts:
  - text with sole "none" fill → fill deleted
  - text with sole "transparent" fill → fill deleted
  - icon_font with "none" → fill deleted
  - text with legitimate hex → unchanged
  - text with mixed [none, #00FF00] → drops illegal, keeps #00FF00
  - sanity guard across frame/ellipse/path → still gets #00000000

pen-core suite: 195 committed tests, all green.
This commit is contained in:
Fini 2026-04-06 20:01:38 +08:00
parent 239f635d2e
commit ef92774700
2 changed files with 127 additions and 12 deletions

View file

@ -36,6 +36,25 @@ const frame = (props: NodeProps & { children?: PenNode[] } = {}): PenNode =>
...props, ...props,
}) as unknown as PenNode; }) as unknown as PenNode;
const text = (props: NodeProps = {}): PenNode =>
({
id: 't',
type: 'text',
content: 'Hello',
fontSize: 16,
...props,
}) as unknown as PenNode;
const iconFont = (props: NodeProps = {}): PenNode =>
({
id: 'i',
type: 'icon_font',
iconFontName: 'heart',
width: 16,
height: 16,
...props,
}) as unknown as PenNode;
const validFill = [{ type: 'solid' as const, color: '#C4F82A' }]; const validFill = [{ type: 'solid' as const, color: '#C4F82A' }];
describe('normalizeStrokeFillSchema — stroke array unwrap', () => { describe('normalizeStrokeFillSchema — stroke array unwrap', () => {
@ -171,6 +190,77 @@ describe('normalizeStrokeFillSchema — illegal fill color replacement', () => {
expect(rec.fill?.[0]?.color).toBe('#00000000'); expect(rec.fill?.[0]?.color).toBe('#00000000');
}); });
it('DELETES text node fill when all entries are CSS keywords', () => {
// text.fill is the foreground color, not a shape background.
// "none"/"transparent" on a text node would hide the text entirely —
// almost certainly a mistake. Delete the field so downstream layers
// (role defaults, button contrast heuristic) can fill in a visible
// color. Replacing with #00000000 would freeze the text invisible.
const node = text({
fill: [{ type: 'solid', color: 'none' }],
});
normalizeStrokeFillSchema(node);
const rec = node as unknown as { fill?: unknown };
expect(rec.fill).toBeUndefined();
});
it('DELETES text node fill for "transparent" too', () => {
const node = text({
fill: [{ type: 'solid', color: 'transparent' }],
});
normalizeStrokeFillSchema(node);
const rec = node as unknown as { fill?: unknown };
expect(rec.fill).toBeUndefined();
});
it('DELETES icon_font fill when all entries are CSS keywords', () => {
// Same reasoning: icon_font.fill is the foreground color.
const node = iconFont({
fill: [{ type: 'solid', color: 'none' }],
});
normalizeStrokeFillSchema(node);
const rec = node as unknown as { fill?: unknown };
expect(rec.fill).toBeUndefined();
});
it('still keeps a legitimate text fill unchanged', () => {
const node = text({
fill: [{ type: 'solid', color: '#FFFFFF' }],
});
normalizeStrokeFillSchema(node);
const rec = node as unknown as { fill?: Array<{ color?: string }> };
expect(rec.fill?.[0]?.color).toBe('#FFFFFF');
});
it('still drops illegal entries from text but preserves legal siblings', () => {
const node = text({
fill: [
{ type: 'solid', color: 'none' },
{ type: 'solid', color: '#00FF00' },
],
});
normalizeStrokeFillSchema(node);
const rec = node as unknown as { fill?: Array<{ color?: string }> };
expect(rec.fill).toHaveLength(1);
expect(rec.fill?.[0]?.color).toBe('#00FF00');
});
it('keeps the transparent-hex replacement for shape fills (frame / ellipse / path)', () => {
// Sanity guard: the shape-vs-foreground split must not regress the
// shape branch. Frames, ellipses, and paths still get the explicit
// #00000000 when an AI writes "none".
for (const node of [
frame({ fill: [{ type: 'solid', color: 'none' }] }),
ellipse({ fill: [{ type: 'solid', color: 'none' }] }),
path({ fill: [{ type: 'solid', color: 'transparent' }] }),
]) {
normalizeStrokeFillSchema(node);
const rec = node as unknown as { fill?: Array<{ color?: string }> };
expect(rec.fill).toHaveLength(1);
expect(rec.fill?.[0]?.color?.toLowerCase()).toBe('#00000000');
}
});
it('also strips illegal colors from stroke.fill arrays', () => { it('also strips illegal colors from stroke.fill arrays', () => {
const node = path({ const node = path({
stroke: { thickness: 2, fill: [{ type: 'solid', color: 'none' }] }, stroke: { thickness: 2, fill: [{ type: 'solid', color: 'none' }] },

View file

@ -138,15 +138,28 @@ function stripIllegalColorsFromStrokeFill(node: PenNode): void {
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
/** /**
* Explicit transparent hex. Used when we need to preserve a node's * Explicit transparent hex. Used for SHAPE fills (frame, rectangle,
* "no fill" intent but cannot leave the fill field absent (which would * ellipse, path, group) where "no fill" really means a hollow shape
* make canvas-object-factory fall back to an opaque default gray fill). * that should let the background show through. We write the 8-digit
* transparent hex so canvas-object-factory doesn't fall back to an
* opaque default gray fill.
*/ */
const EXPLICIT_TRANSPARENT_FILL: SolidFill = { const EXPLICIT_TRANSPARENT_FILL: SolidFill = {
type: 'solid', type: 'solid',
color: '#00000000', color: '#00000000',
}; };
/**
* Node types whose `fill` represents a FOREGROUND color (text color,
* icon color) rather than a shape's background. On these types an
* illegal "none" / "transparent" fill is almost certainly a mistake:
* the user meant "default text color", not "invisible text". Freezing
* them to #00000000 would hide the content entirely, so we delete the
* field and let downstream layers (role defaults, button contrast,
* style inheritance) supply a visible color.
*/
const FOREGROUND_NODE_TYPES = new Set<string>(['text', 'icon_font']);
function normalizeNodeFill(node: PenNode): void { function normalizeNodeFill(node: PenNode): void {
const rec = node as unknown as { fill?: unknown }; const rec = node as unknown as { fill?: unknown };
const raw = rec.fill; const raw = rec.fill;
@ -158,16 +171,28 @@ function normalizeNodeFill(node: PenNode): void {
rec.fill = cleaned as PenFill[]; rec.fill = cleaned as PenFill[];
return; return;
} }
// Every original entry was a CSS keyword ("none" / "transparent"). // Empty in, empty out — leave unchanged.
// The AI's intent was "no fill" — but DELETING the field would let if (raw.length === 0) {
// canvas-object-factory fall back to its default opaque gray fill,
// which is the opposite of no-fill. Replace with an explicit
// transparent hex so the renderer honours the intent.
if (raw.length > 0) {
rec.fill = [EXPLICIT_TRANSPARENT_FILL] as PenFill[];
} else {
// Empty array in, empty array out — leave unchanged.
rec.fill = [] as PenFill[]; rec.fill = [] as PenFill[];
return;
}
// Every original entry was a CSS keyword ("none" / "transparent").
// The correct repair depends on whether `fill` is a background or a
// foreground color on this node type:
//
// SHAPE types (frame, rectangle, ellipse, path, group, …)
// fill = shape background. "no fill" means hollow. Keep the
// explicit transparent hex so canvas doesn't fall back to the
// default gray fill.
//
// FOREGROUND types (text, icon_font)
// fill = text/icon color. "no fill" would hide the content —
// almost certainly not what the AI meant. Delete the field so
// downstream layers can populate a visible color.
if (FOREGROUND_NODE_TYPES.has(node.type)) {
delete rec.fill;
} else {
rec.fill = [EXPLICIT_TRANSPARENT_FILL] as PenFill[];
} }
} }