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

v5: Update colors to add shades and tints #29348

Merged
merged 2 commits into from
Oct 18, 2019
Merged

v5: Update colors to add shades and tints #29348

merged 2 commits into from
Oct 18, 2019

Conversation

mdo
Copy link
Member

@mdo mdo commented Aug 31, 2019

Fresh pass at this to replace #28268. This PR is entirely aimed at providing additional variables, and not any more classes (save for perhaps light background-colors and colors in another PR). I truly wish we could generate the variables another way, but for now, this is a huge boost to customizability purely by color.

  • Dynamic variables aren't possible in Sass, so this manually adds nine variations of each color.
  • Adds two new functions to shade (lighten) and tint (darken) each color.
  • From there, this documents the changes with docs specific classes to help communicate that these classes aren't part of Bootstrap proper.

/cc @twbs/css-review

Preview: https://deploy-preview-29348--twbs-bootstrap.netlify.com/docs/4.3/getting-started/theming/#all-colors

@XhmikosR

This comment has been minimized.

scss/_functions.scss Outdated Show resolved Hide resolved
@joneff
Copy link
Contributor

joneff commented Sep 11, 2019

Depending on how complicated you want it to be, you could go for material's palette and theming approach:

  • there is a _palette.scss file that contains both predefined colors and color maps with contrasting colors
  • in addition, there is _themeing.scss file that deals with the mundane tasks, like color matching, retrieving colors etc.

Trough some heavy lifting, at the end, there is a "theme" palette that consists of primary, secondary sub palettes (containing base, darker, lighter and contrast colors), complimentary sub palette (colors for text, body / application background etc) and a warn color.

White the approach is definitely not for everyone, it actually makes sense to have a "source of truth" palette which you query, when you need a theme color. Of course, should one want to deviate, the rest of the colors are there.

Or to put in some sort of code:

// Not exactcly code
@function make-palette($base, $darker: null, $ligther: null, $contrast-values: () ) {
    // if darker is not provided use shade($base) --> less way shade
    // if ligther is not provided use tint($base) --> less way tint
    // if contrast-values are not provided use contrast function
    // some code
    @return ( $default, $darker, $lighter, $default-contrast, $darker-contrast, $lighter-contrast);
}

// some transformations ...

$theme: (
    "primary": (
        default: ... , // use for, say link text color or button background color
        darker: ... , // use for, say link hover text color or button hover background color
        lighter: ... ,
        default-contrast: ... , // use for, say button text color
        darker-contrast: ... , // use for, say button hover text color
        lighter-contrast: ...
    ),
    "secondary": (
        default: ... , // use for, say secondary button background color
        darker: ... , // use for, say secondary button hover background color
        lighter: ... ,
        default-contrast: ... , // use for, say secondary button text color
        darker-contrast: ... , // use for, say secondary button hover text color
        lighter-contrast: ...
    ),
    "complimentary": (
        body-bg: ... ,
        body-text: ... ,
        component-bg: ... ,
        component-text: ... 
        // more variables probably 
    )
);

If you are trying to create a theme style guide, using the palette would be quite efficient. In addition, having the ability to manually specify colors for the theme palette makes it possible to fulfill designers request, because some times plain white and plain black are not what they want for contrasting color.

@joneff
Copy link
Contributor

joneff commented Sep 12, 2019

@mdo, how are you going to address projects that already have tint and shade defined and use it in the less way? Per se, we already have them defined and we use the less way. Many of our customers use our product in conjuction with bootstrap. So what would happen should they include both ours and bootstrap's functions?

@MartijnCuppens
Copy link
Member

MartijnCuppens commented Sep 12, 2019

I could see why we could add this, however I think a lot of developers will use Sass' built-in darken() or lighten() functions to get a tinted color. There will also be cases where for example $blue is changed to a darker blue and the darkened colors will be a little redundant.

I'm not 100% against this, but I wanna make sure we don't add too much we don't use ourselves.

@mdo
Copy link
Member Author

mdo commented Sep 27, 2019

Just pushed a few updates—sorry for the confusion, folks.

I've fixed the language used for all the tints and shades: tint() is now for lighter colors and shade() is now for darker colors. We're not using the lighten() and darken() functions because those do not accomplish what we want here—those functions only adjust the color's lightness level. What we really want is a blend of our specified color with white or black, which can be done with mix().

See this CodePen example for how each function generates different outputs.

I've added details around this to the documentation so that folks understand the reasons for our custom functions. Can probably make some more improvements here, but this should definitely help explain some key details around our color system.

Some additional reading and resources I found:

@mdo mdo marked this pull request as ready for review September 27, 2019 02:28
@mdo mdo requested a review from a team as a code owner September 27, 2019 02:28
@MartijnCuppens
Copy link
Member

LGTM now, the only thing left to tackle is finding a way to bypass fusv. Probably we'll need to implement something like // fusv-enable and // fusv-disable comments upstream?

@XhmikosR
Copy link
Member

BTW this needs a rebase and cleanup of the patches.

As for fusv, PRs welcome as usual :)

@XhmikosR
Copy link
Member

XhmikosR commented Oct 8, 2019

I cleaned up the branch, fixed tests and used a range.

I still feel this has a lot of duplication. Can't we use loops in Sass?

EDIT: nvm this doesn't work.

@mdo mdo mentioned this pull request Oct 18, 2019
9 tasks
@mdo mdo merged commit 943bef2 into master Oct 18, 2019
@mdo mdo deleted the v5-colors-shades-tints branch October 18, 2019 18:04
lucanos pushed a commit to lucanos/bootstrap that referenced this pull request Oct 27, 2019
* Add variables for shades and tints of each major color

* getting-started/theming.md: use a `range`.
MartijnCuppens added a commit that referenced this pull request Feb 15, 2020
Not applicable anymore since #29348
MartijnCuppens added a commit that referenced this pull request Feb 15, 2020
Not applicable anymore since #29348
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
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.

6 participants