Open Anton-4 opened 4 years ago
I have added a function hex
that returns a Result
and also a function hexOrRed
which defaults to the color red if the result was an error. It feels a bit dirty but I think having to deal with result handling every time you want to use a color is too inconvenient.
The test suite is failing at "Run webfactory/ssh-agent@v0.2.0" with error "##[error]The ssh-private-key argument is empty. Maybe the secret has not been configured, or you are using a wrong secret name in your workflow file.". I feel like I should not mess with this, can you take a look @mdgriffith?
Wouldn't it be easier to have an Int -> Color
version? Something like
hex : Int -> Color
hex n =
rgb255 (n // 0x10000) (n // 0x100 |> modBy 0x100) (modBy 0x100 n)
-- example
white : Color
white = hex 0xffffff
Do you mean easier in use or easier in implementation?
Css and many online color tools use hashtag prefixed hex notation.
These online tools also often use click to copy.
I chose Strings for minimal friction with existing tools, making it easier to port css and familiarity.
It is also common to use for example fff
as a shorthand for ffffff
, so that would have to be added as well.
I also like the hexOrRed
function to notify users when they made a mistake such as providing a 7-letter hex code as argument.
I'm not saying you're wrong just outlining my thought process and arguments :) .
I was also thinking a little bit about performance. I haven't benchmarked any of it but I'm assuming all that string parsing can't do very well. Maybe it doesn't matter.
The 0x
prefix actually feels more natural to me because it doesn't need quotes, but obviously I can't speak for everyone. Some tools I use (such as color syntax highlighting) understand the 0x
notation as well.
FWIW, here are hex versions long and short without and with alpha channel. I haven't tested these extensively but I believe they are correct. The names may be argued about. :D
-- regular 0xRRGGBB
hex6 : Int -> Css.Color
hex6 n =
rgb (n // 0x10000) (n // 0x100 |> modBy 0x100) (modBy 0x100 n)
-- short 0xRGB
hex3 : Int -> Css.Color
hex3 n =
rgb
(n // 0x100 * 0x11)
((n // 0x10 |> modBy 0x10) * 0x11)
(modBy 0x10 n * 0x11)
-- with alpha 0xRRGGBBAA
hex8 : Int -> Css.Color
hex8 n =
rgba
(n // 0x1000000)
(n // 0x10000 |> modBy 0x100)
(n // 0x100 |> modBy 0x100)
(toFloat (modBy 0x100 n) / 0xff)
-- short with alpha 0xRGBA
hex4 : Int -> Css.Color
hex4 n =
rgba
(n // 0x1000 * 0x11)
((n // 0x100 |> modBy 0x10) * 0x11)
((n // 0x10 |> modBy 0x10) * 0x11)
(toFloat (modBy 0x10 n) / 0xf)
I have converted the PR to a draft now so it does not get merged in it's current state. I suggest we let @mdgriffith decide on the use of Ints or Strings or perhaps both.
Any updates on this?
Having hex colors functionality will be great - @mdgriffith @Anton-4 any updates?
@jaladh-singhal everything is ready for @mdgriffith to decide whether he wants to use hex functions that accept Int
or String
or both.
Would be great to have hex functionality, @mdgriffith @Anton-4
I think I am still going to clean up the code a bit. Let me know if you have any suggestions @mdgriffith !