-
Notifications
You must be signed in to change notification settings - Fork 4
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
Primary Color Customization #168
base: main
Are you sure you want to change the base?
Primary Color Customization #168
Conversation
…tailwind.config.js for global color styling
… and green-600 to brand-secondary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting APP_PRIMARY_COLOR
as an environment variable is confusing to me: we don't set any other configuration values via environment variable.
I could see this making sense if it was possible to set APP_PRIMARY_COLOR
within the .env
file but that doesn't seem to work. Please let me know if I'm missing something here, and also please add documentation to the README.
@aaronbrethorst You're right that setting the primary color via environment variable might seem inconsistent with our other configuration patterns. However, I've found that it does work when properly configured. It wasn't working earlier because, without the quotes, the color value was being interpreted as a comment, and without dotenv, the environment variables weren't being properly loaded into the Tailwind configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, but there are a few more changes that need to be done before this can be merged:
- Secondary brand color needs the same treatment as primary color
- Add examples of both to
.env.example
Done the changes. Ready to 🚀 :) |
PR Fixes #155
Changes Done:
Screenshots: