Giter VIP home page Giter VIP logo

Comments (9)

beporter avatar beporter commented on May 23, 2024

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.

from tinycolor.

bgrins avatar bgrins commented on May 23, 2024

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?

from tinycolor.

beporter avatar beporter commented on May 23, 2024

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.

from tinycolor.

bgrins avatar bgrins commented on May 23, 2024

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).

from tinycolor.

beporter avatar beporter commented on May 23, 2024

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.

from tinycolor.

bgrins avatar bgrins commented on May 23, 2024

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?

from tinycolor.

bgrins avatar bgrins commented on May 23, 2024

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);
}

from tinycolor.

beporter avatar beporter commented on May 23, 2024

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.)

from tinycolor.

bgrins avatar bgrins commented on May 23, 2024

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.

from tinycolor.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.