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 support for cssvar colors to withAlphaVariable and withAlphaValue #4659

Conversation

stefan-schweiger
Copy link
Contributor

@stefan-schweiger stefan-schweiger commented Jun 16, 2021

As mentioned in this discussion #4205 (and can be seen in this demo https://play.tailwindcss.com/Q7aU4F9lpb) currently using css variables to define colors breaks down somewhat if the are used anywhere were an alpha channel is added.

I've extended the withAlphaVariable and withAlphaValue utils to check if a css variable was defined as numeric values --color-var: 255, 0, 255 and consequently used like this in the config DEFAULT: 'rgb(var(--color-var))'.

My implementation will work for both rgb and hsl as long as they were valid colors to begin with and should be somewhat resistant to inconsistent whitespace use.

The only downside of this implementation is that you won't be able to use variables containing hex colors directly but you need to define them as a numeric triple as shown in my example above.

Since this is my first PR to this project I really hope everything is in order but feel free discuss issues of this solution with me 🙂

@stefan-schweiger
Copy link
Contributor Author

It took me a bit to figure out how to get a working local build with a custom config, but I was also able to verify that my demo is now working correctly. (The second square uses a non-rgb variable and therefore it's expected that it isn't semi-transparent)

image

@stefan-schweiger stefan-schweiger changed the title Alpha colors with cssvars Add support for cssvar colors to withAlphaVariable and withAlphaValue Jun 16, 2021
@memic84
Copy link

memic84 commented Jul 6, 2021

Any plans to merge this? We really need a solution for custom css variables with bg-opacity.

@stefan-schweiger
Copy link
Contributor Author

@memic84 it's pretty hidden in the documentation but you can actually use this as a "workaround":

const getColor = (colorVar, { opacityVariable, opacityValue }) => {
  if (opacityValue !== undefined) {
    return `rgba(var(${colorVar}), ${opacityValue})`;
  }
  if (opacityVariable !== undefined) {
    return `rgba(var(${colorVar}), var(${opacityVariable}, 1))`;
  }

  return `rgb(var(${colorVar}))`;
};

module.exports = {
  // ...
  theme: {
    extend: {
      colors: {
        primary: (params) => getColor('--color-primary-rgb', params),
      }
   }
}

I still would like this PR merged, because I think everyone would benefit from making this easier.

@PeeJeeDR
Copy link

@memic84 it's pretty hidden in the documentation but you can actually use this as a "workaround":

const getColor = (colorVar, { opacityVariable, opacityValue }) => {
  if (opacityValue !== undefined) {
    return `rgba(var(${colorVar}), ${opacityValue})`;
  }
  if (opacityVariable !== undefined) {
    return `rgba(var(${colorVar}), var(${opacityVariable}, 1))`;
  }

  return `rgb(var(${colorVar}))`;
};

module.exports = {
  // ...
  theme: {
    extend: {
      colors: {
        primary: (params) => getColor('--color-primary-rgb', params),
      }
   }
}

I still would like this PR merged, because I think everyone would benefit from making this easier.

This doesn't seem to work in combination with @apply.

@stefan-schweiger
Copy link
Contributor Author

@PeeJeeDR works for me without a problem in my angular project, even VS Code intellisense picks it up with the tailwind extension.

@adamwathan
Copy link
Member

adamwathan commented Sep 1, 2021

Hey thanks for this! Any thoughts on how this could possibly work with both the old and new rgb/hsl formats?

--color-1: 12, 34, 56
rgb(var(--color-1))

--color-2: 12 34 56
rgb(var(--color-2)

Both of those are valid, and depending on the syntax used, the comma before the alpha channel needs to be omitted 🤔

@stefan-schweiger
Copy link
Contributor Author

@adamwathan good catch, I think there is no good way of handling this besides convention or maybe a config option which way you intend to use. Using JIT you could try to read the value of the var, but I don't know if the overhead is worth it.

@adamwathan
Copy link
Member

We're going to make some internal changes to use the new rgb/hsl format for all colors we generate internally, then let's update this to use that as well and just assume people are using space-separated syntax. Will target v3 which will be in alpha/beta in like a month, and released in November 👍🏻

@reinink
Copy link
Member

reinink commented Sep 23, 2021

Hey we've updated Tailwind internally to use the space separated syntax for all color related work that we do. Do you mind updating this PR to work based on that assumption? 🙏

@stefan-schweiger
Copy link
Contributor Author

@reinink sure I will try to look into it over the weekend 😉

@stefan-schweiger
Copy link
Contributor Author

stefan-schweiger commented Sep 24, 2021

@reinink @adamwathan I've just started looking into this and I was wondering if it's ok to be very specific with the expected format.

// this is going to work
rgb(var(--my-color-var)) // --my-color-var: 255 0 0;
hsl(var(--my-color-var)) // --my-color-var: 0 100% 50%;

// this is not going to work
rgb(var(--my-color-var)) // --my-color-var: 255, 0, 0;
hsl(var(--my-color-var)) // --my-color-var: 0, 100%, 50%;

rgb(var(--my-color-var)) // --my-color-var: 255 0 0 / 0.1;
hsl(var(--my-color-var)) // --my-color-var: 0 100% 50% / 0.1;

rgba(var(--my-color-var)) // --my-color-var: 255 0 0 0.1;
hsla(var(--my-color-var)) // --my-color-var: 0 100% 50% 0.1;

What do you think? Supporting the rgba syntax would in theory be possible, but then the set opacity level (e.g. bg-opacity-10) can still not be applied.

@stefan-schweiger
Copy link
Contributor Author

You can take a look if you are happy with my current changes. rgba/hsla values stay as is, but for rgb/hsl values not in the described format this could be a breaking change if anyone is currently using those as color values.

@adamwathan
Copy link
Member

Just following up on this (we're doing a big GitHub clean up) — my only concern is this one not working:

:root {
  --my-color-var: 255 0 0 / 0.1;
}

It feels bad that if someone wants to define colors that have the opacity baked in that everything would break.

Maybe this is okay though because if someone wanted to do that they wouldn't define the color as channels but rather as a full color?

:root {
  --my-color-var: rgb(255 0 0 / 0.1);
}

I kinda wonder though if trying to do this all automagically is just too much and instead we could just offer a helper function people could use? For example:

// tailwind.config.js
module.exports = {
  theme: {
    colors: ({ rgb, hsl }) => ({
      primary: rgb('--color-primary'),
      secondary: hsl('--color-secondary'),
      // ...
    })
  }
}

You'd need to know about those helpers of course but it feels a lot more mechanically simple to me, with less chances of discovering an edge case down the road that we've accidentally made impossible to solve without a breaking change.

@thecrypticace
Copy link
Contributor

I implemented Adam's suggestion above and added you as a co-author to the PR and have merged it in. You can read more about the solution in #7665 but the gist is:

  1. the variables must be defined using space separated syntax
  2. the variables must not provide an alpha channel as to be able to let Tailwind CSS handle this itself.

We appreciate all your work on this — It is very much appreciated!

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

Successfully merging this pull request may close these issues.

None yet

6 participants