[color-swatch] Update the corresponding custom CSS properties on the `gamuts` change, fixes #79

[DmitrySharabin]

In the dependency graph, the order of props also matters—gamuts should be updated before color.

[netlify[bot]]

✅ Deploy Preview for color-elements ready!

Name	Link
🔨 Latest commit	4da136343b486f0a11eb5bb6128a6cc28a3f085d
🔍 Latest deploy log	https://app.netlify.com/sites/color-elements/deploys/666074bfa508a4000837ff15
😎 Deploy Preview	https://deploy-preview-80--color-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[LeaVerou]

Why?

[DmitrySharabin]

Some more results of my investigation: The propChange event for gamut is fired only once when color is set, but not when the gamuts changes. This is incorrect: If gamuts change, the properties of the current gamut might also be changed, and these changes should be reflected.

Now, it should be fixed for real. Thank you for your question, which allowed me to investigate the problem deeper. 🙏

[LeaVerou]

Hmm, I’m still not sure this is the right way and not a workaround. If the gamuts change in a way that affects the current gamut, a propchange should fire for gamut too…

[DmitrySharabin]

If the gamuts change in a way that affects the current gamut, a propchange should fire for gamut too…

That was one of my guesses during the investigation, too. But this event didn't fire up, and I thought it shouldn't. Obviously (now, at least), it should. So, yeah, you are right—this is more of a workaround.

[LeaVerou]

Can we investigate why it doesn't fire? It sounds like it could be a Nude Element bug.

[DmitrySharabin]

Can we investigate why it doesn't fire? It sounds like it could be a Nude Element bug.

Sure. Will do it now

[DmitrySharabin]

I knew it! I had a feeling that the real reason is in this.

So, the sequence of actions is the following:

• <gamut-badge> added to the DOM
• color prop is set (via the attribute)
• color dependents (gamutInfo) should be updated
• gamutInfo depends on gamuts, so gamuts should be set first
• gamuts takes its default value (since the corresponding attribute is not yet handled, we are still working with the first attribute—color)
• gamutInfo is set, so its dependents (gamut) should be updated
• gamut value is set (rec2020—the id of the corresponding gamut), and the corresponding event (gamutchange) is dispatched—this step is the first key step for the rest of the steps
• time for the gamuts attribute: we update the gamuts prop with a new value taken from the attribute
• gamuts prop is updated, and its dependents (gamutInfo) should be updated
• gamutInfo is updated (it gets the same id as previously—rec2020, but another label—P3+)
• gamutInfo dependents (gamut) should be updated
• Here comes the second key step: gamut is a computed property that returns gamutInfo.id, but it didn't change. So, when we try to set gamuts “updated“ value, we come to these lines: https://github.com/nudeui/element/blob/14a0d68d4adcd242f742c743740f5fbbe97f0b4d/src/props/Prop.js#L193-L195, where parsedValue and oldInternalValue are equal, and we return from the setter. The gamutchange event (correctly) never fires, and <gamut-badge> knows nothing about updated (with a new label) gamut.

As I can conclude, nothing is wrong with how NudeElement works (it works great).

What if we rename gamutInfo to gamut (and get rid of the old one) and work with it all over the <gamut-badge> code?

[LeaVerou]

Ah, so gamut depends on this.gamutInfo.id, but because the object this.gamutInfo didn't change, NudeElement doesn’t realize its property may have changed, right?

Just making sure I got it right before suggesting solutions.

[LeaVerou]

Actually, if we define gamuts as not just an array, but an array of Object values (with suitable defaultKey/defaultValue) this should be fixed automatically, since objects are compared by calling equals() recursively. Whereas now there is no type for array values, so it falls back to the default equals.

This is also a Nude Element issue, since there's no way to do that (typeOptions only goes one level down, so we can do itemType: Object, but then we can't set any typeOptions for said type. So it will require a reworking of that part of the API.

[DmitrySharabin]

Ah, so gamut depends on this.gamutInfo.id, but because the object this.gamutInfo didn't change, NudeElement doesn’t realize its property may have changed, right?

Just making sure I got it right before suggesting solutions.

this.gamutInfo did change (at least, its label property), and NudeElement noticed it. This change launched the update of gamut. But since this.gamutInfo.id didn't change, technically, gamut's new and old values are the same. So, NudeElement stopped propagating changes.

[DmitrySharabin]

This is also a Nude Element issue, since there's no way to do that (typeOptions only goes one level down, so we can do itemType: Object, but then we can't set any typeOptions for said type. So it will require a reworking of that part of the API.

I was going to request this feature on NudeElement later today. Yesterday, I was trying to describe gamuts as an array of objects, but we don't support it yet. If we have it one day, this will be awesome!

[LeaVerou]

This is also a Nude Element issue, since there's no way to do that (typeOptions only goes one level down, so we can do itemType: Object, but then we can't set any typeOptions for said type. So it will require a reworking of that part of the API.

I was going to request this feature on NudeElement later today. Yesterday, I was trying to describe gamuts as an array of objects, but we don't support it yet. If we have it one day, this will be awesome!

We do support it, we just don't support customizing that Object type in anyway, so it's almost useless.

[DmitrySharabin]

Since all code inside the block depends only on gamutInfo, I would change the condition to name === "gamutInfo". I like this fix better—it sounds more logical to me.