mirror of
https://github.com/ZSeven-W/openpencil.git
synced 2026-05-31 19:04:29 +07:00
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:
parent
239f635d2e
commit
ef92774700
2 changed files with 127 additions and 12 deletions
|
|
@ -36,6 +36,25 @@ const frame = (props: NodeProps & { children?: PenNode[] } = {}): PenNode =>
|
|||
...props,
|
||||
}) 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' }];
|
||||
|
||||
describe('normalizeStrokeFillSchema — stroke array unwrap', () => {
|
||||
|
|
@ -171,6 +190,77 @@ describe('normalizeStrokeFillSchema — illegal fill color replacement', () => {
|
|||
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', () => {
|
||||
const node = path({
|
||||
stroke: { thickness: 2, fill: [{ type: 'solid', color: 'none' }] },
|
||||
|
|
|
|||
|
|
@ -138,15 +138,28 @@ function stripIllegalColorsFromStrokeFill(node: PenNode): void {
|
|||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Explicit transparent hex. Used when we need to preserve a node's
|
||||
* "no fill" intent but cannot leave the fill field absent (which would
|
||||
* make canvas-object-factory fall back to an opaque default gray fill).
|
||||
* Explicit transparent hex. Used for SHAPE fills (frame, rectangle,
|
||||
* ellipse, path, group…) where "no fill" really means a hollow shape
|
||||
* 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 = {
|
||||
type: 'solid',
|
||||
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 {
|
||||
const rec = node as unknown as { fill?: unknown };
|
||||
const raw = rec.fill;
|
||||
|
|
@ -158,16 +171,28 @@ function normalizeNodeFill(node: PenNode): void {
|
|||
rec.fill = cleaned as PenFill[];
|
||||
return;
|
||||
}
|
||||
// Every original entry was a CSS keyword ("none" / "transparent").
|
||||
// The AI's intent was "no fill" — but DELETING the field would let
|
||||
// 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.
|
||||
// Empty in, empty out — leave unchanged.
|
||||
if (raw.length === 0) {
|
||||
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[];
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue