color-js / elements

WIP
12 stars 1 forks source link

`<color-gamut>` refactor #61

Closed LeaVerou closed 3 months ago

netlify[bot] commented 3 months ago

Deploy Preview for color-elements ready!

Name Link
Latest commit 43745e2d1ffbcafd2c017dfbd5d71e0a3fea5d22
Latest deploy log https://app.netlify.com/sites/color-elements/deploys/66506810c01e480008beb2de
Deploy Preview https://deploy-preview-61--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.

DmitrySharabin commented 3 months ago

I couldn't suggest a fix for the demo here (since it's not in the PR), so I pushed it directly. Now, the demo is fixed.

image
DmitrySharabin commented 3 months ago

I would also suggest this change (mind min and step) to the demo (L52) to close #15:

<p><label>Chroma increments: <input type=number id=c_step value="0.2" min="0.1" step="0.1">%</label>
LeaVerou commented 3 months ago

I would also suggest this change (mind min and step) to the demo (L52) to close #15:

<p><label>Chroma increments: <input type=number id=c_step value="0.2" min="0.1" step="0.1">%</label>

What change is this? From what to what?

DmitrySharabin commented 3 months ago

What change is this? From what to what?

The demo hangs the page now (see #15). This happens because of an infinite loop we might get when Chroma increments becomes equal to 0. This means that step (L58) becomes equal to 0. And the following loop (L66-68) becomes infinite:

for (let c = c_range.min; c<= c_range.max; c+=step) {
    colors.push(`oklch(${l.value}% ${c.toLocaleString("en")}% ${h.value})`);
}

The fix is rather simple (L52):

- <input type=number id=c_step value="0.2" min="0">
+ <input type=number id=c_step value="0.2" min="0.1" step="0.1">

We might want to adjust step, but min should never be equal to 0.

LeaVerou commented 3 months ago

Sure. Though 0.1 is kinda small. Also, even with a min we'd need to handle the case it may be 0 in the code, since users can still type 0 directly.