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

Way to define contrast-color for a $theme-colors #31208

Closed
SvenBudak opened this issue Jun 30, 2020 · 7 comments
Closed

Way to define contrast-color for a $theme-colors #31208

SvenBudak opened this issue Jun 30, 2020 · 7 comments
Labels

Comments

@SvenBudak
Copy link

currently in bootstrap every contrast color is generated automatically. there is only "dark" and "light". this is a great development!

However, for some $theme-colors you might want to assign a fixed color value.

For example, I'm currently setting a $primary color: #30bc55 The contrast system detects here that $color-contrast-dark should be used. However, I would like to have #f2f2f2 as the contrast color. And that globally in the whole project.

It would be a great enrichment if this could be implemented before the stable version :)

@ffoodd
Copy link
Member

ffoodd commented Jun 30, 2020

The idea is nice, however the color you mentionned lead to an insufficient contrast ratio of 2.22:1.

For now, our function tries $color-contrast-light, $color-contrast-dark, $white and $black and returns the first one which have a sufficient contrast ratio.

If you want to pass with our colors, you may already set #f2f2f2 as $color-contrast-light and decrease $min-contrast-ratio to 2 and it would return your color. You might also use the fourth argument of color-contrast() when using it to set a custom contrast-ratio.

But again, this would lead to accessibility issues.

So I'm not sure about the need for specifying contrast color for each theme color, we already provide a few ways to customize this that are consistent.

@SvenBudak
Copy link
Author

Hmm as default i want to keep $min-contrast-ratio: 3; because it just makes sense to keep this global.

then i just add a class like in all my other older apps: .primary-contrast { color: #f2f2f2; } and add this class to all elements manually in html markup.

@ffoodd
Copy link
Member

ffoodd commented Jun 30, 2020

If you compile sources by yourself, you could add an exception in color-contrast() function— something like:

if ($background == $primary) {
  @return #f2f2f2;
}

Adding this before anything in the function would ensure you that any try to get the contrast color for your $primary would return the specific color you want.

I keep in mind the idea to specify a contrast color, but it kind of short-circuit the way we automate contrasts for now…

@SvenBudak
Copy link
Author

SvenBudak commented Jun 30, 2020

Thanks for your time and the solutions :)

I use the files from the node_modules folder and only overwrite the variables.

What I just noticed is that it might make sense to deactivate the outline in the reboot.scss with a variable?

Current code:

button:focus {
  outline: 1px dotted;
  outline: 5px auto -webkit-focus-ring-color;
}

Must be overwritten with:

button:focus {
    outline: none;
}

I am aware that for accessibility reasons, this should not be disabled. But I attach more importance to the visual appearance, as I'm sure many other people besides myself do.

Just a little hint :) Thanks again for your time! All other new features I find very good in bootstrap 5!

@ffoodd
Copy link
Member

ffoodd commented Jun 30, 2020

Bootstrap does its best to help with accessibility concerns, so we tend to improve accessibility than to decrease it.

For outline, I made a draft PR (which won't get merged) using :focus-visible to only show focus on keyboard navigation: #30872

You may try an approach like this, which would not harm accessibility.

@ffoodd ffoodd closed this as completed Jul 20, 2020
@micka-fdz
Copy link

Hi @ffoodd. Maybe we can create a new SASS map to override color contrast for specific colors. What do you think?

@ffoodd
Copy link
Member

ffoodd commented Apr 19, 2021

Not in our core, as explained we aim to provide accessible defaults, and providing accessibility opt out does not make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants