-
Notifications
You must be signed in to change notification settings - Fork 63
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 format adjustments #257
base: color-first-draft
Are you sure you want to change the base?
Changes from 6 commits
9fa91b1
f878152
af6374e
cee6683
a5a46a0
6954bb9
63051c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -29,7 +29,7 @@ For example, initially color tokens may be defined as such: | |||||||
|
||||||||
```json | ||||||||
{ | ||||||||
"Majestic magenta": { | ||||||||
"Hot pink": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"$hex": "#ff00ff" | ||||||||
|
@@ -52,7 +52,7 @@ Then, the output file may look something like: | |||||||
|
||||||||
```scss | ||||||||
// colors-hex.scss | ||||||||
$majestic-magenta: #ff00ff; | ||||||||
$hot-pink: #ff00ff; | ||||||||
$translucent-shadow: #00000080; | ||||||||
``` | ||||||||
|
||||||||
|
@@ -75,20 +75,20 @@ For example, initially color tokens may be defined as such: | |||||||
|
||||||||
```json | ||||||||
{ | ||||||||
"Majestic magenta": { | ||||||||
"Hot pink": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"$hex": "#c44587", | ||||||||
"$hex": "#ff00ff", | ||||||||
"$colorSpace": { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, and below, these should align with the current spec:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comment |
||||||||
"name": "srgb", | ||||||||
"$components": [196, 69, 135] | ||||||||
} | ||||||||
} | ||||||||
}, | ||||||||
"Simple sage": { | ||||||||
"Acid green": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"$hex": "#b4d8a7", | ||||||||
"$hex": "#00ff66", | ||||||||
"$colorSpace": { | ||||||||
"name": "srgb", | ||||||||
"$components": [180, 216, 167], | ||||||||
|
@@ -107,8 +107,8 @@ Then, the output from a tool’s conversion to RGBA may look something like: | |||||||
|
||||||||
```scss | ||||||||
// colors-rgba.scss | ||||||||
$majestic-magenta: rgba(196, 69, 135, 1); | ||||||||
$simple-sage: rgba(180, 216, 167, 0.75); | ||||||||
$hot-pink: rgba(255, 0, 255, 1); | ||||||||
$acid-green: rgba(0, 255, 102, 1); | ||||||||
``` | ||||||||
|
||||||||
</aside> | ||||||||
|
@@ -125,20 +125,20 @@ Formatted in H (hue), S (saturation), L (lightness) and an optional (A) alpha. H | |||||||
|
||||||||
```json | ||||||||
{ | ||||||||
"Majestic magenta": { | ||||||||
"Hot pink": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"$hex": "#c44587", | ||||||||
"$hex": "#ff00ff", | ||||||||
"$colorSpace": { | ||||||||
"name": "hsl", | ||||||||
"$components": [329, 0.52, 0.52] | ||||||||
} | ||||||||
} | ||||||||
}, | ||||||||
"Simple sage": { | ||||||||
"Acid green": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"$hex": "#b4d8a7", | ||||||||
"$hex": "#00ff66", | ||||||||
"$colorSpace": { | ||||||||
"name": "hsl", | ||||||||
"$components": [104, 0.39, 0.75], | ||||||||
|
@@ -157,8 +157,8 @@ Then, the output variables may look like: | |||||||
|
||||||||
```scss | ||||||||
// colors-hsl.scss | ||||||||
$majestic-magenta: hsl(329, 52%, 52%); | ||||||||
$simple-sage: hsl(104, 39%, 74%, 0.75); | ||||||||
$hot-pink: hsl(300, 100%, 50%); | ||||||||
$acid-green: hsl(144, 100%, 50%, 0.75); | ||||||||
``` | ||||||||
|
||||||||
</aside> | ||||||||
|
@@ -177,16 +177,16 @@ Hex8 uses two extra digits, known as the alpha value, to change the transparency | |||||||
|
||||||||
```json | ||||||||
{ | ||||||||
"Majestic magenta": { | ||||||||
"Hot pink": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"$hex": "#c4458780" | ||||||||
"$hex": "#ff00ff" | ||||||||
} | ||||||||
}, | ||||||||
"Simple sage": { | ||||||||
"Acid green": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"$hex": "#b4d8a780" | ||||||||
"$hex": "#00ff66" | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
@@ -200,12 +200,12 @@ Then, the output variables may look like: | |||||||
|
||||||||
```scss | ||||||||
// colors-hex.scss | ||||||||
$majestic-magenta: #c4458780; | ||||||||
$simple-sage: #b4d8a780; | ||||||||
$hot-pink: #ff00ff; | ||||||||
$acid-green: #00ff66; | ||||||||
|
||||||||
// colors-rgba.scss | ||||||||
$majestic-magenta: rgba(196, 69, 135, 0.5); | ||||||||
$simple-sage: rgba(180, 216, 167, 0.5); | ||||||||
$hot-pink: rgba(255, 0, 255, 1); | ||||||||
$acid-green: rgba(0, 255, 102, 1); | ||||||||
``` | ||||||||
|
||||||||
</aside> | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -423,7 +423,7 @@ Describes a gradient that is solid yellow for the first 2/3 and then fades to re | |||||||||
{ | ||||||||||
"brand-primary": { | ||||||||||
"$type": "color", | ||||||||||
"$value": "#99ff66" | ||||||||||
"$value": "#00ff66" | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can make this update in the interim. The color format editors have a different proposal for color gradient, and we should sync with format to align/make recommendations for community feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK! Yes breaking out changes to gradient as a separate PR would be easier to review! This PR shouldn’t introduce any changes to gradient. Because, yes, gradients do use colors, that doesn’t mean gradients change necessarily. Then we can review those unique changes easier |
||||||||||
}, | ||||||||||
|
||||||||||
"position-end": { | ||||||||||
|
@@ -453,7 +453,7 @@ Describes a gradient that is solid yellow for the first 2/3 and then fades to re | |||||||||
|
||||||||||
Describes a color token called "brand-primary", which is referenced as the mid-point of a gradient is black at either end. | ||||||||||
|
||||||||||
<div style="height: 2rem; background: linear-gradient(90deg, #000000, #99ff66, #000000);"></div> | ||||||||||
<div style="height: 2rem; background: linear-gradient(90deg, #000000, #00ff66, #000000);"></div> | ||||||||||
|
||||||||||
</aside> | ||||||||||
|
||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -12,21 +12,42 @@ If an explicit type is set, but the value does not match the expected syntax the | |||||||
|
||||||||
## Color | ||||||||
|
||||||||
Represents a 24bit RGB or 24+8bit RGBA color in the sRGB color space. The `$type` property MUST be set to the string `color`. The `$value` property MUST be a string containing a [hex triplet/quartet](https://www.w3.org/TR/css-color-4/#typedef-hex-color) including the preceding `#` character. To support other color spaces, such as HSL, translation tools SHOULD convert color tokens to the equivalent value as needed. | ||||||||
Represents a color in the UI. The `$type` property MUST be set to the string `color`. The `$value` property MUST be an object string containing a string `colorSpace`, a numeric `coordinates` array, optional numeric `alpha`, string `colorProfile`, and string `fallback` to support legacy color spaces as needed. | ||||||||
|
||||||||
| Key | Type | Required | Description | | ||||||||
| :------------- | :------------: | :------: | :--------------------------------------------------------------------------------------------------------------------------------------- | | ||||||||
| `colorSpace` | `string` | Y | An string representing the color space. | | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: we could probably link to all the valid values from CSS Color Module 4
Suggested change
|
||||||||
| `colorProfile` | `string` | | String representing url of color profile. | | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we are not using colorProfile, and just aligning with CSS Color Module 4?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In CSS Color Module 4 color terminology section, here's the definition of color space:
Color profile is different, it's about using a particular ICC color profile for device specific implementations (print, for example). For example, you can use the CMYK color space and choose more than one color profile to implement those colors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You’re 100% right, and yes I do see us needing this. As we talked about earlier today, perhaps we limit this initial PR to CSS Color Module 4 where:
Then, we open a separate PR to expand the format to include CSS Color Module 5’s concept of |
||||||||
| `coordinates` | `number array` | Y | An array of numeric values representing individual color coordinates. | | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we were leaning toward “channels” being the term. Also just in the interest of being really specific, “an array” doesn’t specify the length of the list. Calling it a “tuple of 3 values” means that the values must always be 3—no more, no less. Just like any color in CSS, I think we should enforce that the length is always 3, no matter what.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we were leaning towards that! The terms For example in the CSS Color Level 4 color type definition it reads:
The CSS Color Level 5 spec builds upon Level 4, yet the terminology is still interchangeable. In ColorJS, the term "coordinates" could be shorthand for component coordinate or channel coordinate if I assume correctly (need to confirm). Happy to move back to Also, I suggest we adjust wording to include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Oh smart! I like the idea of aligning with all that great thinking as much as possible There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, there have been interest in this support in the legacy color thread! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh also I think as I said in another comment, maybe we like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This should be an array of arbitrary length. Defining these as arrays now means that implementers are less likely to make assumptions that these will always have a length of 3. |
||||||||
| `alpha` | `number` | | An integer or floating-point value representing the numeric value. | | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should specify this MUST be in the range of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can do. Where should this sentence occur? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anywhere in the |
||||||||
| `fallback` | `string` | | Unit of distance. Supported values: [HEX 6](https://www.w3.org/TR/css-color-4/#typedef-hex-color) including the preceding `#` character. | | ||||||||
resource11 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
|
||||||||
For example, initially the color tokens MAY be defined as such: | ||||||||
|
||||||||
<aside class="example"> | ||||||||
|
||||||||
```json | ||||||||
resource11 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
{ | ||||||||
"Majestic magenta": { | ||||||||
"$value": "#ff00ff", | ||||||||
"$type": "color" | ||||||||
"Hot pink": { | ||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"colorSpace": "display-p3", | ||||||||
"colorProfile": "http://example.org/display-p3.icc", | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and below, the format should match the updated syntax of
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for this suggestion! I should clarify why the optional |
||||||||
"coordinates": [0.92, 0.2, 0.97], | ||||||||
"alpha": 1, | ||||||||
"fallback": "#ff00ff" // HEX 6 supported only | ||||||||
} | ||||||||
}, | ||||||||
"Translucent shadow": { | ||||||||
"$value": "#00000080", | ||||||||
"$type": "color" | ||||||||
"$value": "#000000", | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional to have two |
||||||||
"$type": "color", | ||||||||
"$value": { | ||||||||
"colorSpace": "display-p3", | ||||||||
"colorProfile": "http://example.org/display-p3.icc", | ||||||||
"coordinates": [0, 0, 0], | ||||||||
"alpha": 0.5, | ||||||||
"fallback": "#000000" // HEX 6 supported only | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
``` | ||||||||
|
@@ -39,12 +60,12 @@ Then, the output from a tool's conversion to HSL(A) MAY look something like: | |||||||
|
||||||||
```scss | ||||||||
// colors-hex.scss | ||||||||
$majestic-magenta: #ff00ff; | ||||||||
$hot-pink: #ff00ff; | ||||||||
$translucent-shadow: #00000080; | ||||||||
|
||||||||
// colors-hsl.scss | ||||||||
$majestic-magenta: hsl(300, 100%, 50%); | ||||||||
$translucent-shadow: hsla(300, 100%, 50%, 0.5); | ||||||||
$hot-pink: hsl(300, 100%, 50%); | ||||||||
$translucent-shadow: hsla(0, 0%, 0%, 0.5); | ||||||||
``` | ||||||||
|
||||||||
</aside> | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this file, we should just define it in the
srgb
space, and we don’t need thehex
fallback:Here and below they should follow the same format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! No changes have been added to this file yet. Once we get alignment on the rest, we can make adjustments throughout here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a suggestion: it is easier to review if this PR made everything consistent in all files, since that has to be done one way or another. Otherwise it ends up in a state of confusion not being able to tell where the proposal starts and where it ends. Would it be OK to just make everything a consistent format so it’s clear to reviewers?