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

Color API #44

Closed
nikgraf opened this issue Dec 12, 2016 · 14 comments
Closed

Color API #44

nikgraf opened this issue Dec 12, 2016 · 14 comments

Comments

@nikgraf
Copy link
Member

nikgraf commented Dec 12, 2016

#I told @mxstbr to help out here and especially with the color related API 🙌 Before starting to code I want figure out how you think about the API. There was already some discussion here #24, but there are a couple things unclear to me. This is a proposal. Please correct me if I'm wrong or you see room for improvement.

  • We want everything color related in the lib to make sure it's properly flow-typed & tested.
  • The output of every color related function should be a string.
  • We want to create the smallest possible output. For colors without opacity we use hex e.g. #FF553A. If possible only 3 characters #FFF instead of #FFFFFF. In case of opacity we generate rgba(255,0,0,0.3).
  • A function might take a string or multiple values e.g. rgb('22, 255, 0') and rgb(22, 255, 0) and rgb({ red: 22, green: 255, blue: 0 }). Not sure about the object structure. Looks handy, but usually I prefer to have one recommended way of doing things and by exposing less possible ways we would enforce it. (We can always add it later)
  • Functions like saturate accept hex strings or rgb or rgba or hsla strings e.g. '#FFDDFF' or 'rgb(22, 255, 0)', but don't accept plain single values like rgb (there is no conversion). Otherwise it could get confusion if they accept hsla or rgba. On the other hand if we could accept the object notation, but as said it's not necessary.
  • In case of manipulation functions we use a functional style like Rambda with the first argument/arguments being the parameters and the last one the base color. Due the nature of the this API design composition is possible e.g.
// nesting of function calls
const prettyRed = saturate(20, darken(10, rgb(100, 0, 0)));
const prettyBlue = saturate(20, darken(10, rgb(0, 100, 0)));

const prettify = compose(
  darken(10),
  saturate(20)
);
const prettyRed = prettify(rgb(100, 0, 0));
const prettyBlue = prettify(rgb(0, 100, 0));

Note: please ignore 10 & 20 arguments. I need to learn more about color theory first to understand what parameters make sense.

What do you guys think? @bhough @mxstbr @guzart

@mxstbr
Copy link
Member

mxstbr commented Dec 12, 2016

I love this!!

Two discussion points:

I think we should accept the object structure too, some people prefer that and this shouldn't be the most opinionated lib – it's not like that hurts us. (also, jss has added support for object syntax for almost everything so people might be passing those in as variables)

I like the fp/ramda-style design! I don't think we should necessarily sell it like that, but even if beginner users use it (and don't even compose) compare these two:

// Non-fp style
saturate(darken(rgb(0, 100, 0), 10, 20)

// Fp/ramda style
saturate(20, darken(10, rgb(0, 100, 0)));

The second one is so much easier to read, because you avoid having to backtrack with your eyes constantly. On top of that you can compose functions which is really powerful, so I think it'd be a good idea to build it like that even though it deviates from the Sass API a bit.

What do you think about that @bhough?

@geelen
Copy link
Member

geelen commented Dec 12, 2016

Just out of interest, would it be better to have fewer top-level functions and instead return a smarter object?

const baseBlue = rgb(0, 0, 150)

const X = styled.div`
  background: ${baseBlue};
  border: 2px solid ${baseBlue.darken(20).saturate(10)};
  &:hover { background: ${baseBlue.lighten(10)} }
`

We could do this by either overloading toString or defining something like toCSS() that styled-components knows about when doing the interpolations. Feels like a better way of encapsultating the potential colour-related operations than a bunch of top-level methods.

@mxstbr
Copy link
Member

mxstbr commented Dec 12, 2016

color does this, but I'm not a fan. It's pretty tedious to use and is totally different from Bourbon/Sass (which most people will know) so the learning curve will be pretty high, which I'd like to avoid.

Having separate functions seems much more sensible to me, things can always be extracted to variables or functions.

@geelen
Copy link
Member

geelen commented Dec 12, 2016

Hmm, what makes it tedious? Looks pretty ok to me, just the hsl().string() is a bit superfluous.

I don't think trying to make it like bourbon and Sass is all that important. JS is different, we have real objects with methods, not just strings.

Global functions would be great, though, if you could do this:

const base = '#444';
styled.div`
  background: ${base};
  border: ${darken(20, base)};
`

But yeah, IMO if you have to start by wrapping colour values in an object then you may as well use object methods to manipulate it.

@nikgraf
Copy link
Member Author

nikgraf commented Dec 12, 2016

@geelen Spot on. This is in my initial proposal as well, but not good enough explained. Just updated the docs.

I would recommend: Functions like saturate accept hex strings or rgb or rgba or hsla strings e.g. '#FFDDFF' or 'rgb(22, 255, 0)'

If I understood correctly Max would like to support both: the function and the string notation. The object notation in the end also returns a hex string e.g. '#FFF' === rgb({ red: 255, green: 255, blue: 255 })

darken(20, '#444');
darken(20, rgb({ red: 100, green: 100, blue: 100 }));

@mxstbr totally agree, I wouldn't mention Rambda or FP at all in the docs. Just an example on how to compose it. People who like FP will notice it anyway …

So everybody happy with the API? We do object and string notation?

@bhough
Copy link
Contributor

bhough commented Dec 12, 2016

Sorry a little late to the discussion here:

I think we should accept the object structure too, some people prefer that and this shouldn't be the most opinionated lib – it's not like that hurts us.

Definitely agree that we should support object notation wherever possible. The goal is to make polished work well with any css as js approach, and that will go a long way towards that.

Having separate functions seems much more sensible to me, things can always be extracted to variables or functions.

As I think I mentioned in a previous issue, the problem with forcing colors to be instances of a color class of sorts, is that every color needs to be setup that way from the start. That is hard to ask of users I believe. I wouldn't want to force people using polished with a theme that wasn't built withpolished to have to convert all colors over to our proprietary color implementation just to be able to manipulate them.

As @mxstbr mentioned, that was one of the reasons we shied away from using color.

@nikgraf
Copy link
Member Author

nikgraf commented Dec 12, 2016

@bhough what do you think about the API I described + support for object notation? Anything to change?

@jaydenseric
Copy link

jaydenseric commented Dec 14, 2016

Stick to supporting the CSS spec for modifying colors, or else this will be much like the dark days of using mixins for vendor prefixes. You had to remember when to use them for your level of browser support, and the refactoring when Autoprefixer came on the scene was a hassle.

Ideally when browser support is good, transforming the color function values can be dropped without users having to refactor their style rules. A lot of teams are already used to using the spec way to manipulate colors thanks to cssnext.

Here is the postcss color function plugin: postcss/postcss-color-function.

@nikgraf
Copy link
Member Author

nikgraf commented Dec 15, 2016

@jaydenseric awesome suggestion, didn't know about it

@jaydenseric jaydenseric mentioned this issue Dec 16, 2016
@nikgraf
Copy link
Member Author

nikgraf commented Jan 23, 2017

@jaydenseric I learned that a lot of these support percentage as a value from 0-100 if a percent sign % is used. We can't do this in JS, but used 0-1 instead. This also matches with the alpha definition. Do you have any other suggestion?

@jaydenseric
Copy link

jaydenseric commented Jan 24, 2017

If it were a JS API (as opposed to parsing the CSS strings in place) that would be fine, along with separating parameters with commas. The most useful thing from the spec would be to copy function names and behaviors.

At that point, it seems better for advanced users to install their own color library of choice that allows destructured importing of just the APIs they need. Color libraries can massively bloat client bundles. Often by added one more variable to your theme palette you can remove the need for a 30kb library.

43 components into my current styled-components project (with a theme configured with a 5 color palette) and the only 2 color APIs that would be nice are hex -> rgba, and to a lesser extent perhaps brightening/darkening.

Here is my tiny helper:

/**
 * Converts a CSS hex color value to RGBA.
 * @param {string} hex - Expanded hexadecimal CSS color value.
 * @param {number} alpha - Alpha as a decimal.
 * @returns {string} RGBA CSS color value.
 */
export function hex2Rgba (hex, alpha) {
  const r = parseInt(hex.substring(1, 3), 16)
  const g = parseInt(hex.substring(3, 5), 16)
  const b = parseInt(hex.substring(5, 7), 16)
  return `rgba(${r}, ${g}, ${b}, ${alpha})`
}

@nikgraf nikgraf mentioned this issue Feb 18, 2017
2 tasks
@bhough
Copy link
Contributor

bhough commented Mar 3, 2017

@nikgraf anything remaining from this issue we want to address or is this good to close?

@nikgraf
Copy link
Member Author

nikgraf commented Mar 4, 2017

@bhough good to close 👍

@nikgraf nikgraf closed this as completed Mar 4, 2017
@dariye
Copy link

dariye commented May 13, 2019

hey, @nikgraf + @bhough, thanks for polished! @nikgraf, I'm looking to better understand color theory. Are you able to share some resources that were most helpful to you?

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

No branches or pull requests

6 participants