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

[OUDS] Add "Colors" tokens, utilities and documentation #2802

Draft
wants to merge 11 commits into
base: ouds/main
Choose a base branch
from

Conversation

louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Nov 22, 2024

Note: Please transform - [ ] into - (NA) in the description when things are not applicable

Related issues

NA

Description

Remaining tasks and questions

Questions:

  • Should we add some more Bootstrap examples ? Yep probably 28/11/2024.
  • Should the changes we didn't make inside other tokens PR be inside a new branch ? Yes 28/11/2024, see [OUDS] Docs: Change the classes that were missing #2806.
  • Should we change the pre color (syntax highlight) inside this PR or postpone it ?

Tasks:

  • Add test for colors
  • Add and check documentation
  • Check all TODO LM
  • Check color utilities
  • Check dark mode
  • Update bundlewatch
  • Check color for every basic element
  • Check generated CSS in details
  • Add migration guides
  • Add OUDS mod
  • Map correctly the Bootstrap variables with the OUDS ones. (terrifying)
  • Check for doc rendering
  • Check for syntax highlighter
  • Check for tarteaucitron rendering
  • Check basic elements colors with other technologies and designers

Done list

The following was done in the PR:

  • Removed _color-palette.scss.
  • Added a _config.scss to prepare the arrival of tokens/_semantic-colors-custom-props.scss generated by Tokenator.
  • Replaced --#{$prefix}focus-visible-inner-color and --#{$prefix}focus-visible-outer-color by --#{$prefix}color-border-focus-inset and --#{$prefix}color-border-focus.
  • Replaced --#{$prefix}border-color-subtle by --#{$prefix}color-border-emphasized and --#{$prefix}disabled-color by --#{$prefix}color-content-disabled (might be changed in the future) and --#{$prefix}tertiary-active-bg by --#{$prefix}color-action-primary-pressed.
  • Removed all the CSS var from Boosted since they will likely be replaced by the tokens/_semantic-colors-custom-props.scss ones.
  • Removed many Sass Variables introduced by Boosted to handle text or bg utilities. Replaced their call by either raw or semantic tokens depending on the context.
  • Set some Sass variables for basic HTML elements such as body text and body bg or code etc...
  • Add a new import for tokens/_semantic-colors-custom-props.scss.
  • Changed the test accordingly to the new colors.
  • Changed the colors in the components and composite files.
  • Uncommented some raw tokens to be able to map them to Bootstrap ones.
  • Changed many things that we forgot to change in the previous PRs since this one will certainly be the last one on the tokens.
  • Added the Bootstrap example to see how it fits with $enable-bootstrap-compatibility.
  • Updated colors.yml, grays.yml, palette.yml.
  • Created a first mapping of the Bootstrap variables using ours.

To be done after the PR is merged

Motivation & Context

Types of change

  • New feature (non-breaking change which adds functionality)

Live previews

@louismaximepiton louismaximepiton added feature docs Improvements or additions to documentation css labels Nov 22, 2024
@louismaximepiton louismaximepiton added this to the OUDS milestone Nov 22, 2024
@louismaximepiton louismaximepiton marked this pull request as draft November 22, 2024 09:45
@louismaximepiton louismaximepiton force-pushed the ouds/main-lmp-tokens-colors branch 5 times, most recently from 2872936 to 7f97176 Compare November 27, 2024 16:57
Base automatically changed from ouds/main-lmp-tokens-font to ouds/main November 28, 2024 14:13
Copy link

netlify bot commented Nov 28, 2024

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 0fc5dd4
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/676d2cdf3dbde60008fbc72f
😎 Deploy Preview https://deploy-preview-2802--boosted.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@louismaximepiton louismaximepiton force-pushed the ouds/main-lmp-tokens-colors branch from 8d41cf0 to 11e7727 Compare November 29, 2024 14:10
@@ -6,7 +6,7 @@
display: flex; // OUDS mod
flex-direction: column; // OUDS mod
margin: 0 ($bd-gutter-x * -.5) 1rem;
border: solid var(--bs-border-color-subtle); // OUDS mod: instead of `var(--bs-border-color)`
border: solid var(--bs-color-border-emphasized); // OUDS mod: instead of `var(--bs-border-color)`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have replaced border-color-subtle by border-color-default I think, it's much in the subtle spirit than emphasized.

site/content/docs/0.0/utilities/api.md Show resolved Hide resolved
{{< callout danger >}}
Some of the colors below do not belong to the Orange Unified Design System specifications.
<!-- TODO LM: Decide how to link to the new DSM -->
Please refer to our [color palette section](#palette)<!-- and to the [Color guidelines](https://unified-design-system.orange.com/472794e18/p/217ac6-colour/b/4705d3) on the Orange Unified Design System website-->.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should link to this page : https://unified-design-system.orange.com/472794e18/p/92b911-colour/b/886430 ? Not sure since it seems to contain colors we don't have through tokens :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we will see when it'll be uncommented, but for now the page is kinda of desynchronized with the actual colors, I've set another for now: https://unified-design-system.orange.com/472794e18/p/217ac6-color


<h3>Example</h3>

Here's how you should not use these in your Sass:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it dangerous to show what we should not do. Developer may only see the code and don't read this sentence.
Maybe we should have comments directly in the following code, together with the good way to do it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried something in the last commit


### Sass maps

<!-- TODO LM
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you want to do with this commented content ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not quite sure how to handle it properly, maybe we should wait for the Bootstrap compatibility to uncomment it.

@@ -317,6 +298,7 @@ $cyans: (
) !default;
// fusv-enable

// TODO LM: take care of the rest
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does that mean ? Should it be done in this PR ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe at some point when we will tackle the bootstrap compatibility, so it might happen on a PR based on this one.

@hannahiss hannahiss mentioned this pull request Dec 20, 2024
47 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css docs Improvements or additions to documentation feature
Projects
Status: Triage
Status: Need Dev Review
Development

Successfully merging this pull request may close these issues.

3 participants