[color-slider] Incorrect color when programmatically setting stops

[DmitrySharabin]

I could narrow down the problem.

When we initially set the value of <channel-slider>, we update its dependants. Doing so, we update the corresponding <color-slider>. Precisely, the color property.

To update the color property, we call Prop.update(). Since the color prop has a getter, it’s called to get the current color value and set it to <color-picker>.

https://github.com/nudeui/element/blob/a343ed0dd06212593989e6049ff68d3876bb2751/src/props/Prop.js#L248-L249

However, we are setting not the initial value (lightness == 0.6279…) that caused the update, but the one got from <color-slider> (lightness == 0.7081…).

Color change dispatches a propchange event (for color property) and sets the --color custom property (the wrong color!). After that, the color property is never changed, which means the --color custom property is never updated.

Since the slider thumb uses --color as a background, we get the wrong result.

[DmitrySharabin]

A bit of further observations (WIP)

Why <channel-slider> track styles are applied correctly?

Because on every <channel-slider>'s defaultColor change we update the corresponding prop on the underlying <color-slider>: https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/channel-slider/channel-slider.js#L92

However, we should also update the color property of the <color-slider>. Why? Right now, color is a read-only prop of <color-slider>: https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/color-slider/color-slider.js#L225-L230

Its value is calculated from scales:

https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/color-slider/color-slider.js#L147-L168

And scales in their turn are calculated from stops:

https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/color-slider/color-slider.js#L231-L246

That means color is calculated (through a series of steps) from steps.

This leads us to the conclusion that every time we update the <color-sliders>'s stops from <channel-slider>, we “update” the <color-sliders>'s color, too (well, not update as it's read-only, but the getter will return an updated value).

But when we update stops? When we read the <channel-slider>'s color attribute (on <channel-slider> initialization).

Why does the <channel-slider> thumb have the wrong color?

After the <channel-slider>'s color attribute is handled, both <channel-slider> and <color-slider> have correct color stops in oklch with channels equal 0.5, 0.2, and 180 respectively—the default color value. For <color-slider>, that means that its color prop is updated, and eventually, the --color color property is set:

https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/color-slider/color-slider.js#L94-L101

And it's set based on the current color (in oklch with channels [0.5, 0.2, 180]).

Later on, when the default value for the lightness channel of the <channel-slider>'s current color is calculated and updated, the value of the underlying <color-slider> is also updated (by applying the changes):

https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/channel-slider/channel-slider.js#L86-L89

Even though the change of the value of the <channel-slider> will lead us to the correct color value for the <channel-slider> (the value is calculated from defaultColor we have something like: in oklch with channels [new_value1, new_value2, new_value3]):

https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/channel-slider/channel-slider.js#L43-L44

This value change doesn't lead us to the correct update of the color of the underlying <color-slider>—the only thing that is updated is its lightness channel. So, we have something like: in oklch with channels [new_value, 0.2, 180]. And this color is wrong.

Possible Solution

We should probably update the <color-slider>'s color prop (not only stops) of every change of corresponding properties of <channel-slider>, e.g., like so:

        if (name === "defaultColor" || name === "space" || name === "channel" || name === "min" || name === "max") {
            this._el.slider.stops = this.stops;
+           this._el.slider.color = this.defaultColor;

            if (name === "space" || name === "channel" || name === "min" || name === "max") {
                this._el.slot.innerHTML = `${ this.channelName } <em>(${ this.min }&thinsp;&ndash;&thinsp;${ this.max })</em>`;
            }
        }

But for now, color is a read-only prop of <color-slider>. We should probably fix it.

Why is it starting to work correctly on user interaction?

On user interaction, the <channel-slider> value changes, this leads to the change of the value of the (underlying)<color-slider>. This leads to color prop change, and its value is based on stops, which were set correctly.

[DmitrySharabin]

The issue is reproduced with <color-slider> when programmatically sets its stops.

<button id="btn">Set stops</button>
<color-slider space="oklch" stops="red, red" id="slider"></color-slider>

<script>
    btn.addEventListener("click", () => {
        slider.stops = "gold, darkcyan, indigo";
    });
</script>

Debugging further...

[DmitrySharabin]

Found it!!!

The color prop is a computed property and is calculated with colorAt():

https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/color-slider/color-slider.js#L227-L229

colorAt(), in its turn, is calculated with colorAtProgress() https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/color-slider/color-slider.js#L142-L145 which depends on scales: https://github.com/color-js/elements/blob/847d74c70e5a3dd317c3966803a8af0818df349e/src/color-slider/color-slider.js#L147-L168

It means that color depends on scales but doesn't know about it! So, we need to explicitly set dependencies when defining the color prop:

color: {
    type: Color,
    get () {
        return this.colorAt(this.value);
    },
+   dependencies: ["scales", "value"],
},

It seems that we should solve this issue in a more general way than the one proposed above. We should probably also check property getters when building the dependency graph to find what they depend on. Is this possible?

[LeaVerou]

Yay! No, I think setting explicit dependencies is fine, that's exactly what they're for. Trying to infer them in that case would get incredibly complex, and the cost-benefit is not worth it (though I'm open to being convinced otherwise)