Uniform full length hex input like #ffffff is not preserved on output from .toHexString()

[beporter]

If you create a new tinycolor('#ffffff'), the result of .toHexString() is '#fff' and does not match the input. See this fiddle for an example: http://jsfiddle.net/2va59/3/

This causes problems if you happened to be keying an object by hex value, for example, and depend on the output of TinyColor (via the Spectrum color picker, btw) to look up the correct values.

{"#e55731":{"field":"9","detail":"386"},"#ffffff":{"field":"15","detail":"387"})

The simple solution is to disable this if() statement entirely, but I could see an argument for an alternative .toFullHexString() method or an passing an optional argument (in pseudo-code) along the lines of .toHexString(bool shortenHexIfPossible = false) as well.

[beporter]

If you agree a fix is in order and have a preference on which approach you'd prefer, I'd be happy to submit a pull request, otherwise feel free to close as "will not fix" if you don't feel this is an issue worthy of addressing.

[bgrins]

Shortening if possible has been the default behavior since it was first built. However, this issue has been brought up a couple of times in the last few weeks.

I have a modified version of TinyColor within spectrum that can do tinycolor('#ffffff').toString("hex6"). I've been meaning to port this functionality (along with some other additions) back into tinycolor. Two questions are:

1. Should the hex6 behavior be an overload or the default?
2. Is it important that the input directly matches the output? Does tinycolor('#fff').toHexString() need to return #fff?

Thoughts?

[beporter]

I did eventually find the undocumented 'hex6' option in Spectrum, and this has solved my issue entirely. In my case, the data I am using to populate the spectrum palette is also being re-retrieved in the change() event using the hex value of the selected color as the lookup key, so maintaining the 6 characters is essential to my app. To answer your questions a little more directly:

1. I'm usually against changing default behavior for a mature (i.e.: widely adopted) package if it can be avoided. You don't know who out there has built their app to depend on the conversion from 6-char to 3-char, so I'm in favor of making the shortening override-able by argument as Spectrum already implements to provide an avenue for compatibility.
2. On the contrary to point 1, I also think it's important for a package to preserve data accuracy when possible. An add() function that returns all values in base 16 is sometimes able to return an accurate result in base 10 "by accident" for some special cases (add(1, 1)). That's how I feel TinyColor operates right now-- it maintains perfect accuracy...unless the color is represented by a wholly uniform hex value such as #cccccc, in which case your result will be "mangled" (while remaining technically correct.) This is the only thing that rubs me the wrong way.

If it had been my project, I probably would have never implemented the shortening step to begin with so as to guarantee that the app always returned a consistent 6-char hex value, but that doesn't necessarily help address the question now.

[bgrins]

Thanks for the thoughts.

I don't really mind slightly breaking backwards compatibility with a "version 1.0" as long as it is easy to resolve the problem (like setting tinycolor.hexShortByDefault = true or something) and we document it in the README. I'd rather move forward if there is a better way and give people who were relying on the old behavior a fix.

That said, if we switched the default to 6 character, wouldn't you have the same problem come up if you inputted a 3 character and called toHexString() on it (since you would now be getting a 6 character return value).

[beporter]

Yes, I was actually already drafting this update specifically in response to your point about tinycolor('#fff').toHexString() == '#fff':

I think this preservation is actually more important for Spectrum than TinyColor. Given that the only data supplied to the event handlers is the color selected, it's the only resource a programmer has to "find" any other related information they may need to use. Preserving the palette input precisely in the event it is hex might be more critical although this obviously complicates the code a bit.

That actually reminds me too-- I have a feature request I will post separately.

[bgrins]

So the proposal is for toHex and toHexString to return full length or shorthand depending on input? What about the case where the input was not a hex, like rgb 255 0 0, should that be a 6 or 3 character hex value returned?

[bgrins]

Also, @beporter returning exactly the original string is not always possible or desirable. For instance,

tinycolor("rgba 255 20 10 .5").toRgbString()
"rgba(255, 20, 10, 0.5)"

In that case, it cleans up some of your formatting and normalizes it to a standard output format. I see where you would be wanting to use hexes as identifiers though.

I have added the following functionality inside spectrum, which I could port back into tinycolor if you think it would solve your issue:

toHexString: function (force6Char) {
    return '#' + rgbToHex(r, g, b, force6Char);
}

[beporter]

You're right that there is ambiguity abound in this topic. I don't believe any of us are going to be able to fully resolve it. (Let's not even talk about #FFFFFF vs. #ffffff!)

The reason I prefer 6-char hex strings is because they are not special cases and don't need to be treated differently. #ffcc00 may be more verbose strictly than necessary, but it's just as accurate and (marginally) easier to shorten to 3 chars after the fact than it would be to length a shorthand value back up to 6.

As for the result of creating a TinyColor with rgba and outputting the hex value-- I don't see any need to maintain input/output fidelity there because the format is already different and you'd explicitly be performing a format conversion. I was only concerned with hex in/out.

Bottom line though is that as I mentioned in my comment from the 19th, the (undocumented) force6char option in the Spectrum version of TinyColor actually already solved my problem. I honestly don't have an issue with using the same approach in TinyColor directly as long as some method is exposed for controlling the formatting of the hex return value (which you've already done in Spectrum.)

[bgrins]

Yeah I understand your point about 6 character hex strings not having special cases. I think that we can add the option to force6char and I would even be OK with possibly making that the default via a settable option, like tinyColor.force6charByDefault = true, as I am seeing more needs to use the 6 char (as you point out with spectrum). For now I'll just add the parameter into TinyColor.