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

fix(titlesColor): Fixed utilities' primary color #872

Merged
merged 17 commits into from
Nov 23, 2022

Conversation

louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Oct 27, 2021

Fix #815: Color of text utilities from #ff7900 to #f16e00 on white backgrounds

Netlify : https://deploy-preview-872--boosted.netlify.app/docs/5.2/utilities/colors/

scss/_root.scss Outdated Show resolved Hide resolved
@louismaximepiton louismaximepiton force-pushed the main-louismaximepiton-titles-color branch from 2d401a7 to 908434e Compare April 28, 2022 15:27
@louismaximepiton louismaximepiton changed the title fix(titlesColor): Fixed title's color fix(titlesColor): Fixed utilities' primary color Apr 28, 2022
@julien-deramond julien-deramond force-pushed the main-louismaximepiton-titles-color branch from 908434e to 78872fb Compare May 3, 2022 05:38
@julien-deramond

This comment was marked as resolved.

@netlify
Copy link

netlify bot commented Jun 30, 2022

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 2eb3794
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/637dfab647bb1a00081cd8f8
😎 Deploy Preview https://deploy-preview-872--boosted.netlify.app/docs/5.2/utilities/colors
📱 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 settings.

@julien-deramond julien-deramond added v5 and removed v5 labels Jun 30, 2022
@julien-deramond
Copy link
Member

In Helpers > Colored links we have orange on black bg. So the orange should be "Brand orange" instead of "Accessible orange" IMO.

@julien-deramond
Copy link
Member

At some point (maybe not in this PR) we could maybe say in Utilities > Color that .text-primary should only be used in combination with large texts (source: https://system.design.orange.com/0c1af118d/p/7059a5-colour/b/86fdf5)

@julien-deramond
Copy link
Member

While I'm checking for .text-primary, maybe we can drop .bg-transparent from:

<span class="d-inline-flex mb-3 display-0">B<span class="text-primary bg-transparent">oo</span>sted</span>

@louismaximepiton
Copy link
Member Author

In Helpers > Colored links we have orange on black bg. So the orange should be "Brand orange" instead of "Accessible orange" IMO.

I'd think so too, but I feel actually really weird using them with an incoming background. If the background is set by the user (using bg-* utilities) it would turn into brand Orange. IMO, the colored links are the thing to change in here but I might be wrong. Furthermore, Bootstrap isn't defining background for those. I'll try to dig a bit more to know where it comes from.

At some point (maybe not in this PR) we could maybe say in Utilities > Color that .text-primary should only be used in combination with large texts (source: https://system.design.orange.com/0c1af118d/p/7059a5-colour/b/86fdf5)

Would be great for accessibility !

Signed-off-by: louismaximepiton <louismaxime.piton@orange.com>
Signed-off-by: louismaximepiton <louismaxime.piton@orange.com>
Signed-off-by: louismaximepiton <louismaxime.piton@orange.com>
@louismaximepiton louismaximepiton force-pushed the main-louismaximepiton-titles-color branch from b4dba7b to 44ef23b Compare August 30, 2022 08:34
scss/_root.scss Outdated Show resolved Hide resolved
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

I've spotted an issue with our spinners that use text-primary to define the color so our orange spinners now use #f16e00 instead of #ff7900 (DSM guidelines reference)

@julien-deramond
Copy link
Member

@louismaximepiton
Copy link
Member Author

Returns from the spec review: the spinners should be $accessible-orange on white and $brand-orange on black.

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Just before merging, discovered that https://deploy-preview-872--boosted.netlify.app/docs/5.2/utilities/colors/#opacity doesn't work anymore

scss/_root.scss Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Nov 23, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@julien-deramond julien-deramond merged commit e918550 into main Nov 23, 2022
@julien-deramond julien-deramond deleted the main-louismaximepiton-titles-color branch November 23, 2022 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The orange colour used for the text samples on white background should be #F16E00 instead of #FF7900
4 participants