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

feat(Theming): add color palette #451

Merged
merged 1 commit into from
Dec 5, 2018
Merged

feat(Theming): add color palette #451

merged 1 commit into from
Dec 5, 2018

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Nov 8, 2018

Fixes #423.

@layershifter
Copy link
Member Author

This PR will be never merged. Using it to prove some ideas.

@codecov
Copy link

codecov bot commented Nov 8, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@f9a1806). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #451   +/-   ##
=========================================
  Coverage          ?   88.17%           
=========================================
  Files             ?       42           
  Lines             ?     1455           
  Branches          ?      212           
=========================================
  Hits              ?     1283           
  Misses            ?      167           
  Partials          ?        5
Impacted Files Coverage Δ
src/components/Divider/Divider.tsx 100% <ø> (ø)

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 f9a1806...526b020. Read the comment docs.

@levithomason levithomason added this to layershifter in Core Team Nov 16, 2018
CHANGELOG.md Outdated Show resolved Hide resolved

<Header as="h2">Text colors</Header>
<p>
Text variants are also separated as state color because in the most cases it's not
Copy link
Contributor

Choose a reason for hiding this comment

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

I am reading this as a client, and have a reasonable question: text color is largely influenced by the backgeround where text is used, but thjis paragraph suggests that there is only single text color suggested (yes, maybe with several tints, but still just a single color) - and this contradicts most approaches to color palette where text color is largely defined by the background - and, thus, on variants for text color (generally - content color, such as text, icons, etc) are suggested: onPrimary, onSecondary, etc. More than that, depending on the tint of the background different text color might be used, so that content will appear with enough visual contrast.

What is our suggestion here?

Copy link
Contributor

Choose a reason for hiding this comment

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

also, might suggest to rename it to something that is more generic than text, as this color generally applies not only to text content

@kuzhelov
Copy link
Contributor

kuzhelov commented Dec 3, 2018

one thing that I still missing, even after reading the guide, is what are the steps I should follow as a client to introduce color palette to my theme? What are the tools that will aid me in that? If it is not the purpose of this PR - please, let me know what are our steps planned to address that?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Core Team
  
layershifter
Development

Successfully merging this pull request may close these issues.

None yet

4 participants