Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a contrast function #730

Closed
wants to merge 9 commits into from
Closed

Add a contrast function #730

wants to merge 9 commits into from

Conversation

Synchro
Copy link
Member

@Synchro Synchro commented Mar 22, 2012

This is a second take on the previously rejected #488.

This adds a contrast function that given an existing colour, returns a contrasting colour value, either black/white, or other configurable light/dark values, and the threshold at which the colour choice is made is configurable too.
Cases where you might want to use this:

  • ensuring text contrasts with a given background colour
  • ensuring background contrasts with a given text colour
  • ensuring borders contrast with background
  • ensuring links contrast with normal text
  • building gradients with sufficient colour range

A simple use example:

.someblock {
  background-color: @background;
  color: contrast(@background);
}

I've re-implemented the function so the method signature and behaviour is the same as the equivalent SASS function and added the threshold parameter. This is a good partner to the colour processing functions in @rmariuzzo's #596.

There is currently no concise/DRY way of achieving this same result in LESS: mixin guards are way more verbose and require repeating every time they need to be applied.

@matthew-dean
Copy link
Member

Again, what if I want the contrasting colors to be yellow or dark blue, depending on input color?

@Synchro
Copy link
Member Author

Synchro commented Mar 23, 2012

Did you look at the code? You can supply your own light and dark colours. You'd do it like this:

color: contrast(@background, blue, yellow);

which would compile to:

color: blue;

or

color: yellow;

depending on how light @background is.

@matthew-dean
Copy link
Member

Hmm, I thought I did. I only saw one input value, but maybe I was
looking at the wrong thing.

On 2012-03-23, at 12:32 AM, Marcus Bointon
[email protected]
wrote:

Did you look at the code? You can supply your own light and dark colours.


Reply to this email directly or view it on GitHub:
#730 (comment)

@matthew-dean
Copy link
Member

Oh, I see, I wasn't looking at the last iteration. Nice work! This is exactly the way I was thinking! I'd love it if it went in! ^_^

Use same luma calc for contrast calculation
@Synchro
Copy link
Member Author

Synchro commented Mar 23, 2012

I looked at the luma calculation in #591 but there were two things amiss: alpha was not included in the calculation, and the rgb coefficients in the calculation were some that I've not seen used anywhere else (though they are similar to the YIQ values).

I implemented a similar function using the ITU-R BT 709 values (which are apparently used by Firefox and suggested by the W3C), and also took alpha into account (though that can't be a complete calculation using only a single colour). Strictly speaking I've used a luminance calculation rather than a luma calc, which doesn't take gamma into account.

Initially I tried implementing .luma as a property of color objects, so that you could use color.luma in the same way that you can use color.alpha, however I couldn't figure out how to make that work - it would be much more elegant that way. Any advice on how to do that?

There's a good article on color contrast, though they use a slightly different calculation, and you can see how a good luminance calculation results in a much better choice of contrasting colours here.

Incidentally I've been having trouble with the test script overwriting output files, so it keeps failing because it's overwriting the CSS file it's supposed to be comparing with. There appear to be some test failures going on in the first section of functions.less (e.g. color: _color("evil red"); // #660000 fails) which I don't think are anything to do with me.

@Synchro
Copy link
Member Author

Synchro commented Apr 11, 2012

Any chance of getting this pulled in? There are some problems in twitter bootstrap I'd like to fix with this, in particular where it puts white text on yellow buttons because it's missing a contrast function...

@matthew-dean
Copy link
Member

I think with the work you've put in it, this is an easy slam dunk of a feature. Hopefully, it gets into the next release.

@Synchro
Copy link
Member Author

Synchro commented Apr 25, 2012

I've created an issue on twitter bootstrap to make use of this function when it's available.

@Synchro
Copy link
Member Author

Synchro commented Jun 12, 2012

Please can this get pulled in? Someone else posted yet another contrast function (linked above), so there's clearly demand for it. It would be great if this could get in in time for Bootstrap 2.1.

@matthew-dean
Copy link
Member

I agree. It would even be good to get yes / no on pull requests. That is, "Will go in, but is pending", "No, rejected as a feature", or even "Seen it but haven't decided yet".

I don't know if more volunteers are needed or we need to tie @cloudhead to a chair under bright lights and then go through a list of questions. I think two others were added as admins to go through the issue list and rank issues, but haven't seen much come from that yet.

@krisives
Copy link

Yes please sign off on pull requests even if it takes a while to actually do a merge just knowing if the feature is accepted would be nice.

@jmgunn87
Copy link

Well let's just fork less.js and start merging and using features we want!

@mythril
Copy link
Contributor

mythril commented Jun 13, 2012

@jmgunn87 I've already moved on to Stylus http://learnboost.github.com/stylus/ as it has some awesome features to it (such as user-defined value oriented functions). I was finding that writing when() guards could get explosive if you wanted to do anything relatively complex and apply it to multiple different properties.

My only complaint is that their white-space oriented language leaves a lot to be desired when it comes to error reporting

@jmgunn87
Copy link

wow. that is awesome. yeah, see you later less...

@Synchro
Copy link
Member Author

Synchro commented Jun 13, 2012

I find guards pretty useless - while it's possible to implement a simplistic contrast function using guards, it would result in hundreds of additional rules if you needed to do it often, rather than just having a built-in function to do it inline.

I'm keen to see this in LESS principally because other projects use it, in particular Bootstrap, and that's also why I don't see much value in forking it: I don't want to manage a whole fork, but LESS could clearly use some help. You could of course submit a patch to Bootstrap to convert it all to stylus, but that's a fair bit of work. SASS is certainly more capable than LESS, but I was put off it by the ugly syntax they started with - SCSS is better.

@krisives
Copy link

I find guards pretty useless - while it's possible to implement
a simplistic contrast function using guards, it would result in hundreds of
additional rules if you needed to do it often, rather than just having a built-in function to do it inline.

This seems like a short coming of less to implement functions in user-space and not a good reason

I don't want to manage a whole fork

I agree a fork wouldn't help many people

but LESS could clearly use some help.

Which isn't happening right now because pull requests aren't getting merged

@jmgunn87
Copy link

Which is why cloudhead needs to open the project up.

On Wed, Jun 13, 2012 at 11:16 PM, Kristopher Ives <
[email protected]

wrote:

I find guards pretty useless - while it's possible to implement
a simplistic contrast function using guards, it would result in hundreds
of
additional rules if you needed to do it often, rather than just having a
built-in function to do it inline.

This seems like a short coming of less to implement functions in
user-space and not a good reason

I don't want to manage a whole fork

I agree a fork wouldn't help many people

but LESS could clearly use some help.

Which isn't happening right now because pull requests aren't getting merged


Reply to this email directly or view it on GitHub:
#730 (comment)

@matthew-dean
Copy link
Member

Forking is a big deal. I wouldn't want to do it unless LESS really looked abandoned as far as bug fixes, and without an endorsement of a major library.

Unless the forked library ended up being a perfect superset of LESS. (So that a LESS file was a valid X file.) I mean, you can always fork for yourself, but it would take a long time to get people on board.

What about asking the question as a separate Github issue (for this library)? Or contact Alexis and ask him directly if he needs support?

@jmgunn87
Copy link

Yeah, good idea.

On Thu, Jun 14, 2012 at 12:06 AM, matthewdl <
[email protected]

wrote:

Forking is a big deal. I wouldn't want to do it unless LESS really looked
abandoned as far as bug fixes, and without an endorsement of a major
library.

Unless the forked library ended up being a perfect superset of LESS. (So
that a LESS file was a valid X file.) I mean, you can always fork for
yourself, but it would take a long time to get people on board.

What about asking the question as a separate Github issue (for this
library)? Or contact Alexis and ask him directly if he needs support?


Reply to this email directly or view it on GitHub:
#730 (comment)

@fat
Copy link
Contributor

fat commented Jun 13, 2012

FWIW I think Less was only ever envisioned as a stopgap to hold us over until CSS got better.

At least that's largely how we use it in Bootstrap.

@cloudhead has told me a number of times that as soon as CSS get's better, he'll just write a Less -> CSS script and be done with it.

I think the goal at this point shouldn't be to distance us even further from CSS - particularly as we see variables and other features just starting to land in CSS.

I think people should try to think of LESS on the same terms as CSS (not SASS). Do you think a contrast method would ever land in CSS? I personally don't think so.

To me, it seems more like something which should be in a library - not a core part of a language.

Just my 2 cents.

@Synchro
Copy link
Member Author

Synchro commented Jun 13, 2012

On 14 Jun 2012, at 01:26, Jacob Thornton wrote:

Do you think a contrast method would ever land in CSS? I personally don't think so.

To me, it seems more like something which should be in a library - not a core part of a language.

And then we'll be back at square one, using some other language to generate CSS. The reason that functions like spin etc are in LESS is that it's the most logical place to put them, (pseudo)directly in the presentation layer. We can't realistically do them in client-side JS (it's bad enough having interactivity fail with JS off, let alone presentation), so we'd end up moving them into PHP, ruby or whatever, that's far more abstracted from CSS and has a danger of leaking into other layers - and that's the unpleasant place we were in before LESS came along.

Colour processing operations are to colours what changing widths and heights are to sizes. I really don't see it being any more out of place in 'future-CSS' than the myriad 3D, audio and animation properties we have already - if we can use CSS to transform in space/time/audio dimensions, why not in colour ones too?

I wouldn't hold your breath on CSS being updated that fast - the pace of CSS3 adoption has been fairly glacial, and doing it in a LESS style means we don't have to wait (we could perhaps use CSS4 syntax in LESS?).

@krisives
Copy link

as soon as CSS get's better, he'll just write a Less -> CSS script and be done with it.

Not holding my breath for CSS getting better in a way we can depend on.

I think the goal at this point shouldn't be to distance us even further from CSS - particularly as we see variables and other features just starting to land in CSS.

This is still many years from being usable with the way vendors implement CSS changes.

I think people should try to think of LESS on the same terms as CSS (not SASS). Do you think a contrast method would ever land in CSS? I personally don't think so.

Fair point, but there are other features of LESS that will likely never be adopted by the working CSS standards bodies.

@matthew-dean
Copy link
Member

I'm with some other responses in that:

  1. CSS is not going to improve to replace LESS. If they use variables as proposed, it's not as elegant as LESS. And are mixins even on the table for any CSS working group? But the color functions alone make LESS powerful.

  2. Whether or not it would GO into CSS isn't necessarily the best way to evaluate LESS. Something like contrast is absolutely intuitive next to the other functions that do exist. Contrast wouldn't go into CSS because CSS really has no concept of helper functions. That is, values that can evaluate to other values. LESS is a pre-processing language, which means that it is partially abstracting what happens in CSS, by it's very nature. In an ideal universe, CSS may adopt the features of LESS, but I find that extremely unlikely, based on their work thus far. It's just not in the W3C's DNA. CSS is pretty much the styling version of XHMTL2, where everything is rigidly defined and over-verbose. If something like the HTML5 working group for a CSS version that could pressure W3C the way HTML5 did, maybe that could change. TL;DR LESS is not CSS. It's additive to CSS. It's CSS+.

I like LESS's simplicity, so I would never recommend feature-stuffing to the degree SASS does, which is crazy. A contrast function is not; it's pretty much on par with lighten, darken, fadein, fadeout.

Forking or addressing how LESS pull requests get resolved I vote is kept to a separate issue.

@krisives
Copy link

which is crazy. A contrast function is not; it's pretty much on par with lighten, darken, fadein, fadeout.

I agree.

Forking or addressing how LESS pull requests get resolved I vote is kept to a separate issue.

But this is a good example of the problem and peoples frustrations.

@jmgunn87
Copy link

why is forking a big deal? have you ever branched using git? the same thing.

@Synchro
Copy link
Member Author

Synchro commented Jun 15, 2012

That's not the issue - it's not the trivial practical matter of forking, it's the political issues. When you fork a popular project, none of the projects using it will fork with you without some major wrangling. I'd love to use my contrast function with bootstrap, but bootstrap is linking to the current less project, and thus it would break if bootstrap accepted changes for my forked version (which they obviously wouldn't do), hence my desire to get it into the original project, not my own fork.

@jmgunn87
Copy link

They will certainly start using it if merge some of the good features suggested here.
You could easily fork bootstrap and modify as needed.

@krisives
Copy link

You could easily fork bootstrap and modify as needed

Unadopted this wouldn't help much

@matthew-dean
Copy link
Member

This conversation went off track. Revisiting my previous position, I agree with @Synchro that this is an intuitive and natural feature to add based on already-existing functions, such as "mix".

@lukeapage
Copy link
Member

Based on everyone seeming to think this is a good idea, I have squashed the commits into 1 and pulled.

@matthew-dean
Copy link
Member

Great, thanks @agatronic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants