From 5ea9dd5ffce4aedc911016f9061a50cf481dcef5 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 00:02:40 +0200 Subject: [PATCH 1/9] [color-swatch] Handle complex color values gracefully --- src/color-swatch/color-swatch.js | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/color-swatch/color-swatch.js b/src/color-swatch/color-swatch.js index b15b9ad..3bcaebf 100644 --- a/src/color-swatch/color-swatch.js +++ b/src/color-swatch/color-swatch.js @@ -23,10 +23,13 @@ const Self = class ColorSwatch extends ColorElement { `; + static resolvedColors = new Map(); + constructor () { super(); this._el = { + swatch: this.shadowRoot.querySelector("slot[name=swatch]::slotted(*), #swatch"), wrapper: this.shadowRoot.querySelector("#wrapper"), label: this.shadowRoot.querySelector("[part=label]"), colorWrapper: this.shadowRoot.querySelector("[part=color]"), @@ -205,7 +208,29 @@ const Self = class ColorSwatch extends ColorElement { return null; } - return ColorSwatch.Color.get(this.value); + try { + return ColorSwatch.Color.get(this.value); + } + catch { + // Color.js can't parse the color value + let color = Self.resolvedColors.get(this.value); + if (color) { + return color; + } + + if (CSS.supports("color", this.value)) { + // One of the supported color values; resolve and cache it + this._el.swatch.style.backgroundColor = this.value; + color = getComputedStyle(this._el.swatch).backgroundColor; + Self.resolvedColors.set(this.value, color); + this._el.swatch.style.backgroundColor = ""; + + return color; + } + + // Not supported or invalid value + return null; + } }, set (value) { this.value = ColorSwatch.Color.get(value)?.display(); From 48dd2e771c15a9295f35b8549a8c9d1d4c0d5ddf Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 00:56:01 +0200 Subject: [PATCH 2/9] Address feedback: Move the code to a util function --- src/color-swatch/color-swatch.js | 21 +++------------------ src/common/util.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/color-swatch/color-swatch.js b/src/color-swatch/color-swatch.js index 3bcaebf..ce92931 100644 --- a/src/color-swatch/color-swatch.js +++ b/src/color-swatch/color-swatch.js @@ -1,4 +1,5 @@ import ColorElement from "../common/color-element.js"; +import { resolveColor } from "../common/util.js"; import "../gamut-badge/gamut-badge.js"; let importIncrementable; @@ -212,24 +213,8 @@ const Self = class ColorSwatch extends ColorElement { return ColorSwatch.Color.get(this.value); } catch { - // Color.js can't parse the color value - let color = Self.resolvedColors.get(this.value); - if (color) { - return color; - } - - if (CSS.supports("color", this.value)) { - // One of the supported color values; resolve and cache it - this._el.swatch.style.backgroundColor = this.value; - color = getComputedStyle(this._el.swatch).backgroundColor; - Self.resolvedColors.set(this.value, color); - this._el.swatch.style.backgroundColor = ""; - - return color; - } - - // Not supported or invalid value - return null; + // Color.js can't parse the color value; possibly one of the values we can handle gracefully + return resolveColor(this.value, this._el.swatch, Self.resolvedColors); } }, set (value) { diff --git a/src/common/util.js b/src/common/util.js index 71c13d2..15edeb1 100644 --- a/src/common/util.js +++ b/src/common/util.js @@ -118,3 +118,32 @@ export function getType (value) { export function noOpTemplateTag (strings, ...values) { return strings.reduce((acc, string, i) => acc + string + (values[i] ?? ""), ""); } + +/** + * Resolve a color value and cache it. + * @param {string} value Color value to resolve. + * @param {Element} element Element to get computed style from to resolve the color value. + * @param {Map} cache + */ +export function resolveColor (value, element, cache) { + if (!value || !element || !(element instanceof Element) || !cache) { + return null; + } + + if (cache.has(value)) { + return cache.get(value); + } + + if (!CSS.supports("color", value)) { + // Not supported or invalid value + return null; + } + + // One of the supported color values; resolve and cache it + element.style.backgroundColor = value; + let color = getComputedStyle(element).backgroundColor; + cache.set(value, color); + element.style.backgroundColor = ""; + + return color; +} From f6f88bc869a27c910e6e4b2bc7d498f435eee072 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 01:19:42 +0200 Subject: [PATCH 3/9] Address feedback: Move to a static method --- src/color-swatch/color-swatch.js | 11 +---------- src/common/color-element.js | 31 +++++++++++++++++++++++++++++++ src/common/util.js | 29 ----------------------------- 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/src/color-swatch/color-swatch.js b/src/color-swatch/color-swatch.js index ce92931..d25ce09 100644 --- a/src/color-swatch/color-swatch.js +++ b/src/color-swatch/color-swatch.js @@ -1,5 +1,4 @@ import ColorElement from "../common/color-element.js"; -import { resolveColor } from "../common/util.js"; import "../gamut-badge/gamut-badge.js"; let importIncrementable; @@ -24,8 +23,6 @@ const Self = class ColorSwatch extends ColorElement { `; - static resolvedColors = new Map(); - constructor () { super(); @@ -209,13 +206,7 @@ const Self = class ColorSwatch extends ColorElement { return null; } - try { - return ColorSwatch.Color.get(this.value); - } - catch { - // Color.js can't parse the color value; possibly one of the values we can handle gracefully - return resolveColor(this.value, this._el.swatch, Self.resolvedColors); - } + return ColorSwatch.resolveColor(this.value, this._el.swatch); }, set (value) { this.value = ColorSwatch.Color.get(value)?.display(); diff --git a/src/common/color-element.js b/src/common/color-element.js index d34b15e..5d66f81 100644 --- a/src/common/color-element.js +++ b/src/common/color-element.js @@ -31,6 +31,7 @@ const Self = class ColorElement extends NudeElement { static Color; static all = {}; static dependencies = new Set(); + static resolvedColors = new Map(); static globalStyles = [{ css: baseGlobalStyles }]; @@ -137,6 +138,36 @@ const Self = class ColorElement extends NudeElement { customElements.define(this.tagName, this); } + + /** + * Resolve a color value and cache it. + * @param {string} value Color value to resolve. + * @param {Element} element Element to get computed style from to resolve the color value. + */ + static resolveColor (value, element) { + try { + return Self.Color.get(value); + } + catch { + // Color.js can't parse the color value; possibly one of the values we can handle gracefully + if (Self.resolvedColors.has(value)) { + return Self.resolvedColors.get(value); + } + + if (!CSS.supports("color", value)) { + // Not supported or invalid value + return null; + } + + // One of the supported color values; resolve and cache it + element.style.backgroundColor = value; + let color = getComputedStyle(element).backgroundColor; + Self.resolvedColors.set(value, color); + element.style.backgroundColor = ""; + + return color; + } + } }; export default Self; diff --git a/src/common/util.js b/src/common/util.js index 15edeb1..71c13d2 100644 --- a/src/common/util.js +++ b/src/common/util.js @@ -118,32 +118,3 @@ export function getType (value) { export function noOpTemplateTag (strings, ...values) { return strings.reduce((acc, string, i) => acc + string + (values[i] ?? ""), ""); } - -/** - * Resolve a color value and cache it. - * @param {string} value Color value to resolve. - * @param {Element} element Element to get computed style from to resolve the color value. - * @param {Map} cache - */ -export function resolveColor (value, element, cache) { - if (!value || !element || !(element instanceof Element) || !cache) { - return null; - } - - if (cache.has(value)) { - return cache.get(value); - } - - if (!CSS.supports("color", value)) { - // Not supported or invalid value - return null; - } - - // One of the supported color values; resolve and cache it - element.style.backgroundColor = value; - let color = getComputedStyle(element).backgroundColor; - cache.set(value, color); - element.style.backgroundColor = ""; - - return color; -} From 92f46d0c7faf89abdd9eb5751e9dc48c0aac3ff4 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 01:42:30 +0200 Subject: [PATCH 4/9] Address feedback: Improve API + add some docs --- src/common/color-element.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/common/color-element.js b/src/common/color-element.js index 5d66f81..4b42978 100644 --- a/src/common/color-element.js +++ b/src/common/color-element.js @@ -31,7 +31,9 @@ const Self = class ColorElement extends NudeElement { static Color; static all = {}; static dependencies = new Set(); - static resolvedColors = new Map(); + + /** @type {Map} */ + static #resolvedColors = new Map(); static globalStyles = [{ css: baseGlobalStyles }]; @@ -143,6 +145,7 @@ const Self = class ColorElement extends NudeElement { * Resolve a color value and cache it. * @param {string} value Color value to resolve. * @param {Element} element Element to get computed style from to resolve the color value. + * @returns {Color | null} Resolved color value or null if the value cannot be resolved. */ static resolveColor (value, element) { try { @@ -150,8 +153,8 @@ const Self = class ColorElement extends NudeElement { } catch { // Color.js can't parse the color value; possibly one of the values we can handle gracefully - if (Self.resolvedColors.has(value)) { - return Self.resolvedColors.get(value); + if (Self.#resolvedColors.has(value)) { + return Self.#resolvedColors.get(value); } if (!CSS.supports("color", value)) { @@ -162,7 +165,8 @@ const Self = class ColorElement extends NudeElement { // One of the supported color values; resolve and cache it element.style.backgroundColor = value; let color = getComputedStyle(element).backgroundColor; - Self.resolvedColors.set(value, color); + color = Self.Color.get(color); + Self.#resolvedColors.set(value, color); element.style.backgroundColor = ""; return color; From 5df5d9378d1ba4c741c4f497b98b2cc415dab858 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 13:46:21 +0200 Subject: [PATCH 5/9] Address feedback --- src/common/color-element.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/common/color-element.js b/src/common/color-element.js index 4b42978..246255d 100644 --- a/src/common/color-element.js +++ b/src/common/color-element.js @@ -32,7 +32,7 @@ const Self = class ColorElement extends NudeElement { static all = {}; static dependencies = new Set(); - /** @type {Map} */ + /** @type {Map} */ static #resolvedColors = new Map(); static globalStyles = [{ css: baseGlobalStyles }]; @@ -154,7 +154,8 @@ const Self = class ColorElement extends NudeElement { catch { // Color.js can't parse the color value; possibly one of the values we can handle gracefully if (Self.#resolvedColors.has(value)) { - return Self.#resolvedColors.get(value); + let color = Self.#resolvedColors.get(value); + return Self.Color.get(color); } if (!CSS.supports("color", value)) { @@ -165,11 +166,14 @@ const Self = class ColorElement extends NudeElement { // One of the supported color values; resolve and cache it element.style.backgroundColor = value; let color = getComputedStyle(element).backgroundColor; - color = Self.Color.get(color); - Self.#resolvedColors.set(value, color); element.style.backgroundColor = ""; - return color; + let resolvedColor = Self.resolveColor(color, element); + if (resolvedColor) { + Self.#resolvedColors.set(value, color); + } + + return resolvedColor; } } }; From 580bd656ba8134dc13445cb0dbd31921a6d817ab Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 17:09:30 +0200 Subject: [PATCH 6/9] Address feedback: Avoid infinite loop --- src/common/color-element.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/color-element.js b/src/common/color-element.js index 246255d..39b8a0d 100644 --- a/src/common/color-element.js +++ b/src/common/color-element.js @@ -158,8 +158,8 @@ const Self = class ColorElement extends NudeElement { return Self.Color.get(color); } - if (!CSS.supports("color", value)) { - // Not supported or invalid value + if (!CSS.supports("color", value) || !element) { + // Not supported/invalid value, or no element to resolve the color value from return null; } @@ -168,7 +168,7 @@ const Self = class ColorElement extends NudeElement { let color = getComputedStyle(element).backgroundColor; element.style.backgroundColor = ""; - let resolvedColor = Self.resolveColor(color, element); + let resolvedColor = Self.resolveColor(color); if (resolvedColor) { Self.#resolvedColors.set(value, color); } From cc39297edf1ed0b58a9019b141e42d71035f6804 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 18:41:55 +0200 Subject: [PATCH 7/9] Prevent bugs Co-authored-by: Lea Verou --- src/common/color-element.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/color-element.js b/src/common/color-element.js index 39b8a0d..8f7fb5b 100644 --- a/src/common/color-element.js +++ b/src/common/color-element.js @@ -158,7 +158,7 @@ const Self = class ColorElement extends NudeElement { return Self.Color.get(color); } - if (!CSS.supports("color", value) || !element) { + if (!globalThis.CSS?.supports("color", value) || !element) { // Not supported/invalid value, or no element to resolve the color value from return null; } From fe90abf7ca3d96bb558584812cf2a7b815506839 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Tue, 5 Aug 2025 18:49:31 +0200 Subject: [PATCH 8/9] Address feedback: Lessen nesting --- src/common/color-element.js | 38 ++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/common/color-element.js b/src/common/color-element.js index 8f7fb5b..90c6aed 100644 --- a/src/common/color-element.js +++ b/src/common/color-element.js @@ -151,30 +151,30 @@ const Self = class ColorElement extends NudeElement { try { return Self.Color.get(value); } - catch { - // Color.js can't parse the color value; possibly one of the values we can handle gracefully - if (Self.#resolvedColors.has(value)) { - let color = Self.#resolvedColors.get(value); - return Self.Color.get(color); - } + catch {} - if (!globalThis.CSS?.supports("color", value) || !element) { - // Not supported/invalid value, or no element to resolve the color value from - return null; - } + // Color.js can't parse the color value; possibly one of the values we can handle gracefully + if (Self.#resolvedColors.has(value)) { + let color = Self.#resolvedColors.get(value); + return Self.Color.get(color); + } - // One of the supported color values; resolve and cache it - element.style.backgroundColor = value; - let color = getComputedStyle(element).backgroundColor; - element.style.backgroundColor = ""; + if (!globalThis.CSS?.supports("color", value) || !element) { + // Not supported/invalid value, or no element to resolve the color value from + return null; + } - let resolvedColor = Self.resolveColor(color); - if (resolvedColor) { - Self.#resolvedColors.set(value, color); - } + // One of the supported color values; resolve and cache it + element.style.backgroundColor = value; + let color = getComputedStyle(element).backgroundColor; + element.style.backgroundColor = ""; - return resolvedColor; + let resolvedColor = Self.resolveColor(color); + if (resolvedColor) { + Self.#resolvedColors.set(value, color); } + + return resolvedColor; } }; From ac62f551d48b2519ad26e7498fd70be035efddb4 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Wed, 6 Aug 2025 15:47:20 +0200 Subject: [PATCH 9/9] Preserve original background color --- src/common/color-element.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/color-element.js b/src/common/color-element.js index 90c6aed..7b33edc 100644 --- a/src/common/color-element.js +++ b/src/common/color-element.js @@ -165,9 +165,10 @@ const Self = class ColorElement extends NudeElement { } // One of the supported color values; resolve and cache it + let backgroundColor = element.style.backgroundColor; element.style.backgroundColor = value; let color = getComputedStyle(element).backgroundColor; - element.style.backgroundColor = ""; + element.style.backgroundColor = backgroundColor; let resolvedColor = Self.resolveColor(color); if (resolvedColor) {