Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(Label): added black and white colors for the color prop #855

Merged
merged 15 commits into from
Feb 7, 2019

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Feb 6, 2019

This PR adds the black and white color options for the Label, in order to fix #739
Here is how all the variants for color look:
image

As the white and black are primitive colors, the color scheme for these colors is manually defined:

const primitiveColorsScheme = {
  black: {
    foreground: colors.white,
    border: colors.white,
    shadow: colors.white,
    background: colors.black,
  },
  white: {
    foreground: colors.black,
    border: colors.black,
    shadow: colors.black,
    background: colors.white,
  },
}

If there are any other ideas, please let me know.

BREAKING CHANGES (Teams theme)

The value for the siteVariables.colors.black was changed from the grey[900](#252424) variant to #000. If there are usages of this variable, replaced them in the following manner:

siteVariables.colors.black <=> siteVariables.colors.grey[900]

Whenever you want to use the black color (#000) use the variable siteVariables.colors.black.

@codecov
Copy link

codecov bot commented Feb 6, 2019

Codecov Report

Merging #855 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #855   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files          21       21           
  Lines         728      728           
  Branches       69       69           
=======================================
  Hits          681      681           
  Misses         47       47

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f431ed...bce973f. Read the comment docs.

CHANGELOG.md Outdated Show resolved Hide resolved
@codepretty
Copy link
Collaborator

Are you going to update the references to the old #252424 color of black as well? For example in buttonVariables.ts.

@mnajdova
Copy link
Contributor Author

mnajdova commented Feb 6, 2019

@codepretty as far as I see in buttonVariables.ts we are using siteVariables.black, which still have the same value as before. I intentionally didn't change the value of that one as it is widely used, and I would introduce lots of breaking changes. If #000 should be used anywhere, we can update the styles of the components one by one. Let me know if I misunderstood your question.

@codepretty
Copy link
Collaborator

codepretty commented Feb 6, 2019

@codepretty as far as I see in buttonVariables we are using siteVariables.black, which still have the same value as before. I intentionally didn't change the value of that one as it is widely used, and I would introduce lots of breaking changes. If #000 should be used anywhere, we can update the styles of the components one by one. Let me know if I misunderstood your question.

ah, i see. i missed the update to siteVariables you did to reference colors.grey[900]. do you think it will be confusing to devs using this to know that siteVars.black is different from colors.black without digging in?

@mnajdova
Copy link
Contributor Author

mnajdova commented Feb 6, 2019

Yep, totally agree with the confusion, but will address this in a separate PR because I want this one to be simple. The basic idea I have in mind is that we should remove all colors we have in siteVariables, and use only the ones from the colors object. We will map all current color in the siteVariables with the corresponding matching from the colors object. As you may guess this will be a big change, and we need to keep in mind to create colors and scheme for the other themes as well (dark and HOC), so that theme switching would work as expected.

@mnajdova mnajdova merged commit de45998 into master Feb 7, 2019
@layershifter layershifter deleted the feat/color-palette-black-color branch February 7, 2019 16:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Color Scheme for black color
4 participants