Closed avh4 closed 6 years ago
Color
type, but with additional record based types that should be used for public API. This way, the function name can be shorter while still being explicit because of the type.Like:
type Color = ... -- opaque
type alias StandardRgb = { red : Float, green : Float, blue : Float, alpha : Float } -- public
type alias StandardRgb255 = { red : Int, green : Int, blue : Int, alpha : Float } -- public
fromRgb: StandardRgb -> Color
fromRgb255: StandardRgb255 -> Color
toRgb : Color -> StandardRgb
toRgb255 : Color -> StandardRgb255
I am strongly in favor of passing records and avoiding fromRgb: Float -> Float -> Float -> Color
we don't need partial application. If there is a strong demand for that form, I'm for adding shortcuts like: rgb: Float -> Float -> Float -> Color
but the fromXXX
and toXXX
should manipulate records.
Rgb8 is shorter, but Rgb255 is clearer and explicit and should be used.
We need CSS color parser, cf my comment https://github.com/avh4/elm-color/issues/3#issuecomment-418623860 I had to deal with some export from some lib which used rgba(x%...)
format I had to parse in elm, I'm sure it would help a lot of folks.
I think all functions should include the alpha channel and be called rgb, hsl
except the toHex : Color -> { hex : String, alpha : Float }
form which is a nice idea. Possible "short forms" might also default the alpha to 1 rgb: Float -> Float -> Float -> Color
but records should include the alpha.
grayscale: Float -> Color
is nice, but I never remember if 0 is white or black and what is the range, I end up doing c = setLightness black 0.3
if I need a 30% gray.
Speaking of the 30% gray, I really think elm would be well served by a perceptually linear color space (some guy implemented hslvu in elm :D ), if we have a gradient or whatever colored element, if we change the hue, we want to preserve saturation and contrast/lightness.
Color manipulation and accessibility also need an adapted color space.
For the last two points, I will keep my hsluv package up to date and add support for the Color
type as a first step. Then we can discuss if it should be included in the core color package.
To comment on some specific points (hopefully not too nitpicky):
goals: agreed
api:
setAlpha
? This makes it clear that you can also change the opacity for a color that already has an alpha, as opposed to 'adding' an alpha channel. Again, weakly held opinion.( fromRgba, fromRgb, fromHsla, fromHsl, fromRgba255, fromRgb255, setAlpha )
could be a very decent api in terms of clearness and ease of use.( toRgba, toRgba255, toHsla )
sounds like a good idea to me.toHex
: I'm luke warm on having to deconstruct for result, especially for one simple reason: I'm not ever going to use that alpha it returns. So yes, it makes the lossiness clear, but it's kind of not a nice user experience. I think we can trust the programmers to check the doc and understand that they convert to hex with full opacity?fromHex
: I don't personally foresee ever needing it. But I'm happy to defer to other people's needs.greyscale: no need to include this, except as an example in the docs showing how easy it is to create this function with HSL in one line: greyscale x = fromHSL 0 0 x
.
basic colors: that's actually a great idea, I forgot about that feature. We could re-use existing work: https://clrs.cc/ (MIT-licensed).
@kuon since we have different opinions on the input type, I just wanted to elaborate a tiny bit: since I foresee people to create tons of ad-hoc colors, I felt that we don't gain much from writing out
fromRgba {red=0.4, green=0.1, blue=0.9, alpha=0.3}
every time. But if we have a short version that let's you write
rgba 0.4 0.1 0.9 0.3
then I guess we have the best of both worlds? I don't know how other people feel about exposing loads of near-synonymous functions, but I personally wouldn't mind.
I don't think we should take a record as input, as everybody knows the ordering of the colors of RGB, since it's in the name. Same for HSL.
It's not about order, it's to allow easier "moving around" of colors (for example, decoded from json).
I think we could have both. The fromXXX
form taking records (aliased for expliciteness of the "standard") and the xxx
form taking multiple arguments.
Whatever decisions are made about the actual semantics and implementation, I think the key is to make the base Color
package as simple and maintainable as possible. Allow other community packages to support other color spaces/formats and expose additional functionality.
Great, I agree with the comments about using records / having both fromXxxx [record]
and xxxx [multiple params]
, as well as with just calling functions "rgb" and noting in the docs the subtlety about sRGB. (After doing research, I had originally thought color profiles would be more prominent, but I haven't yet seen any real uses of them in web dev.)
I've updated the proposal based on those comments.
Allow other community packages to support other color spaces/formats and expose additional functionality.
@danielnarey What I'd like this package to help avoid is having many packages pop up that have certain functions that are basically the same -- for example, converting to/from hex strings is something that several 0.18 packages implemented (and all in slightly different ways). (Another example is color manipulation--certainly a standard package doesn't need to include all possibly manipulations, but there's probably a subset that many end-users will need that probably should become standard.) I agree we want to make something that will support other packages to implement more nuanced or advanced functionality, but also I think we should try to discover the natural break between commonly-needed and needed-in-specific-scenarios, and try to reduce the number of packages that most users need to depend on, if possible.
I'd be interested in starting to collect a list of the types of functionality that we'd expect to be the responsibility of other community packages. Do you have more to add to this list?
On September 5, 2018 5:30:07 PM GMT+02:00, Aaron VonderHaar notifications@github.com wrote:
Great, I agree with the comments about using records / having both
fromXxxx [record]
andxxxx [multiple params]
, as well as with just calling functions "rgb" and noting in the docs the subtlety about sRGB. (After doing research, I had originally thought color profiles would be more prominent, but I haven't yet seen any real uses of them in web dev.)I've updated the proposal based on those comments.
-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/avh4/elm-color/issues/4#issuecomment-418774257
I think we are getting there. I would still declare alias type for records.
I also agree about avoiding multiple implementations of the same thing, we should provide a nice reference tool for color manipulations, with tests and all. We don't want package A to provide a darken function using float in the 0-1 range and B, using a percentage in the 0-100 range...
Elm now strip unused code, we won't bloat apps if we provide a full featured package. -- Nicolas Goy
@kuon can you say a bit more about the record type aliases?
type alias StandardRgb = { red : Float, green : Float, blue : Float, alpha : Float } -- public
fromRgb: StandardRgb -> Color
...
For me personally, I find the extra indirection confusing what APIs excessively use type aliases (it adds another name that I have to learn and remember)... Do you tend to prefer more aliases in general, or is there something specific to colors that makes these particularly useful? I'm guessing it's for cases where you want to store these records in another data structure? I was thinking if there's a way we could encourage most users to story Color values most of the time and only convert to records at the boundaries of their application, then maybe the record aliases wouldn't be needed?
@avh4 As a general rule, when designing an API, I avoid type alias as I use records mostly for options, a bit like https://package.elm-lang.org/packages/mdgriffith/elm-ui/latest/Element-Input
Now for the color package, I thought of alias for a few reasons:
That extra level of indirection is something I want, because I want the API use to wonder "what is StandardRgb
" and read the type documentation where we mention it's 0->1 range in sRGB space. I think it is clearer to expose the API user to the alias once, read about normalized values and sRGB and then have a consistent experience across the API using that type.
I think it is good to expose a public type other library or user code can use. Using that type in another library implies the sRGB and normalization of value and makes everything consistent. The user will be familiar with how the components are represented.
Knowing that those alias will be "public elm knowledge", I think it will help make code clearer when components needs to be stored. We may imagine an image manipulation program where the developer wants direct pixel data, with Array StandardRgb
, the person reading the code will be in familiar and documented water.
As a note, we can drop the Standard and use RgbComponents
and document it to be in sRGB.
Adding the points that @avh4 made, I guess what I don't like about the type alias so much is that
Color
value. So it will probably never even show up in my own type signatures.I get the point about Array StandardRgb
, but you will have people using intermediate formats anyway, so that for example people will inevitably use stuff like Array (Float, Float, Float)
, Matrix Float
, and whatever other permutations, with and without alpha, List
, Dict
instead of Array
, Int
instead of Float
etc.
I would advocate that our goal should be to have the same pleasant clarity and simplicity that you can find in the rest of the elm ecosystem. Ideally people have this impression that using color in a typesafe way is no big deal and feels very natural.
Maybe I can write up an example of what I think the docs snippet for the Color type should look like?
@2mol well, maybe I am being optimistic about the reuse of the component type. I guess you might be right about the "everybody is going to use a custom type anyway, because of requirements" part.
My main point was about documentation, because they are not just record containing floats, they are records containing floats clamped in the 0->1 range, and the float type does not enforce that. The type alias is a way to document that, but well, it will not add any type safety.
I like them in this case because I have the feeling it will tidy the API and the documentation, but I don't have a strong opinion about it. Especially if we do all functions in fromRgb
and fromRgba
variant, as this will double the number of type alias and might get out of hands.
To try to summarize:
fromRgb : { red : Float, green : Float, blue : Float } -> Color
fromRgba : { red : Float, green : Float, blue : Float, alpha : Float } -> Color
fromRgb255 : { red : Int, green : Int, blue : Int} -> Color
fromRgba255 : { red : Int, green : Int, blue : Int, alpha : Float } -> Color
fromHsl : { hue : Float, saturation : Float, lightness : Float } -> Color
fromHsla : { hue : Float, saturation : Float, lightness : Float, alpha : Float } -> Color
rgb255 : Int -> Int -> Int -> Color
rgba255 : Int -> Int -> Int -> Float -> Color
rgb : Float -> Float -> Float -> Color
rgba : Float -> Float -> Float -> Float -> Color
hsl : Float -> Float -> Float -> Color
hsla : Float -> Float -> Float -> Float -> Color
toRgb : Color -> { red : Float, green : Float, blue : Float }
toRgba : Color -> { red : Float, green : Float, blue : Float, alpha : Float }
toRgb255 : Color -> { red : Int, green : Int, blue : Int }
toRgba255 : Color -> { red : Int, green : Int, blue : Int, alpha : Float }
toHsl : Color -> { hue : Float, saturation : Float, lightness : Float }
toHsla : Color -> { hue : Float, saturation : Float, lightness : Float, alpha : Float }
setAlpha : Float -> Color -> Color
setRed : Float -> Color -> Color
setGreen : Float -> Color -> Color
setBlue : Float -> Color -> Color
setHue : Float -> Color -> Color
setSaturation : Float -> Color -> Color
setLightness : Float -> Color -> Color
alpha : Color -> Float
red : Color -> Float
green : Color -> Float
blue : Color -> Float
hue : Color -> Float
saturation : Color -> Float
lightness : Color -> Float
-- hex, rgba() and hsla() support, maybe color names?
parse : String -> Maybe Color
-- generate a string, that can be passed back to parse
toString : Color -> String
-- we can drop the alpha for that one
toHex : Color -> String
-- same as parse but default to black and support only hex
fromHex : String -> Color
I would add a few tools to help other packages development:
mapRgba: ({ red : Float, green : Float, blue : Float, alpha : Float } -> { red : Float, green : Float, blue : Float, alpha : Float }) -> Color -> Color
mapHsla: ({ hue : Float, saturation : Float, lightness : Float, alpha : Float } -> { hue : Float, saturation : Float, lightness : Float, alpha : Float }) -> Color -> Color
maybe mapRed, mapBlue....
And the few more things in the original posts (color names...)
I think the summary that @avh4 keeps updating in the top post is already pretty good.
I think the summary that @avh4 keeps updating in the top post is already pretty good.
Yes it is, mine was not a replacement, it was more a way to have an overview. It helps view the size of the API, at least for me.
Im really glad to see this package come back. The api looks good too. I suspect theres nothing I can do to help, but if there is I would love to be a part of this package.
Okay, it sounds like the core conversion functions seem stable enough to start working on. I've set up the project and CI, and will push a work in progress branch for the basic conversion shortly.
Given there's some debate about the record aliases for the {red, green, blue}, etc records, I think we can put off adding those immediately (they can easily be added later if it becomes clear they're valuable).
I agree, let's start, a lot of people need a color type.
Do you want me to prepare my color parser for review and inclusion in the project?
On September 7, 2018 6:51:41 AM GMT+02:00, Aaron VonderHaar notifications@github.com wrote:
Okay, it sounds like the core conversion functions seem stable enough to start working on. I've set up the project and CI, and will push a work in progress branch for the basic conversion shortly.
Given there's some debate about the record aliases for the {red, green, blue}, etc records, I think we can put off adding those immediately (they can easily be added later if it becomes clear they're valuable).
-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/avh4/elm-color/issues/4#issuecomment-419322092
-- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Do you want me to prepare my color parser for review and inclusion in the project?
@kuon after chatting with the elm-css author, I'm not sure I understand in what situations parsing CSS-like color strings will be useful. I'm thinking of leaving that out of 1.0.0, and trying to collect some info about who would use it and in what scenarios. (Can you open an issue if you have some off the top of your head?)
Okay, most of this has been implemented in #9, so please review the code there.
I split remaining questions in separate issues: #11, #12, #13.
Here is my best argument for not having a complex type as input to a function needing a color. Let's say your function works with Rgb255 values. Instead of making this explicit by having the input type
{ red : Int, blue : Int, green : Int }
you make it accept Color
thereby not giving any indication about the preferred color space. The user will then have to guess (or read the source code, or maybe you document it) which Color
to create to avoid multiple conversions.
I don't see any downside to accepting an explicit record. If the user wants to convert, they call the appropriate conversion function before calling the function.
An analogy would be to not accept List a
but instead some custom type that has both List a
and Array a
.
This is not to say this package is not needed. We do need color conversion. I'm just argumenting for not using it as a standard type in other packages.
Goals
Guarantees
API
Basic colors
For later discussion (for a future version)
The following are things that probably make sense to include, but I think are less urgent than everything above and can be postponed for now: