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

prop to describe color theme #26

Closed
emplums opened this issue May 21, 2018 · 3 comments
Closed

prop to describe color theme #26

emplums opened this issue May 21, 2018 · 3 comments

Comments

@emplums
Copy link

emplums commented May 21, 2018

We need to come up with a name for a prop that describes a color theme (background color + text color, etc)... theme could easily be mixed up with our themes (light/dark). Maybe colorPattern? colorScheme ? maybe theme is ok? thoughts?

@shawnbot
Copy link
Contributor

I'm wondering if we can get away with bg for some components, since that's clearly what it drives. I could see how it might be confusing, though, to be able to use bg="blue.1" for some components and bg="blue" for others, though.

What about even just color, which might map to a pairing of bg and text props for some components?

@shawnbot shawnbot mentioned this issue May 21, 2018
4 tasks
@broccolini broccolini moved this from 🙇‍♀️ WIP to To do v0.0.1-beta in Primer Components release tracking 📋 May 22, 2018
@broccolini broccolini moved this from To do v0.0.1-beta to Backlog in Primer Components release tracking 📋 May 22, 2018
@broccolini
Copy link
Member

Here's a bunch of thoughts I have on this....

Currently styled-system uses bg for background color, and color for foreground. I prefer color over text since it can apply to more than just text (I know that's what we use now but I think this is a good chance to move away from it). It also matches the CSS property.

I think we should not use prop names that match utility props because that's confusing, and because often because we're applying a combination of colors.

Not sure how I feel about this yet but styled-system uses colors since it's applying multiple colors (see docs). I'm a tad worried folks would typo or mis-read, however it is easy to understand what it is. Theme works, however I agree that theme could be confusing and we probably want to reserve it for light/dark. colorScheme is not bad, though I'd prefer we keep prop names succinct so was considering if scheme is clear enough?

We can customize props in styled-system and use fg for foreground to avoid confusion with colors, however that's not the CSS property 😩. I know I know, neither is bg but it's such a common shorthand and at least uses one of the words from the CSS property 🙃.

Overall I prefer colors or scheme. As long as we don't use the same name as utility props like bg, I think we should just try something and get feedback when we have enough to test with - we can iterate :)

@shawnbot
Copy link
Contributor

Agreed: scheme it is!

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

No branches or pull requests

4 participants