Skip to content

Conversation

@samselikoff
Copy link
Contributor

@samselikoff samselikoff commented Jan 21, 2019

This implementation has some debt. We can decide if we want the feature now or want to figure out a cleaner solution now.

If we could just use CSS Custom Properties, this would be super clean as the Tailwind config var would just be --brand-primary. However, there's not full browser support, so I wanted to provide a fallback. The fallback is to define a vanilla color: (or background-color or border-color) property before the variable use.

I used a Tailwind Component to insulate the rest of the codebase from this knowledge, that way we can continue to use docs-color-brand and have it work everywhere.

I had to use !important because components come before utilities, so a docs-text-grey-dark was overriding the brand color. If/when we get the router service in to improve the docs links in the header/nav components, this can be dropped.

@samselikoff
Copy link
Contributor Author

samselikoff commented Jan 21, 2019

@samselikoff
Copy link
Contributor Author

This debt can easily be refactored so I’m going to go ahead and merge now so I can use this on mirage 😁

@samselikoff samselikoff merged commit 491dc81 into master Jan 22, 2019
@samselikoff
Copy link
Contributor Author

Docs here 452394a

@samselikoff samselikoff deleted the use-css-var-for-brand-color branch January 22, 2019 04:39
@samselikoff samselikoff mentioned this pull request Jan 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants