bgrins / TinyColor

Fast, small color manipulation and conversion for JavaScript
https://bgrins.github.io/TinyColor/
MIT License
5.08k stars 438 forks source link

toRgbString doc is missleading (misses alpha) #73

Closed bitplanets closed 9 years ago

bitplanets commented 9 years ago

Is toRgbString but your doc seems to not support it

toRgbString
var color = tinycolor("red");
color.toRgbString(); // "rgb(255, 0, 0)"

please add another example

toRgbString
var color = tinycolor("red").setAlpha(0.5);
color.toRgbString(); // "rgba(255, 0, 0, 0.5)"
bgrins commented 9 years ago

I suppose we should show that on all of the toString methods. toRgbString is addressed in setAlpha: https://github.com/bgrins/TinyColor#setalpha, but it would probably help to document what all of the methods do (including things like toHex).

bitplanets commented 9 years ago

Yes, you are right. But also toRgbString seems that only outputs rgb. I think toRgbaString would be a good method to add.

bgrins commented 9 years ago

I think toRgbaString would be a good method to add.

I've thought about adding this, but then you get into weird cases where you have an alpha set, call toRgbString(), and get incorrect data back. I think it's easier to always return the 'right' data and allow the user to get around that (via setAlpha(1) or toHexString()). We just need to make sure it's better documented.

Any chance you could file a PR with the updated docs?

bitplanets commented 9 years ago

I've thought about adding this, but then you get into weird cases where you have an alpha set, call toRgbString(), and get incorrect data back. I think it's easier to always return the 'right' data and allow the user to get around that (via setAlpha(1) or toHexString()). We just need to make sure it's better documented.

You can add only for consistency.

Any chance you could file a PR with the updated docs?

Yes. Comment something after so I get a notification (:

bgrins commented 9 years ago

Cool, thanks :). I think ideally we would have something documented like this for each of the toString functions (toHsvString, toHslString, toHexString, toHex8String, toRgbString, toPercentageRgbString, toName)

var color = tinycolor("red");
color.toRgbString(); // "rgb(255, 0, 0)"

color.setAlpha(0.5);
color.toRgbString(); // "rgba(255, 0, 0, 0.5)"
M-Zuber commented 9 years ago

What about the toRgb, toHsv, toHsl methods? It seems from the docs that they return an object with an a property, without touching the alpha:

var color = tinycolor("red");
color.toHsl(); // { h: 0, s: 1, l: 0.5, a: 1 }

which seems to be an unexpected behaviour

bgrins commented 9 years ago

It seems from the docs that they return an object with an a property, without touching the alpha which seems to be an unexpected behaviour

Yes, in general we return the a without a special request. Adding a special request for each would increase the surface area of the API and also be harder to guess. The theory is that a user could always just ignore the a if they don't care about it. We could add a note to the docs about this, though

bgrins commented 9 years ago

Fixed by #75