Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch internal color type to avh/elm-color? #13

Closed
peacememories opened this issue Feb 13, 2019 · 14 comments
Closed

Switch internal color type to avh/elm-color? #13

peacememories opened this issue Feb 13, 2019 · 14 comments

Comments

@peacememories
Copy link

As far as I know, avh/elm-color is supposed to become kind of a community standard when it comes to color. This would make interop of packages much easier. What are your thoughts on this?
It would of course add an alpha channel, but palette could just ignore that, as far as I'm concerned (I personally need support for it, but I don't need it in the functions palette provides)

@peacememories
Copy link
Author

I think one issue with this could be that avh/elm-color currently doesn't have native support for an hsl color space. I like the idea of treating color spaces differently, so maybe I'll ask there if the plan is to add that at some point.

@tesk9
Copy link
Owner

tesk9 commented Feb 13, 2019

Hi! Thanks for bringing this up!

It's definitely something that I'm considering. In the short term, I'll think about adding helpers for going from an avh4/elm-color Color to a tesk9/palette Color and back again. I worry a bit about becoming more dependent than that on avh4/elm-color, since I think there are still lots of discussions happening around how color should be modeling in the library, and what the API will be..

Are you currently using both avh4/elm-color and tesk9/palette? I guess that would be naming-collision prone... 🤔

@peacememories
Copy link
Author

Currently I'm using neither. I'm writing a wrapper around elm-ui for our application that allows runtime skinning, and at the same time I'm creating a small element library. In some of these parts I want the ability to automatically select good-contrast colors for text etc, and in the process of looking these things up I decided to wrap the elm-ui color values completely (since I'm already doing a wrapper).
palette seems to have very nice accessibility functions, but at the same time it doesn't have alpha support (which would at least be necessary for full elm-ui "compatibility") and if there's already an effort to standardize color in Elm, I would not want to go a different route.
There is apparently elm-color-extra which also has some contrast helpers. It doesn't have the nice blending options of palette though (which I'm planning to use for disabled buttons etc.), so I'm torn^^

@tesk9
Copy link
Owner

tesk9 commented Feb 14, 2019

That makes sense! I probably won't have much time to work on this for about a week or so, but I'll come back to it after that.

At minimum, I think the naming collision is an issue that needs to be resolved. Plus some level of support for avh4/elm-color should be on the roadmap. And of course transparency.

Thanks again for bringing this up!

@peacememories
Copy link
Author

Thanks for reacting so quickly!

For now I've decided to use avh4/elm-color because I didn't want to make the decision to drop transparency support from my wrapper. Plus, the maximumContrast function in elm-color-extra is pretty handy.

I think what I'd like to see in the future is just more specialized packages? For example I don't think avh4/elm-color needs to include a color palette. Although of course it can be exhausting to have to install 5 packages just to get the funcitonality you expected from one, so I don't know^^

I really like palettes functions for testing accessibility though. And its explicit support for different color spaces.

@tesk9
Copy link
Owner

tesk9 commented Feb 22, 2019

Thank you for following up! It's really helpful to hear what you ended up doing & why.

@RalfNorthman
Copy link

RalfNorthman commented May 19, 2019

I ran into an unfortunate circumstance relating to this issue.

I really like the functions relating to the color circle from color theory in this package. In a test project I wanted to do color animation on an svg object with help from those functions. My go-to package for svg is elm-community/typed-svg.

But not only does typed-svg have avh4/elm-color as a dependency, many of its functions uses its opaque Color type as arguments. Since the color type from your package is also opaque, in combination with both color packages having a module called Color, conversions between the types become impossible.

So, in short, if I use typed-svg, I can't use palette and vice versa.

@tesk9
Copy link
Owner

tesk9 commented May 20, 2019

I'm working on changing the names to avoid this conflict -- the next major version of palette should be usable.

Thanks for bringing this up!

@tesk9
Copy link
Owner

tesk9 commented May 21, 2019

The renames in #18 should allow you to use this package along with avh4/elm-color (although you'll need to write your own conversion helpers to go from the elm-color implementation to the palette implementation).

I want to take some time to get API reviews and thoughts from folks before I publish, so it may be a while yet before these changes are available.

Please feel free to (gently 🙏) share your thoughts on the PR.

@ianmackenzie
Copy link

What are your current thoughts on this? It looks like the Color -> Colour name change got reverted, which I think makes sense but leaves the question of compatibility/interoperability with avh4/elm-color open.

(I do like the design of tesk9/palette for use in ianmackenzie/elm-3d-scene, but I'm trying to weigh that against the interop benefits of using avh4/elm-color which currently seems more widely used. Happy to chat on Slack if that's easier!)

@tesk9
Copy link
Owner

tesk9 commented Apr 25, 2020

Ian and I talked on slack and over zoom and we pulled some other perspectives in too.. I've drafted a PR #26 to resolve the naming collisions.

Let me know if you have thoughts on the changes!

I'm waffling a bit on "SolidColor" and "TransparentColor." Perhaps "OpaqueColor" and "TranslucentColor"?

@tesk9
Copy link
Owner

tesk9 commented Jul 4, 2020

I decided to stop waffling on getting the best-possible fix here and just went ahead and published what I had.

Please let me know if there are any issues with the new names. If it's a pain to upgrade, let me know. I'll try to put something together to help.

@tesk9 tesk9 closed this as completed Jul 4, 2020
@erlandsona
Copy link

Plus, the maximumContrast function in elm-color-extra is pretty handy.

I really like palettes functions for testing accessibility though. And its explicit support for different color spaces.

Speaking of @ianmackenzie / @tesk9 y'all may find the widen function I wrote interesting :) I'm using in a ui-framework hobby project I'm working on (super super alpha) that's built on theming & accessibility guaranteed to be correct by construction ala "Make Impossible States" and all...
https://ellie-app.com/bFd9x3KcPVQa1

Not sure if I'd call it a fold on Colors but widen is how I think of the behaviour 🤷‍♂️

@tesk9
Copy link
Owner

tesk9 commented Dec 7, 2020

That's very neat! Thanks for the link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants