color-js / elements

WIP
12 stars 1 forks source link

First stab on adding support for deltas and contrast from other color #96

Open DmitrySharabin opened 3 months ago

DmitrySharabin commented 3 months ago

I chose vs as the attribute name for the other color. We can easily change it if needed.

For now, it looks like this:

image
netlify[bot] commented 3 months ago

Deploy Preview for color-elements ready!

Name Link
Latest commit 09f413684d43fc5f94b0975f7c120d1457069ceb
Latest deploy log https://app.netlify.com/sites/color-elements/deploys/6670759649e2a00008f29b72
Deploy Preview https://deploy-preview-96--color-elements.netlify.app
Preview on mobile
Toggle QR Code...

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 commented 3 months ago

Could you add some docs so I can review the API too?

DmitrySharabin commented 3 months ago

Could you add some docs so I can review the API too?

Sure

DmitrySharabin commented 3 months ago

Could you add some docs so I can review the API too?

Sure

Done!

DmitrySharabin commented 3 months ago

Thank you for your feedback. I addressed all the issues you mentioned. Here is the summary of the changes:

LeaVerou commented 2 months ago

Some ideas:

Option 1: Two separate data structures

colorInfo: {"oklch.l": 0.5, deltaE2000: undefined, contrastAPCA: undefined},
vsInfo: {"oklch.l": 0.2, deltaE2000: 10, contrastAPCA: 25},

Option 2: One data structure, one entry per key

Option 2.1: Object when 2 values, value otherwise

colorInfo: {
    "oklch.l": {value: 0.5, delta: 0.2}, 
    deltaE2000: 10, 
    contrastAPCA: 25
},

Option 2.2: Object always

colorInfo: {
    "oklch.l": {value: 0.5, delta: 0.2}, 
    deltaE2000: {value: 10, delta: 10}, // or {value: undefined, delta: 10}?
    contrastAPCA: {value: 25, delta: 25}, // or {value: undefined, delta: 25}?
},

What do you think are the pros and cons of each approach? Do you see any other options?

DmitrySharabin commented 2 months ago

Option 1: Two separate data structures

colorInfo: {"oklch.l": 0.5, deltaE2000: undefined, contrastAPCA: undefined},
vsInfo: {"oklch.l": 0.2, deltaE2000: 10, contrastAPCA: 25},

Pros

Cons

Option 2.1: Object when 2 values, value otherwise

colorInfo: {
  "oklch.l": {value: 0.5, delta: 0.2}, 
  deltaE2000: 10, 
  contrastAPCA: 25
},

Pros

Cons

Option 2.2: Object always

colorInfo: {
  "oklch.l": {value: 0.5, delta: 0.2}, 
  deltaE2000: {value: 10, delta: 10}, // or {value: undefined, delta: 10}?
  contrastAPCA: {value: 25, delta: 25}, // or {value: undefined, delta: 25}?
},

Pros

Cons

I would also add the isAngle (angle?) property to display deltas correctly (as a percentage or a number without any units). It can be calculated once and read when needed.

colorInfo: {
    "oklch.l": {value: 0.5, delta: 0.2, isAngle: false}, 
},

Another option would be to provide the unit itself.

colorInfo: {
    "oklch.l": {value: 0.5, delta: 0.2, unit: "%"}, 
},

However, in that case, we should probably either adjust delta depending on the unit or calculate it with this unit in mind. Or, alternatively, we could have rawDelta (calculated) and delta (adjusted):

colorInfo: {
    "oklch.l": {value: 0.5, rawDelta: 0.2, delta: 20, unit: "%"}, 
},

If we go with option 2.2, should we also add these properties (rawDelta, unit; which should be undefined to my mind) to deltaE2000 and contrastAPCA? Luckily, if an object doesn't have a property, it's considered undefined.

LeaVerou commented 2 months ago

This is mostly spot on. Some comments on the analysis:

Option 1: Two separate data structures

colorInfo: {"oklch.l": 0.5, deltaE2000: undefined, contrastAPCA: undefined},
vsInfo: {"oklch.l": 0.2, deltaE2000: 10, contrastAPCA: 25},

Pros

  • All the needed info about the color (the current one and vs) in one place
  • Both colorInfo and vsColor have the same (predictable) structure
  • All colorInfo and vsColor properties are scalars; no need for extra checks

Cons

  • Both colorInfo and vsInfo should stay in sync: when one changes, the other should be recalculated (it may lead to a circular dependency)

How would it lead to a circular dependency? colorInfo depends on color and info vsInfo depends on colorInfo (or color), vs, and info. I don’t see a cycle?

image

If anything, another pro of this approach is that it separates the dependencies: if only the vs color changes, we don’t need to also update colorInfo.

  • When we need deltas, we have to recalculate them based on values in two properties instead of simply reading them from one (from colorInfo)
  • When we need info concerning the current color (e.g., deltaE and contrast), we should get it from another property—vsInfo

What do you mean? deltaE and contrast don't make sense for a single color, they are inherently operators that take two colors.

Option 2.1: Object when 2 values, value otherwise

colorInfo: {
    "oklch.l": {value: 0.5, delta: 0.2}, 
    deltaE2000: 10, 
    contrastAPCA: 25
},

Pros

  • All the needed info is calculated once, stored in one place, and can be cached
  • No circular dependencies: colorInfo is updated on color and vs change

Cons

  • On every property read, we should check whether its value is an object or a scalar. OTOH, we can easily see which properties are coords (objects) and which are not (scalars)

I think that's a pretty big downside fwiw. It makes using it quite annoying:

element.colorInfo["oklch.l"]?.value ?? element.colorInfo["oklch.l"]

Yikes.

Option 2.2: Object always

colorInfo: {
    "oklch.l": {value: 0.5, delta: 0.2}, 
    deltaE2000: {value: 10, delta: 10}, // or {value: undefined, delta: 10}?
    contrastAPCA: {value: 25, delta: 25}, // or {value: undefined, delta: 25}?
},

Pros

  • All the needed info is calculated once, stored in one place, and can be cached
  • No circular dependencies: colorInfo is updated on color and vs change
  • On reading a property value, there is no need to check whether it's an object or a scalar: all properties have the same structure—{value, delta}

Cons

  • In deltaE2000: {value: 10, delta: 10} and contrastAPCA: {value: 25, delta: 25}, delta doesn't make sense (?); we need value only.

Think of it from the perspective of code that handles this info generically, without knowing which ones apply to one color and which ones to two. If we only have value there’s no way to tell (without duplicating the logic about which one is what in calling code). But that could be addressed with a flag.

OTOH, we can use this to distinguish coords from other data

Yup!

I would also add the isAngle (angle?) property to display deltas correctly (as a percentage or a number without any units). It can be calculated once and read when needed.

I’ll need to think about this more.

colorInfo: {
  "oklch.l": {value: 0.5, delta: 0.2, isAngle: false}, 
},

Another option would be to provide the unit itself.

colorInfo: {
  "oklch.l": {value: 0.5, delta: 0.2, unit: "%"}, 
},

However, in that case, we should probably either adjust delta depending on the unit or calculate it with this unit in mind. Or, alternatively, we could have rawDelta (calculated) and delta (adjusted):

colorInfo: {
  "oklch.l": {value: 0.5, rawDelta: 0.2, delta: 20, unit: "%"}, 
},

If we go with option 2.2, should we also add these properties (rawDelta, unit; which should be undefined to my mind) to deltaE2000 and contrastAPCA? Luckily, if an object doesn't have a property, it's considered undefined.

Not sure about this either, since we have no way of selecting a unit currently.


Another con of 2.2 is that long chains of properties are annoying and error prone. Having to do element.colorInfo['oklch.l'].value to get the value is tedious AF.


So weighing these tradeoffs, which option would you go with?

Personally I’m leaning toward Option 1, but need to hear more about the circular dependency in case I’m missing something. We could even have a vsColorInfo property to cache the other color’s info to partition the dependencies even more.

DmitrySharabin commented 2 months ago

How would it lead to a circular dependency? colorInfo depends on color and info vsInfo depends on colorInfo (or color), vs, and info. I don’t see a cycle?

You are right; there is no cycle. My brain simply refuses to work properly today. 🤦‍♂️ Thank you for checking it.

What do you mean? deltaE and contrast don't make sense for a single color, they are inherently operators that take two colors.

I looked at your example code and noticed that the deltaE and contrast info is stored in vsInfo: {"oklch.l": 0.2, deltaE2000: 10, contrastAPCA: 25}, in colorInfo those properties are undefined. But I thought that we calculate deltaE and contrast for the current color (using the vs color as the second one), so, all the relevant info should be stored in colorInfo. While I was writing this, it dawned on me that it makes more sense to store this data in vsInfo, as you suggested: current value for the chosen color coord, distance from the current color, and contrast with the current color—all these pieces of data are relevant to the vs color.

I think that's a pretty big downside fwiw. It makes using it quite annoying:

element.colorInfo["oklch.l"]?.value ?? element.colorInfo["oklch.l"]

Yikes.

Agreed!

So weighing these tradeoffs, which option would you go with?

After re-reading this one more time, I'm also leaning toward Option 1. It seems like it will also make the code more readable than we might get with other options.