oddbird / oddcontrast

https://www.oddcontrast.com/
BSD 3-Clause "New" or "Revised" License
28 stars 1 forks source link

Add background color gradients to sliders #88

Closed jamesnw closed 1 year ago

jamesnw commented 1 year ago

Description

Uses colorjs to generate gradient steps, and add them as a CSS variable to the element.

Steps to test/reproduce

Open up the preview, and verify the sliders work as expected, showing the color on the slider that it would be when the slider is at that point. Based off of a ColorJS slider, I chose 10 steps, which is likely more than needed in some circumstances, but Hue sliders especially can have a wide range of visual difference. There is a way of setting the maximum DeltaE (visual difference) between steps, so if we are running into issues where 10 steps is too much (or not enough in some cases), we can experiment with that.

Show me

image
netlify[bot] commented 1 year ago

Deploy Preview for oddcontrast ready!

Name Link
Latest commit 055f8fe8c4cbd69765c4e5cffb85218d0ccf6486
Latest deploy log https://app.netlify.com/sites/oddcontrast/deploys/64cbfed1ffc1b200084338fa
Deploy Preview https://deploy-preview-88--oddcontrast.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.

jamesnw commented 1 year ago

JS is ready to review, styles have not been applied, so I'm handing over to @stacyk.

jamesnw commented 1 year ago

@jgerigmeyer It appears that all 8 slider gradients are getting recalculated whenever any of them changes. It doesn't seem to be a performance issue, but is there a way to have Svelte only recalculate the FG or BG sliders when its values change?

jgerigmeyer commented 1 year ago

@jgerigmeyer It appears that all 8 slider gradients are getting recalculated whenever any of them changes. It doesn't seem to be a performance issue, but is there a way to have Svelte only recalculate the FG or BG sliders when its values change?

@jamesnw I looked into this a bit. It's not calculating all 8 sliders on every change -- it's calculating the 4 relevant FG or BG sliders twice each, once on mousedown and again on mouseup. From what I can tell, this isn't really a change new to this PR. If I add in the following code on main:

  $: if ($color) {
    console.log('color changed');
  }

  afterUpdate(() => {
    console.log('updated');
  });

...I see both events fired twice, once on mousedown and again on mouseup. So I think this must relate to how Svelte handles clicks/keyboard events on input[type="range"] when using bind:value? The same repeat does not happen if you make a single edit to the color text input, triggering the change to color from outside the slider component.

So the part that is new to this PR is that now the sliders calculation is dependent on $color, which means that's being run twice also. But I'm not sure that's really a practical issue?

jgerigmeyer commented 1 year ago

I want the sliders to have background color values.

jgerigmeyer commented 1 year ago

But I'm not sure that's really a practical issue?

@jamesnw Not sure if it's worth it, but this commit seems to address the issue.

stacyk commented 1 year ago

Would anyone mind if we changed our default colors to something maybe still in gamut, but would show those awesome gradient sliders a little more? Something like:

Screenshot 2023-07-28 at 11 35 32 AM

stacyk commented 1 year ago

Wanted to add in case we want to reference this later, I played with both a dark marker (like the current state and the mockups) and a smaller transparent marker with a dark and light border/shadow which are visible on all colors. I also added a light border to the dark one. There's something about the simplicity of the dark marker that I like but it can get a little lost on dark colors even though it is larger than the input range bar.

The inputs on this pen don't change the bg color, so here is a screenshot of the options I tried: https://codepen.io/stacy/pen/jOQQyaO/e5a8cf42d46f56cd634de2c35fcd4df2?editors=1100 oddcontrast-range-thumbs

stacyk commented 1 year ago

@jamesnw Im not sure if this is an issue you already knew, but in both Chrome and Firefox when playing with the sliders and then changing the color format drop down, I get unexpected results when switching to HSL sometimes. I can't pinpoint what I do differently, but sometimes the Hue doesn't change from the original color, and sometimes the Lightness gradient is very narrow. Here's a screen recording of playing around with this a bit.

https://github.com/oddbird/oddcontrast/assets/1581694/e56b909b-0285-4eff-9315-577377809fcf

jamesnw commented 1 year ago

@stacyk Good catch! It looks like this is an issue when switching to a space where the current color is out of gamut. This is an issue before this change, but becomes quite noticeable on this branch. You can see it by-

  1. Go to oklch space, and enter oklch(0.918 0.3879 315.77)
  2. Switch to p3 space, and note that the red channel is greater than 1.
  3. Move the red slider down, and see a large jump, and then take it back to 1, and you can't get back to the original number.

The switch to HSL is especially visible because it can't represent values that are outside of gamut (but the other spaces we show here can).

I think we may need to constrain it to in gamut colors when converting, but there's some potential UX questions around loss of info, but also values greater than the slider max.

There may also be an impact on the gradients where the slider value is greater than the slider max (or rather, when a channel value is out of gamut, which causes that to be the case.) I think this is all related to this card- https://trello.com/c/6ne6Wz2w/42-i-want-a-better-way-to-handle-color-out-of-gamut-situations.

SondraE commented 1 year ago

@stacyk

Wanted to add in case we want to reference this later, I played with both a dark marker (like the current state and the mockups) and a smaller transparent marker with a dark and light border/shadow which are visible on all colors.

I consistently like the "dark range thumb with light border." To me it appears to be the consistently visible no matter the colors in the background.

I also really like your suggestion of new default colors that show off the gradient sliders. Are there colors that are closer to our brand that would work?

jgerigmeyer commented 1 year ago

I think we may need to constrain it to in gamut colors when converting, but there's some potential UX questions around loss of info, but also values greater than the slider max.

There may also be an impact on the gradients where the slider value is greater than the slider max (or rather, when a channel value is out of gamut, which causes that to be the case.) I think this is all related to this card- https://trello.com/c/6ne6Wz2w/42-i-want-a-better-way-to-handle-color-out-of-gamut-situations.

Yep, this ☝️. We could pretty simply map the color to the nearest in-gamut for the selected space, which (I think) would solve the problem. As James mentioned, loss of info is the trade-off -- you could no longer switch from oklch to hsl and back to oklch (without touching the sliders) and be guaranteed to have the same original color. @SondraE @mirisuzanne @stacyk Thoughts on this? Maybe doesn't need to hold up this PR, unless we think because it's more noticeable now with the gradients that it's worth addressing first?

mirisuzanne commented 1 year ago

I think we should at least make it clear what's happening, when this happens? Long term, we could even consider asking if the value should be clipped or mapped, but that's a separate feature.

jgerigmeyer commented 1 year ago

@mirisuzanne @stacyk I adjusted in https://github.com/oddbird/oddcontrast/commit/a7c9213c9b1b6c7344bfe46560641d429bf98c64 to apply gamut mapping by default when pasting a color or switching between color formats. I think this is a reasonable start that can reduce confusion, and we can provide more info or options as part of https://trello.com/c/6ne6Wz2w. What do you think?

stacyk commented 1 year ago

I am going to switch to the range thumb with white border and update the default colors. Then this PR can be merged and we can work on the gamut warning in that other Trello card. Thanks @jgerigmeyer @jamesnw @SondraE @mirisuzanne for your help and feedback (and patience with my notification issue).