[OUDS] Add text color utilities to the Colors tokens PR #2830
Conversation
louismaximepiton
force-pushed
the
ouds/main-lmp-tokens-colors-border-utilities
branch
from
10b3d2a to
741f161
Compare
louismaximepiton
force-pushed
the
ouds/main-lmp-tokens-colors-text-utilities
branch
from
2ca8c15 to
d44a18d
Compare
louismaximepiton
changed the base branch from
ouds/main-lmp-tokens-colors-border-utilities
to
ouds/main-lmp-tokens-colors-bg-utilities
louismaximepiton
force-pushed
the
ouds/main-lmp-tokens-colors-text-utilities
branch
2 times, most recently
from
e0ad436 to
b3965bc
Compare
louismaximepiton
force-pushed
the
ouds/main-lmp-tokens-colors-text-utilities
branch
from
b3965bc to
2c8b907
Compare
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
louismaximepiton
force-pushed
the
ouds/main-lmp-tokens-colors-text-utilities
branch
from
2c8b907 to
a11c9b3
Compare
louismaximepiton
force-pushed
the
ouds/main-lmp-tokens-colors-text-utilities
branch
5 times, most recently
from
6b27bac to
77424c7
Compare
louismaximepiton
force-pushed
the
ouds/main-lmp-tokens-colors-text-utilities
branch
from
77424c7 to
da1f218
Compare
| To be sure to respect the specifications, it is necessary to define `color`, `background-color` and `font-size` altogether. | ||
|
|
||
| Thus, the `.text-brand-primary` color on light background (`#f15e00`) should only be used in a font size greater than 24px (using for example `.fs-ds` utility), or 19px bold (using for example `.fs-hm` and `.fw-bold` utilities). | ||
| The `.text-primary` color on dark background (`#ff7900`) can be used in any size, and it shouldn't be used on light grey backgrounds at all. |
There was a problem hiding this comment.
text-primary doesn't exist in OUDS. It should be text-brand-primary I think. This color can be used on any background color If I remember well.
| - **Normal texts** are the remaining texts references. | ||
| {{< /callout >}} | ||
|
|
||
| In the next table we assume that the background is used in a color mode switching context and that the associated theme is good, so we have to consider the light and dark mode for the text color. |
There was a problem hiding this comment.
next should be replaced by following here. But I find the whole sentence a bit complicated. Maybe we should just say that the table take into account the light/dark mode switch with good use of bs-theme on container ? (If it's what you mean)
| In the next table we assume that the background is used in a color mode switching context and that the associated theme is good, so we have to consider the light and dark mode for the text color. | ||
|
|
||
| {{< bs-table >}} | ||
| | Color utility | Allowed `.bg-*` utility for normal text | Allowed `.bg-*` utility for large text | |
There was a problem hiding this comment.
To avoid repetition of colors between normal text and large text, we may change the title of the second column for "Allowed .bg-* utility for normal and large text". So any color in this column is implicitely ok on large and we have on the third column only what change in regard of the second column.
There was a problem hiding this comment.
This might need to change also the title of the third column for something like "Allowed .bg-* utility for large text only".
Not sure if it's clear enough tho.
|
|
||
| <!-- ## Opacity --> | ||
|
|
||
| ## Specificity |
There was a problem hiding this comment.
I'm not sure to understand this section. Is it linked to the !important on text helpers ?
| Most `color` utilities are generated by our theme colors, reassigned from our generic color palette variables. | ||
|
|
||
| <!-- Boosted mod --> | ||
| Boosted supersedes Bootstrap color variables with Orange brand color. |
There was a problem hiding this comment.
Reference to Boosted should be renamed
|
|
||
| Most `color` utilities are generated by our theme colors, reassigned from our generic color palette variables. | ||
|
|
||
| <!-- Boosted mod --> |
There was a problem hiding this comment.
Reference to Boosted should be changed
|
|
||
| {< scss-docs name="brand-colors" file="scss/_variables.scss" >}} | ||
|
|
||
| {< scss-docs name="brand-colors-dark" file="scss/_variables-dark.scss" >}} |
There was a problem hiding this comment.
brand-colors and brand-colors-dark does not seems to work on the page deployed on netlify
| {{< /callout >}} | ||
|
|
||
| {{< callout info >}} | ||
| **Heads up!** `.link-body-emphasis` is currently the only colored link that adapts to color modes. It's treated as a special case until v6 arrives and we can more thoroughly rebuild our theme colors for color modes. Until then, it's a unique, high-contrast link color with custom `:hover` and `:focus` styles. However, it still responds to the new link utilities. |
There was a problem hiding this comment.
should we specify that we talk about the v6 of bootstrap. Since we don't share the same version number, It may be v2 or v3 for us. We are in a bootstrap compatibility section so I don't know if it's obvious or not.
| <!--```html | ||
| <p class="text-danger"> | ||
| ```html | ||
| <p class="text-status-negative"> |
There was a problem hiding this comment.
I find this example a bit weird. If I use a screen reader, I can't see the color red. And If I don't use a screen reader the text is not displayed, so why would we make it red ! Furthermore we don't have the right to use this helper class for text, only for icon :)
|
|
||
| li li li::marker { color: var(--#{$prefix}tertiary-color); } | ||
| li li li::marker { color: var(--#{$prefix}color-content-disabled); } |
There was a problem hiding this comment.
I wanted to add this requirement in Figma for the bullet list component but we don't have the figma file yet. Should we add a "todo" comment here to not forget to talk about list in list with designer ?
| @@ -16,6 +16,7 @@ | |||
| } | |||
|
|
|||
| // OUDS mod | |||
| // TODO LM: Decide if we keep the dark mode or not | |||
There was a problem hiding this comment.
Current result on netlify seems good to me. What would it imply not to keep it ?
| @@ -141,15 +141,14 @@ The `<hr>` element has been simplified. Similar to browser defaults, `<hr>`s are | |||
| {{< example >}} | |||
| <hr> | |||
|
|
|||
| <div class="text-status-positive"> | |||
There was a problem hiding this comment.
Should we keep this color as an example since it's not allowed to have green border ?
The sentence on line 139 talks about a 0.25 opacity. That's not true in OUDS
Note: Please transform
- [ ]into- (NA)in the description when things are not applicableRelated issues
NA
Description
Remaining tasks and questions
Tasks:
Done list
The following was done in the PR:
.text-color occurrences in the documentationTo be done after the PR is merged
Motivation & Context
Types of change
Live previews