From 91d0ca44ac0650bbc83b97295f79ebebd09987ca Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Thu, 18 Jun 2026 09:05:33 +0200 Subject: [PATCH] fix: only tint single-tone grayscale SVGs A grayscale SVG can draw its background as a full-canvas path (not just a rect), e.g. a white background path plus a black glyph. The rect-only background check missed that, and the icon flattened to a solid currentColor block under the CSS mask. Tint only when the SVG resolves to a single grayscale tone. Any second tone (a background shape drawn as a path or rect, an accent, or a second shade) now preserves the icon's own colors, which covers full-canvas path backgrounds without per-shape geometry parsing. --- client/src/utils/__tests__/svg.test.ts | 31 +++++++++--- client/src/utils/svg.ts | 65 +++++++++++++++----------- 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/client/src/utils/__tests__/svg.test.ts b/client/src/utils/__tests__/svg.test.ts index 9a200e9166..3d72abf127 100644 --- a/client/src/utils/__tests__/svg.test.ts +++ b/client/src/utils/__tests__/svg.test.ts @@ -17,13 +17,14 @@ describe('isMonochromeSvg', () => { expect(isMonochromeSvg(svg)).toBe(true); }); - it('treats grayscale shades as monochrome', () => { - const svg = ''; + it('treats one grayscale tone written several ways as monochrome', () => { + const svg = + ''; expect(isMonochromeSvg(svg)).toBe(true); }); - it('treats named grayscale colors as monochrome', () => { - const svg = ''; + it('treats a single named grayscale color as monochrome', () => { + const svg = ''; expect(isMonochromeSvg(svg)).toBe(true); }); @@ -43,12 +44,30 @@ describe('isMonochromeSvg', () => { expect(isMonochromeSvg(svg)).toBe(true); }); - it('handles colors defined inside a style block', () => { - const svg = ''; + it('handles a single color defined inside a style block', () => { + const svg = ''; expect(isMonochromeSvg(svg)).toBe(true); }); }); + describe('multiple grayscale tones (conservatively preserved)', () => { + it('preserves an SVG with two grayscale shades', () => { + const svg = ''; + expect(isMonochromeSvg(svg)).toBe(false); + }); + + it('preserves a black-and-white two-tone glyph', () => { + const svg = ''; + expect(isMonochromeSvg(svg)).toBe(false); + }); + + it('rejects a full-canvas path background with a glyph', () => { + const svg = + ''; + expect(isMonochromeSvg(svg)).toBe(false); + }); + }); + describe('multi-color icons (colors preserved)', () => { it('treats a saturated hex color as multi-color', () => { const svg = ''; diff --git a/client/src/utils/svg.ts b/client/src/utils/svg.ts index 51e00c3298..4a04ecdbdb 100644 --- a/client/src/utils/svg.ts +++ b/client/src/utils/svg.ts @@ -11,21 +11,21 @@ import DOMPurify from 'dompurify'; /** Color keywords that carry no chromatic information and are ignored. */ const IGNORABLE_COLORS = new Set(['none', 'transparent', 'inherit', 'currentcolor']); -/** Named CSS colors that are pure grayscale. Unknown names are treated as chromatic. */ -const GRAY_NAMES = new Set([ - 'black', - 'white', - 'gray', - 'grey', - 'silver', - 'gainsboro', - 'whitesmoke', - 'lightgray', - 'lightgrey', - 'darkgray', - 'darkgrey', - 'dimgray', - 'dimgrey', +/** Named CSS grayscale colors mapped to their 0-255 level. Unknown names are chromatic. */ +const GRAY_LEVELS = new Map([ + ['black', 0], + ['white', 255], + ['gray', 128], + ['grey', 128], + ['silver', 192], + ['gainsboro', 220], + ['whitesmoke', 245], + ['lightgray', 211], + ['lightgrey', 211], + ['darkgray', 169], + ['darkgrey', 169], + ['dimgray', 105], + ['dimgrey', 105], ]); /** Paint properties whose color values determine whether an SVG is monochrome. */ @@ -74,24 +74,29 @@ function functionalToValues(color: string): number[] | null { return values.some(Number.isNaN) ? null : values; } -function isGrayscaleColor(color: string): boolean { +/** Returns the 0-255 gray level of a grayscale color, or null when it is chromatic. */ +function grayLevel(color: string): number | null { if (color.startsWith('#')) { const rgb = hexToRgb(color); - return rgb ? rgb[0] === rgb[1] && rgb[1] === rgb[2] : false; + return rgb && rgb[0] === rgb[1] && rgb[1] === rgb[2] ? rgb[0] : null; } if (color.startsWith('rgb')) { const rgb = functionalToValues(color); if (!rgb) { - return false; + return null; } const [r, g, b] = rgb.map(Math.round); - return r === g && g === b; + return r === g && g === b ? r : null; } if (color.startsWith('hsl')) { const hsl = functionalToValues(color); - return hsl ? hsl[1] === 0 : false; + if (!hsl) { + return null; + } + return hsl[1] === 0 ? Math.round(hsl[2]) : null; } - return GRAY_NAMES.has(color); + const named = GRAY_LEVELS.get(color); + return named === undefined ? null : named; } function parseSvgRoot(svg: string): Element | null { @@ -233,10 +238,10 @@ function collectColors(root: Element): string[] { /** * Returns true when an SVG can be safely tinted to match the theme: it embeds no * raster (``) or foreign (``) content, has no opaque - * background, and every paint color is grayscale (or it relies on the default - * black fill / `currentColor`). Multi-color logos, icons with an opaque - * background, embedded rasters, and anything unparseable return false so they - * keep their own colors. + * background, and resolves to a single grayscale tone (or relies on the default + * black fill / `currentColor`). Anything with a second tone (a background shape, + * an accent, or a second shade), a chromatic color, or that is unparseable + * returns false, since a CSS mask would flatten it to a solid block. */ export function isMonochromeSvg(svg: string): boolean { const root = parseSvgRoot(svg); @@ -253,7 +258,15 @@ export function isMonochromeSvg(svg: string): boolean { if (colors.length === 0) { return true; } - return colors.every(isGrayscaleColor); + const levels = new Set(); + for (const color of colors) { + const level = grayLevel(color); + if (level === null) { + return false; + } + levels.add(level); + } + return levels.size <= 1; } /**