chaining manipulation methods about tinycolor HOT 16 CLOSED

That would be definitely be convenient. The only issue is that the return value of tinycolor.lighten() would not be the exact same object - at least if we implemented it like so on the tinycolor prototype:

lighten: function() {
    var args = [this].concat([].slice.call(arguments));
    return tinycolor.lighten.apply(null, args);
},

In other words:

var foo = tinycolor("red");
var bar = foo.lighten(); 
foo == bar // false

Which is maybe confusing? I think I would expect calling an instance method on the object to modify the existing instance in place, which would be doable but I'm not sure about doing it that way.

from tinycolor.

I was thinking something more like this:

tinycolor.prototype = {
    lighten: function(amount) {
        amount = (amount === 0) ? 0 : (amount || 10);
        var hsl = this.toHsl();
        hsl.l += amount / 100;
        hsl.l = clamp01(hsl.l);
        // modify the below with the changes made to the color:
        // this._r, this._g, this._b, this._a
        return this;
    },
   ...and so on...

call it:

tinycolor("red").lighten(10) // ... and so on

from tinycolor.

I agree the more I think about it - if we went with this chaining API then we should modify the color in place. This really an API question - this could be solved technically pretty easily either way. Pinging @levithomason and @marcandre who both may have some feeback about this.

So the main question is if we want to stick with A or switch to B:

// A
tinycolor.lighten("red");

// B
tinycolor("red").lighten();

// C - support both
tinycolor.lighten("red");
tinycolor("red").lighten();

The plus side to A is that if you had a color and wanted to apply a variety of changes to it:

// A
var color = tinycolor("red");
var light = tinycolor.lighten(color)
var dark = tinycolor.darken(color);

Luckily the constructor accepts a TinyColor object, so this same effect could be achieved like this:

// B
var color = tinycolor("red");
var light = tinycolor(color).lighten();
var light = tinycolor(color).darken();

As pointed out in earlier comments, the plus side of B would be chaining: tinycolor("red").lighten().saturate(). If we switched to B then a clear follow up question is if tinycolor.splitcomplement(color) should also switch to tinycolor(color).splitcomplement().

As for C, I'm thinking we don't officially support both options - and instead stick with one way and suggest that. The only benefit to C would be if we wanted to switch to B but not beak backwards compat - in which case we would remove it from the docs but still support it in code. At some point I will want to tag a v1 so will want to have the API settled by then, but I don't think breaking the API would be unreasonable given the current version.

from tinycolor.

your example is a bit confusing...

// B
var color = tinycolor("red");
var light = tinycolor(color).lighten();
var light = tinycolor(color).darken();

should be:

// B (user wants to create different colors from a base color)
var color = tinycolor("red"); // retruns a tinycolor object
var light = tinycolor(color).lighten(10); // returns a NEW, manipulated, tinycolor object,
var lighter = tinycolor(light).lighten(10); // returns a NEW, manipulated, tinycolor object

and support this situation

// B (user wants change the same color, without instantiating new objects every time)
var color = tinycolor("red"); // retruns a tinycolor object
var light = color.lighten(10); // returns a NEW, manipulated, tinycolor object,
var lighter = light.lighten(10); // returns a NEW, manipulated, tinycolor object

This last example is useless, since all the variables point to the same object, so they are all the same, and it should never be used this way (why would it anyway), but should be used like so:

var color = tinycolor("red").lighten(10).saturate(20); // every methods returns the same object without creating a new one

This makes a lot of sense, no?

from tinycolor.

Hi @bgrins and @yairEO. Glad to see this convo. I have been wanting to do these things as well. I agree with the chaining example above.

var color = tinycolor("red").lighten(10).saturate(20); // every methods returns the same object without creating a new one

Intuitively I've wanted to do two things:

1. Chain methods
2. Apply modifications inplace

Something like this:

var color = tinycolor('red');

color.spin(180).darken(30);  // color is now dark green

from tinycolor.

OK, so what are your thoughts about:

var color = tinycolor("red");
tinycolor.analogous(color, results = 6, slices = 30)
tinycolor.complement(color)
tinycolor.monochromatic(color, results = 6)
tinycolor.splitcomplement(color)
tinycolor.triad(color)
tinycolor.tetrad(color)

Switching to this (which would have the same meaning, and would obviously not be chainable since these return arrays of new colors):

var color = tinycolor("red");
color.analogous(results = 6, slices = 30)
color.complement()
color.monochromatic(results = 6)
color.splitcomplement()
color.triad()
color.tetrad()

As far as the readable functions, I'd say leave that alone since it takes in two colors and to me color1.readable(color2) is not any better than tinycolor.readable(color1, color2)

from tinycolor.

Regarding the color combinations, yep. Makes good sense and of course they are not chainable.

As for readable(). I'd assume all the methods would be available on the instance if some were.

from tinycolor.

As for readable(). I'd assume all the methods would be available on the instance if some were.

For me, I've used libs where there are both functions set directly on the main global and on instances. For instance, in jQuery you can do $(el).html() but also $.ajax(). And it's pretty normal to attach non-instance functions or properties to $.myplugin when the instance is defined as $.fn.myplugin = foo.

For whatever reason, readable feels different than the other functions to me. My gut says that it can live on it's own (along with other general utility functions that may get added in the future). I may be wrong about that though

from tinycolor.

I can see classifying the readability methods as utility methods and having them live only on the main global as well. This made me also reconsider color combination methods. Hm. Considering that it indeed feels different, I started asking why? I think becuase it does not modify the color in place, it just returns something. So, it makes sense to be a global object method.

I started sketching out what I thought the API would look like. I ended up closer to what I had first imagined a robust color library would look like back when I started searching. In TinyColor flavor, I envisioned something like this:

//
// Global object methods

// Color schemes (combinations)
tinycolor.analogous(color);
tinycolor.complement(color);
tinycolor.monochromatic(color);
tinycolor.splitcomplement(color);
tinycolor.triad(color);
tinycolor.tetrad(color);

//
// Instantiation and instance methods

var color = tinycolor('red');

// Color manipulation (chainable)
color.lighten();
color.brighten();
color.darken();
...

// Getters
color.getAlpha();
color.getBrightness();
color.getHue();         // perhaps add in future?
color.getRed();         // "
...

// Setters (perhaps add in future?)
color.setAlpha();
color.setBrightness();
color.setHue();         
color.setRed();
...

// Utilities
color.isValid();
color.isDark();
color.isLight();
color.isReadable();     // renamed for consistency
color.readability();
color.mostReadable();
color.equals();

// Conversions
color.toRgb();
color.toHsl();
color.toHex();
...

I can now see allowing this as well:

//
// Global object methods

// Utilities
tinycolor.isValid();
tinycolor.isDark();
tinycolor.isLight();
tinycolor.isReadable();     // renamed for consistency
tinycolor.readability();
tinycolor.mostReadable();
tinycolor.equals();

//
// Instance methods

// Color combinations
color.analogous();
color.complement();
color.monochromatic();
color.splitcomplement();
color.triad();
color.tetrad();

Which ever you decide, I will stick with TinyColor and help out in any way I can. I love what you've done so far and have every intention to build it out as you see fit.

from tinycolor.

I like your second group - I'm trying to decide if isValid in particular (and possibly isDark/isLight) belongs on an instance instead of a utility. For now I'm leaning towards isValid being on an instance method since that is something that may be wanting to check quite often - we could move that around though.

Here is a potential API for a v1 release:

//
// Global object methods

// Utilities
tinycolor.isDark();
tinycolor.isLight();
tinycolor.isReadable();     // renamed from readable
tinycolor.readability();
tinycolor.mostReadable();
tinycolor.equals();

//
// Instance methods

// Getters
color.isValid();
color.getFormat();
color.getAlpha();
color.getBrightness();

// Setters (chainable - edits in place and returns this)
color.setAlpha();

// Manipulation (chainable - edits in place and returns this)
color.lighten();
color.brighten();
color.darken();
color.desaturate();
color.saturate();
color.greyscale();
color.spin();
color.mix();

// Formatters (returns object or string)
color.toHsv();
color.toHsvString();
color.toHsl();
color.toHslString();
color.toHex();
color.toHexString();
color.toHex8();
color.toHex8String();
color.toRgb();
color.toRgbString();
color.toPercentageRgb();
color.toPercentageRgbString();
color.toName();
color.toFilter();
color.toString();

// Color combinations (returns array of new [TinyColor] objects)
color.analogous();
color.complement();
color.monochromatic();
color.splitcomplement();
color.triad();
color.tetrad();

from tinycolor.

This is really beautiful. I have your same hunch, that isLight() and isDark() could live on the instance. These are two I would use quite often for automatically choosing font and background colors. Then I think you've nailed it.

Can't wait to use this! Also, this may be a good point to consider the documentation rework as well?

from tinycolor.

I'm working on a branch with these changes, and I'll ask for feedback on a couple of specific things shortly.

Can't wait to use this! Also, this may be a good point to consider the documentation rework as well?

Which documentation rework? For the README or index.html? I'd love to have an updated demo / marketing page to go along with a v1 release (and @yairEO was working on something that could maybe be adapted for this in #55). I'm thinking we leave API documentation in README to keep things simple.

from tinycolor.

OK, pushed 4aeb7d5. @levithomason I'd like your feedback on a couple of things:

1. complement is kind of a weird one. It only returns a single color, so I put it in the "modifications" pile, but then I realized that would be kind of a weird modification. It is really more like a "combination". I guess we could make it a combination but just document that it returns a single TinyColor instead of an array?
2. At least for now, I kept mix as a "utility". The API for color1.mix(color2, 30) just seems kind of weird to me that it would modify it in place. I've never had a need for mix, so I may not be the best consumer for judging what makes sense. I'm inclined to leave it as a utility for now, and to revisit if we want to make it an instance method

TODO:

• docs
• update index.html

from tinycolor.

Take a look at the docs I've added for modification at https://github.com/bgrins/TinyColor/tree/v1api#color-modification and for combinations at https://github.com/bgrins/TinyColor/tree/v1api#color-combinations. I think it could still be better, but hopefully that will be a good starting point.

I'm pretty happy overall with the changes, so went ahead and opened #57. Please let me know if you have any feedback, I'll wait to merge it.

from tinycolor.

1. Agreed, it is odd as a modification. We can always spin 180 for the same affect. I love consistency as well, so returning an array with a single TinyColor object is great by me. This way a combination handler function doesn't need a special case for complimentary, it can always expect an array.

2. Would like to see mix on the instance if at all possible. I use this a lot as a replacement for partially transparent colors due to performance issues when rendering lots of partially opaque content. One example, font colors look fantastic when they are ~90% opaque, however, you can create lag when scrolling with all that transparency. So, setting my font color by mixing black or white with my background color I can avoid the transparency and lag.

Love the update docs :)

I'm tied up tomorrow but I'll give #57 a spin soon and give any feedback. Thanks for being so responsive and considerate. I'm really happy with TinyColor and my experiences here so far.

from tinycolor.

This was fixed by #57 in version 1.0

from tinycolor.