From 4ef6667ffc963ee77aa3c864fa7a592d708e5885 Mon Sep 17 00:00:00 2001 From: mrhard9090 <74858909+mrhard9090@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:32:34 +0300 Subject: [PATCH] fix(tools): judge describe text contrast by WCAG ratio (#758) The describe tool judges text contrast by the WCAG 2 ratio (4.5:1, or 3:1 for large text) instead of a dark-on-dark luminance heuristic, and reports the ratio and threshold. It measures translucent and faded text as drawn, treats text as large only when the base style and every style run are, and skips variable-bound colors. contrastRatio() and compositeOver() now live in @open-pencil/scene-graph/color and are shared with the color-contrast lint rule and canvas labels. Fixes #735. --- CHANGELOG.md | 1 + packages/core/src/canvas/labels/color.ts | 30 +----- .../core/src/lint/rules/color-contrast.ts | 3 +- packages/core/src/lint/utils.ts | 18 ---- packages/core/src/tools/describe/issues.ts | 2 +- .../core/src/tools/describe/layout-issues.ts | 47 ++++++--- packages/scene-graph/src/color/contrast.ts | 25 +++++ packages/scene-graph/src/color/index.ts | 1 + .../scene-graph/tests/color/contrast.test.ts | 33 +++++++ tests/engine/tools/describe.test.ts | 99 ++++++++++++++++++- 10 files changed, 199 insertions(+), 60 deletions(-) create mode 100644 packages/scene-graph/src/color/contrast.ts create mode 100644 packages/scene-graph/tests/color/contrast.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f89222931..b1dfdccc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ ### Fixed +- Judge text contrast in the AI and MCP `describe` tool by its WCAG 2 ratio (4.5:1, or 3:1 for large text), the same ratio the `color-contrast` lint rule computes. It no longer reports passing dark text on mid-tone backgrounds as "dark on dark", now reports low-contrast light text, measures translucent and faded text as it is drawn, skips text whose color is bound to a variable, and says the ratio and the threshold it missed (#735). - Warn about options the paint and effect helpers ignore when rendering JSX instead of dropping them silently, and point `blur` in effect helpers at `radius`, the name Figma uses (#736). - Size groups and boolean operations made through the AI and MCP `group_nodes` and `boolean_*` tools to what they contain, as the editor's commands already do, instead of a default 100 × 100 box or the first operand's box (#738). - Keep a layer where it is drawn when it moves into or out of a rotated or flipped parent, instead of shifting it and leaving it at its old angle (#737). diff --git a/packages/core/src/canvas/labels/color.ts b/packages/core/src/canvas/labels/color.ts index 344bd0273..276e1c476 100644 --- a/packages/core/src/canvas/labels/color.ts +++ b/packages/core/src/canvas/labels/color.ts @@ -1,35 +1,11 @@ -import { wcagContrast } from 'culori' - import type { Color } from '@open-pencil/scene-graph' +import { compositeOver, contrastRatio } from '@open-pencil/scene-graph/color' import { BLACK } from '#core/constants' const WHITE: Color = { r: 1, g: 1, b: 1, a: 1 } -function compositeColor(foreground: Color, background: Color): Color { - const alpha = foreground.a - return { - r: foreground.r * alpha + background.r * (1 - alpha), - g: foreground.g * alpha + background.g * (1 - alpha), - b: foreground.b * alpha + background.b * (1 - alpha), - a: 1 - } -} - export function canvasLabelForeground(background: Color, canvasBackground: Color = WHITE): Color { - const composited = compositeColor(background, canvasBackground) - const color = { mode: 'rgb' as const, r: composited.r, g: composited.g, b: composited.b } - const blackContrast = wcagContrast(color, { - mode: 'rgb', - r: BLACK.r, - g: BLACK.g, - b: BLACK.b - }) - const whiteContrast = wcagContrast(color, { - mode: 'rgb', - r: WHITE.r, - g: WHITE.g, - b: WHITE.b - }) - return blackContrast >= whiteContrast ? BLACK : WHITE + const composited = compositeOver(background, canvasBackground) + return contrastRatio(composited, BLACK) >= contrastRatio(composited, WHITE) ? BLACK : WHITE } diff --git a/packages/core/src/lint/rules/color-contrast.ts b/packages/core/src/lint/rules/color-contrast.ts index 7b477e945..3701c98b9 100644 --- a/packages/core/src/lint/rules/color-contrast.ts +++ b/packages/core/src/lint/rules/color-contrast.ts @@ -1,5 +1,6 @@ +import { contrastRatio } from '@open-pencil/scene-graph/color' + import { defineRule } from '#core/lint/rule' -import { contrastRatio } from '#core/lint/utils' export default defineRule({ meta: { diff --git a/packages/core/src/lint/utils.ts b/packages/core/src/lint/utils.ts index 4be5a5b64..baf8bfc1c 100644 --- a/packages/core/src/lint/utils.ts +++ b/packages/core/src/lint/utils.ts @@ -25,22 +25,4 @@ export function getNodePath(node: LintPathNode): string[] { return path } -export function relativeLuminance(rgb: { r: number; g: number; b: number }): number { - const [r, g, b] = [rgb.r, rgb.g, rgb.b].map((c) => - c <= 0.03928 ? c / 12.92 : ((c + 0.055) / 1.055) ** 2.4 - ) as [number, number, number] - return 0.2126 * r + 0.7152 * g + 0.0722 * b -} - -export function contrastRatio( - a: { r: number; g: number; b: number }, - b: { r: number; g: number; b: number } -): number { - const l1 = relativeLuminance(a) - const l2 = relativeLuminance(b) - const lighter = Math.max(l1, l2) - const darker = Math.min(l1, l2) - return (lighter + 0.05) / (darker + 0.05) -} - export const SPACING_SCALE = [0, 1, 2, 4, 8, 12, 16, 20, 24, 32, 40, 48, 56, 64, 80, 96, 128] diff --git a/packages/core/src/tools/describe/issues.ts b/packages/core/src/tools/describe/issues.ts index b23a207d2..a37c6ef4e 100644 --- a/packages/core/src/tools/describe/issues.ts +++ b/packages/core/src/tools/describe/issues.ts @@ -402,7 +402,7 @@ const ERROR_PATTERNS = [ /no color/i, /collapses/i, /no fill and no stroke/i, - /dark on dark/i, + /below WCAG AA/i, /Touch target too small/i, /Nested Text/i ] diff --git a/packages/core/src/tools/describe/layout-issues.ts b/packages/core/src/tools/describe/layout-issues.ts index 37a3ff6b4..3fa3a63ea 100644 --- a/packages/core/src/tools/describe/layout-issues.ts +++ b/packages/core/src/tools/describe/layout-issues.ts @@ -1,17 +1,36 @@ -import { wcagLuminance } from 'culori' import { sumBy } from 'es-toolkit/math' -import type { SceneGraph, SceneNode } from '@open-pencil/scene-graph' -import { colorToHex } from '@open-pencil/scene-graph/color' +import type { Fill, SceneGraph, SceneNode } from '@open-pencil/scene-graph' +import { colorToHex, compositeOver, contrastRatio } from '@open-pencil/scene-graph/color' import type { Color } from '@open-pencil/scene-graph/primitives' import type { DescribeIssue } from './issues' import { CONTAINER_TYPES, findAncestorBackground } from './shared' -const DARK_BG_LUMINANCE = 0.35 +const WCAG_AA_CONTRAST = 4.5 +const WCAG_AA_LARGE_TEXT_CONTRAST = 3 -function rgbLuminance(c: Color): number { - return wcagLuminance({ mode: 'rgb', r: c.r, g: c.g, b: c.b }) +// Translucent text, through its fill or the node's own opacity, is seen blended with the background. +function blendOver(fill: Fill, nodeOpacity: number, background: Color): Color { + return compositeOver(fill.color, background, fill.opacity * fill.color.a * nodeOpacity) +} + +// Round down so a failing ratio such as 4.499 is not shown as 4.50. +function formatRatio(ratio: number): string { + return (Math.floor(ratio * 100) / 100).toFixed(2) +} + +// WCAG large text is at least 18pt, or 14pt bold (1pt = 4/3px). +function isLargeSize(fontSize: number, fontWeight: number): boolean { + return fontSize >= 24 || (fontSize >= 56 / 3 && fontWeight >= 700) +} + +// Text qualifies only when every range does: the base style and each style run's own size and weight. +function isLargeText(node: SceneNode): boolean { + if (!isLargeSize(node.fontSize, node.fontWeight)) return false + return node.styleRuns.every(({ style }) => + isLargeSize(style.fontSize ?? node.fontSize, style.fontWeight ?? node.fontWeight) + ) } interface LayoutContext { @@ -204,7 +223,8 @@ function checkTextVisibility(ctx: LayoutContext): void { for (const childId of node.childIds) { const child = graph.getNode(childId) if (!child?.visible || child.type !== 'TEXT') continue - const textFill = child.fills.find((f) => f.visible && f.type === 'SOLID') + const textFillIndex = child.fills.findIndex((f) => f.visible && f.type === 'SOLID') + const textFill = child.fills[textFillIndex] as Fill | undefined if (!textFill) { issues.push({ message: `"${child.name || child.text.slice(0, 20) || 'Text'}" has no color — invisible`, @@ -212,14 +232,17 @@ function checkTextVisibility(ctx: LayoutContext): void { }) continue } - const textLum = rgbLuminance(textFill.color) - if (textLum > DARK_BG_LUMINANCE) continue + // A variable-bound color depends on the active mode, so its static value proves nothing; + // the color-contrast lint rule skips it for the same reason. + if (child.boundVariables[`fills/${textFillIndex}/color`]) continue const bg = findAncestorBackground(child, graph) if (!bg) continue - if (rgbLuminance(bg) < DARK_BG_LUMINANCE) { + const ratio = contrastRatio(blendOver(textFill, child.opacity, bg), bg) + const required = isLargeText(child) ? WCAG_AA_LARGE_TEXT_CONTRAST : WCAG_AA_CONTRAST + if (ratio < required) { issues.push({ - message: `"${child.name || child.text.slice(0, 20) || 'Text'}" dark on dark (${colorToHex(textFill.color)} on ${colorToHex(bg)})`, - suggestion: 'Use a light color' + message: `"${child.name || child.text.slice(0, 20) || 'Text'}" contrast ${formatRatio(ratio)}:1 is below WCAG AA ${required}:1 (${colorToHex(textFill.color)} on ${colorToHex(bg)})`, + suggestion: 'Increase contrast between text and background' }) } } diff --git a/packages/scene-graph/src/color/contrast.ts b/packages/scene-graph/src/color/contrast.ts new file mode 100644 index 000000000..03be5421b --- /dev/null +++ b/packages/scene-graph/src/color/contrast.ts @@ -0,0 +1,25 @@ +import { wcagContrast } from 'culori' + +import type { Color } from '../primitives' + +/** Opaque color seen when `foreground` at `alpha` is drawn over an opaque `background`. */ +export function compositeOver(foreground: Color, background: Color, alpha = foreground.a): Color { + const mix = (top: number, bottom: number) => top * alpha + bottom * (1 - alpha) + return { + r: mix(foreground.r, background.r), + g: mix(foreground.g, background.g), + b: mix(foreground.b, background.b), + a: 1 + } +} + +/** WCAG 2 contrast ratio of two colors, from 1 to 21; alpha is ignored, so composite first. */ +export function contrastRatio( + a: Pick, + b: Pick +): number { + return wcagContrast( + { mode: 'rgb', r: a.r, g: a.g, b: a.b }, + { mode: 'rgb', r: b.r, g: b.g, b: b.b } + ) +} diff --git a/packages/scene-graph/src/color/index.ts b/packages/scene-graph/src/color/index.ts index bb3f283a4..fa9342cc0 100644 --- a/packages/scene-graph/src/color/index.ts +++ b/packages/scene-graph/src/color/index.ts @@ -1,3 +1,4 @@ +export * from './contrast' export * from './management' export { normalizeColor } from './normalize' export * from './okhcl' diff --git a/packages/scene-graph/tests/color/contrast.test.ts b/packages/scene-graph/tests/color/contrast.test.ts new file mode 100644 index 000000000..d65dc7a3d --- /dev/null +++ b/packages/scene-graph/tests/color/contrast.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, test } from 'bun:test' + +import { compositeOver, contrastRatio } from '@open-pencil/scene-graph/color' +import type { Color } from '@open-pencil/scene-graph/primitives' + +const BLACK: Color = { r: 0, g: 0, b: 0, a: 1 } +const WHITE: Color = { r: 1, g: 1, b: 1, a: 1 } +const GRAY_94: Color = { r: 0x94 / 255, g: 0x94 / 255, b: 0x94 / 255, a: 1 } + +describe('contrastRatio', () => { + test('matches the WCAG 2 extremes and is symmetric', () => { + expect(contrastRatio(BLACK, WHITE)).toBeCloseTo(21, 6) + expect(contrastRatio(WHITE, BLACK)).toBeCloseTo(21, 6) + expect(contrastRatio(WHITE, WHITE)).toBeCloseTo(1, 6) + }) + + test('measures a mid gray on white', () => { + expect(contrastRatio(GRAY_94, WHITE)).toBeCloseTo(3.03, 2) + }) +}) + +describe('compositeOver', () => { + test('uses the foreground alpha by default and returns an opaque color', () => { + expect(compositeOver({ ...BLACK, a: 0.5 }, WHITE)).toEqual({ r: 0.5, g: 0.5, b: 0.5, a: 1 }) + }) + + test('takes an explicit alpha for fill or node opacity', () => { + const blended = compositeOver(BLACK, WHITE, 0.4) + expect(blended.r).toBeCloseTo(0.6, 10) + expect(contrastRatio(blended, WHITE)).toBeCloseTo(2.849, 3) + expect(compositeOver(BLACK, WHITE, 0)).toEqual(WHITE) + }) +}) diff --git a/tests/engine/tools/describe.test.ts b/tests/engine/tools/describe.test.ts index 1e395d904..89c369849 100644 --- a/tests/engine/tools/describe.test.ts +++ b/tests/engine/tools/describe.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from 'bun:test' -import type { Color, Stroke } from '@open-pencil/scene-graph' +import type { Color, Fill, SceneNode, Stroke } from '@open-pencil/scene-graph' import { expectDefined } from '#tests/helpers/assert' import { getTool, setupToolTest, type ToolResult } from '#tests/helpers/tools' @@ -65,3 +65,100 @@ describe('describe stroke summaries', () => { expect(result.visual).toContain('#021A3B 10.13px stroke') }) }) + +interface DescribedIssue { + message: string + severity?: string +} + +function hex(value: string): Color { + const channel = (offset: number) => Number.parseInt(value.slice(offset, offset + 2), 16) / 255 + return { r: channel(1), g: channel(3), b: channel(5), a: 1 } +} + +function solid(color: string, opacity = 1): Fill { + return { type: 'SOLID', color: hex(color), opacity, visible: true } +} + +function describeTextOnBackground( + textColor: string, + backgroundColor: string, + fontSize: number, + fontWeight = 400, + textOpacity = 1, + options: { opacity?: number; boundColor?: string; styleRuns?: SceneNode['styleRuns'] } = {} +): DescribedIssue[] { + const { figma, graph } = setupToolTest() + const frame = figma.createFrame() + frame.name = 'Button' + frame.resize(200, 60) + graph.updateNode(frame.id, { fills: [solid(backgroundColor)] }) + + const label = figma.createText() + label.name = 'Label' + frame.appendChild(label) + graph.updateNode(label.id, { + fills: [solid(textColor, textOpacity)], + fontSize, + fontWeight, + opacity: options.opacity ?? 1, + boundVariables: options.boundColor ? { 'fills/0/color': options.boundColor } : {}, + styleRuns: options.styleRuns ?? [] + }) + + const result = getTool('describe').execute(figma, { id: frame.id }) as ToolResult + const issues = (result.issues ?? []) as DescribedIssue[] + return issues.filter((issue) => issue.message.startsWith('"Label"')) +} + +describe('describe text contrast', () => { + test('accepts dark text on a mid-tone background that passes WCAG AA', () => { + expect(describeTextOnBackground('#14101F', '#FF5FA2', 17, 700)).toEqual([]) + }) + + test('reports the ratio and threshold for text below WCAG AA', () => { + const issues = describeTextOnBackground('#949494', '#FFFFFF', 14) + + expect(issues).toHaveLength(1) + expect(issues[0]).toMatchObject({ + message: '"Label" contrast 3.03:1 is below WCAG AA 4.5:1 (#949494 on #FFFFFF)', + severity: 'error' + }) + }) + + test('measures a translucent text fill as blended with the background', () => { + expect(describeTextOnBackground('#000000', '#FFFFFF', 14)).toEqual([]) + expect(describeTextOnBackground('#000000', '#FFFFFF', 14, 400, 0.4)[0]?.message).toContain( + 'contrast 2.84:1 is below WCAG AA 4.5:1' + ) + }) + + test('uses the 3:1 threshold for large text', () => { + expect(describeTextOnBackground('#949494', '#FFFFFF', 24)).toEqual([]) + expect(describeTextOnBackground('#949494', '#FFFFFF', 19, 700)).toEqual([]) + expect(describeTextOnBackground('#B0B0B0', '#FFFFFF', 24)[0]?.message).toContain( + 'below WCAG AA 3:1' + ) + }) + + test('measures text faded by its own opacity', () => { + expect( + describeTextOnBackground('#000000', '#FFFFFF', 14, 400, 1, { opacity: 0.4 })[0]?.message + ).toContain('contrast 2.84:1 is below WCAG AA 4.5:1') + }) + + test('skips text whose fill color is bound to a variable', () => { + expect( + describeTextOnBackground('#949494', '#FFFFFF', 14, 400, 1, { boundColor: 'v1' }) + ).toEqual([]) + }) + + test('applies the normal threshold when a style run makes part of large text small', () => { + const smallRun = { styleRuns: [{ start: 0, length: 4, style: { fontSize: 14 } }] } + expect( + describeTextOnBackground('#949494', '#FFFFFF', 24, 400, 1, smallRun)[0]?.message + ).toContain('contrast 3.03:1 is below WCAG AA 4.5:1') + const largeRun = { styleRuns: [{ start: 0, length: 4, style: { fontSize: 32 } }] } + expect(describeTextOnBackground('#949494', '#FFFFFF', 24, 400, 1, largeRun)).toEqual([]) + }) +})