omgovich / colord

👑 A tiny yet powerful tool for high-performance color manipulations and conversions
https://colord.omgovich.ru
MIT License
1.67k stars 49 forks source link

New API to get the color format used to create the colord instance #43

Closed AnsonH closed 3 years ago

AnsonH commented 3 years ago

I recently migrated from tinycolor2 after discovering this awesome library 😄. However, your library is currently missing a similar API to tinycolor2's getFormat method , which returns the color format used to create the tinycolor instance. I found this API extremely useful when I try to parse user-inputted color code strings.

It would be great if there is a similar API in your library. For example:

var color = colord("#ff0000");
color.getFormat();  // "hex"

color = colord({ r: 139, g: 200, b: 208, a: 1 });
color.getFormat();  // "rgba"

Thank you very much!

omgovich commented 3 years ago

Hey! Thanks for the kind words! I'll definitely think about that!

dbismut commented 3 years ago

I support this as well, I need it to switch from tinycolor in leva! Ideally also an API similar to the toString(format?: string) would be awesome, but I can probably sort it out without it.

Thanks @omgovich ;)

omgovich commented 3 years ago

Hi! It feels like toString(format?: string) is not the best way for TS applications since it makes the method's return type unpredictable.

dbismut commented 3 years ago

I think in this case it would be string? Or are you questioning naming that function toString as it conflicts with native Object.prototype.toString?

omgovich commented 3 years ago

Oh, sorry, my bad 😅 I was thinking about different API when was writing my comment =) It just feels that having toString(format) will force me to create toObject(format) which would be hard to type in TS.

omgovich commented 3 years ago

Anyway having getFormat and toString shouldn't be a big deal. The only thing that bothers me here is the bundle size changes, but I'll try to make the new methods as small as possible.

dbismut commented 3 years ago

Haha so that's a case of typings over feature I guess ^^ That's ok no worries I can sort this out, I'll ignore the typings on my side. Thanks!

omgovich commented 3 years ago

I'll try to add getFormat and toString in a few days. Will let you know once I have the first results.

dbismut commented 3 years ago

The only thing that bothers me here is the bundle size changes, but I'll try to make the new methods as small as possible.

Sure! again don't sweat it too much about toString, that one you can leave in userland if it's more than a two-liner.

omgovich commented 3 years ago

Oof. getFormat() appears to be a big deal for me.

Tinycolor2 is mutable and it keeps the same instance when a developer changes a color (which is super weird in my opinion), so it's easy for them to preserve data about the input color.

const color1 = tinycolor("#000");
const color2 = color.lighten(10)
console.log(color1 === color2); // true (I guess nobody expects that)

My library is immutable (this approach is way safer and more predictable for developers) — Colord creates a new instance on each color change.

For example, in the method below I convert a color to HSL and use it to create a new Colord instance.

public lighten(amount = 0.1): Colord {
  return colord(lighten(this.rgba, amount)); // lighten returns HSL color
}

It makes it super hard to get the input format after a transformation.

const color1 = colord("#000");
const color2 = color1.lighten(0.1)
console.log(color1 === color2); // false
// 😞
console.log(color1.getFormat()); // 'hex'
console.log(color2.getFormat()); // 'hsl'
dbismut commented 3 years ago

@omgovich In my specific use case I actually only need getFormat to keep track of the initial input. So actually even this would work for me:

import { getFormat } from 'colord'

const format = getFormat('#000') // hex
const format = getFormat('#ff000033') // hex8
const format = getFormat({ r: 0, g: 0, b: 255 }) // rgb
const format = getFormat({ r: 0, g: 0, b: 255, a: 1 }) // rgba (not sure about that one though)
// etc
omgovich commented 3 years ago

Also thought about a top-level function 👍

dbismut commented 3 years ago

Looking at the source code, it could be a derivative of the parse function, that would be aware of which parser matched the user input?

omgovich commented 3 years ago

Yeah, it has to be a simple update of the parse I guess.

BTW: Do you think we need to work "hex8" or "rgba". From the library's point of view, I think it makes sense to return a color model of the input (hex, rgb, hsl, lab, lch, ...). Detection of the alpha channel seems like another task.

dbismut commented 3 years ago

In leva we show the alpha scale of the react-colorful only when the user input has alpha. But if there's another way to check if alpha is part of the user input, then sure, we don't need hex8 or rgba.

omgovich commented 3 years ago

Got it. One of the possible ways is:

colord('#aabbcc80').alpha() < 1 // true or false
dbismut commented 3 years ago

That might not be enough as for me #f00000ff has alpha set although being equal to 1. But don't sweat it if that's too much bloat.

omgovich commented 3 years ago

BTW: Why don't you want to always display the alpha picker in Leva? Colord outputs an alpha value only if alpha is not 1.

omgovich commented 3 years ago

In opposite to tinycolor which has toHexString and toHex8String, Colord has single toHex method which return hex8 if a color's alpha is less than 1.

dbismut commented 3 years ago

Ok I think I can make it work.

omgovich commented 3 years ago

Hi guys. Please check #44. Would it work for your apps?

dbismut commented 3 years ago

Should be fine!

omgovich commented 3 years ago

Released v1.7 with getFormat utility included 🚀 +50 bytes and zero performance affect. I guess we did our best)