Fix color object closures with gradients

[innocenzi]

This PR applies two fixes, both related to #1676.

Color object flatening

#1676 actually won't work at all (my bad 😢) until this is fixed, because lodash's isObject will return true if it is given a function. So, the condition had to be changed according to this information.

New gradient stop plugin

This one won't just not work, but will crash if it encounters a closure. This is because the CSS is given the actual closure, so PostCSS will complain. Like, it will complain by crashing.

To fix that, the value variable needs to be checked. If it is a function, it needs to be executed. But that function will take an object with an opacityVariable key as a parameter, and I thought it would be confusing to give it an empty value.

So I also introduced new opacity variables. I'm not sure how useful it can be, nor if it should be only one variable, though.

• For .from-<color> utilities, it is given --gradient-from-opacity
• For .via-<color> utilities, it is given --gradient-via-opacity
• For .to-<color> utilities, it is given --gradient-to-opacity

The variable name has been copied from the ones you used already, like --gradient-from-color.

[adamwathan]

Have to figure out the best way to handle the transparentTo variable for this case... It should default to whatever the current color is but fully transparent. We can't really pass through the opacity variable name because we actually want to explicitly pass 0, but var(0) isn't valid CSS.

If the user's color is configured as rgba(var(--primary), var(--gradient-from-opacity)) how can we convert that to rgba(var(--primary), 0) in a bullet proof way?

[adamwathan]

Can you think of any downside to passing the whole var(--text-opacity, 1) block? Or on the flip side any nice benefits of it? If this is going to be a new public API it's very important we have no regrets about it down the road because I take breaking changes pretty seriously in Tailwind :(

Currently regretting not passing in the whole alpha value as that would have let us just pass in 0 for this transparent case without issues.

[innocenzi]

It should default to whatever the current color is but fully transparent.

I don't really understand this. I mean, I'd prefer to use the user's color instead of an arbitrary one for the transparent color too... but whatever value is given from the user won't change the result, right? It's going to be transparent, and the user has no way to change the opacity anyway? So, is it really a big deal?

Either way, I can think of a solution to fix that. How about giving a new parameter, opacityValue, in addition to opacityVariable?

return value({
  opacityVariable: `--gradient-${type}-opacity`,
  opacityValue
})

If the user's color is configured as rgba(var(--primary), var(--gradient-from-opacity)) how can we convert that to rgba(var(--primary), 0) in a bullet proof way?

The user should now configure it that way:

primary: ({ opacityVariable, opacityValue }) => `rgba(var(--primary), var(${opacityVariable}, ${opacityValue}))`

That would require a change in every place where withAlphaVariant is used, too.

---

And for what it's worth to you, I still think it was better to give only the variable name. I think it's more flexible that way. 👍

[adamwathan]

The problem is that this would generate the wrong output:

primary: ({ opacityVariable, opacityValue }) => `rgba(var(--primary), var(${opacityVariable}, ${opacityValue}))`

Check out the actual CSS for one of the from classes:

.from-red-500 {
  --gradient-from-color: #f56565;
  --gradient-color-stops: var(--gradient-from-color), var(--gradient-to-color, rgba(245, 101, 101, 0));
}

The transparentTo value does not use the same opacity as the actual color, it should always be zero.

So the only way I can think to fix this is to pass opacityVariable or opacityValue, and the user will have to conditionally check which one is provided and react accordingly:

primary: ({ opacityVariable, opacityValue }) => {
  if (opacityValue !== undefined) {
    return `rgba(var(--primary), ${opacityValue})`
  }

  return `rgba(var(--primary), var(${opacityVariable}, 1))`
}

[adamwathan]

I made some changes and merged, which you can see here:

a7e19ca

The summary is:

• We now pass through opacityValue or opacityVariable or nothing — the user needs to verify the value they want to use is actually defined before using it
• opacityValue is passed through explicitly as 0 for the transparency situation, and users of this API should prefer opacityValue to opacityVariable if it is provided
• I've removed the --gradient-from-opacity and similar variables since they aren't currently used. If we do add support for gradient opacity (wouldn't be surprised if we do) we will bring this back 👍 Just no sense having dead code in the framework for no reason.

Thanks again for your help getting this working, going to publish this as a patch release as long as you don't see any remaining issues.

[innocenzi]

I'm fine with your changes, it's a bit more complicated that what I was imagining, but the functionality is there so not gonna complain 👍

Thank you for your time!

[adamwathan]

The problem is we'd have to invoke it like:

if (_.isFunction(color)) {
  return color({ opacityVariable: '--madeUpVariableThatDoesntExist', opacityValue: 0 })
}

...because there is no variable for --gradient-transparent-opacity you know? Or we'd have to create a variable for that and make sure we always set it to 0, which could be an okay solution as well but feels sort of weird.

[innocenzi]

Yes, I edited out when I finally understood. I'm a bit slow. Thanks again 😅