-
Notifications
You must be signed in to change notification settings - Fork 21
That one color.a edge case #31
Comments
I'm not sure it would make sense to add this kind of conditional, it seems a bit niche use case, and can just be used as 1.0 instead of being ignored in most cases I'm aware of. Can you explain what's the use case here? |
I noticed that when i was working on a CSS emit, where the norm is to use
But this is not possible with alpha, so in that case I needed to emit either
Or dependant of the use case:
|
I still don't think there's anything here requiring a conditional. When using auto-generated code (like here), you want to reach for the most common and basic use-case. So in this case, emitting By the way, for Andorid they have ARGB (where the first two digits are the alpha). I see that in CSS there's RGBA (https://drafts.csswg.org/css-color/#hex-notation, see "8 digits"), but I'm not sure if it's supported in all browsers - if yes, this might be a nice addition to have here. WDYT ? |
you are correct. i think i guess it comes down to being stylistic too. i was trying to match Zeplins emit, with the thought that it will reduce mental load, when debugging in the browser and when some Zeplin output shows the same as the generated files. This is to prevent for the same thing to be added multiple times into the codebase with different syntaxes. This is what Zeplins CSS extension outputs:
Here is their conditional: https://github.com/zeplin/stylesheet-extensions/blob/6906b11c7e118f50f5fc41918d98099fe9ed85cd/packages/zeplin-extension-style-kit/values/color.js#L65 But as you were saying, in the end this comes down to a teams preference how they want it. While researching their extension and wondering why they do that special emit with alpha i had to find out, that including an alpha channel might incur a performance hit on older browsers: https://stackoverflow.com/a/54018451/3650874 |
The performance hit you're mentioning is negligible. Regardless, I think having I'll close this issue and open a tracking issue for that, if that's OK with you. Thanks! |
Now that you introduced inverting conditionals, the alpha component of the
color
token has come to my attention. In most cases, when it is1.0
, it can be ignored. Unfortunately if it is not, one might need a different emit. For examplecolor.rgb
vscolor.argb
.Do you think it would make sense to allow to use a conditional on
color.a
?In one possible implementation this could be
{{% IF color.a %}}
. This would match all but1.0
. Which i can see, might be confusing. As the value would cast to the opposite. Alternatively maybe{{% IF color.hasAlpha %}}
{{% IF color.isOpaque %}}
{{% IF color.a == 1.0 %}}
The latter probably being the most complicated to implement and just opening a completely different can of worms.
Sorry for bombarding you with issues.
The text was updated successfully, but these errors were encountered: