evidence-dev / evidence

Business intelligence as code: build fast, interactive data visualizations in pure SQL and markdown
https://evidence.dev
MIT License
3.44k stars 167 forks source link

Basic Theme Support #1517

Closed ItsMeBrianD closed 2 months ago

ItsMeBrianD commented 3 months ago

Description

This PR has a simple support for extending / overwriting the built-in evidence tailwind configuration

You do this my adding a tailwind.config.js file that contains your desired tailwind config.

image

There are still areas that are not impacted by the tailwind configuration, and there is a pass needed to rename colors (e.g. adding primary, secondary, success, etc.) but the scope of those changes is much larger than this initial basic support and will be included in a follow-up PR

Checklist

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: a474a9aa36fc2bc984eaab4100fe18725a25cbcc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | ----------------------------- | ----- | | @evidence-dev/core-components | Patch | | @evidence-dev/components | Patch | | @evidence-dev/evidence | Patch | | evidence-test-environment | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify[bot] commented 3 months ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit a474a9aa36fc2bc984eaab4100fe18725a25cbcc
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/65d526c62d69c50008e7c7f3
Deploy Preview https://deploy-preview-1517--evidence-development-workspace.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 20, 2024 10:26pm
ItsMeBrianD commented 3 months ago

That understanding is correct, we basically put the user's configuration as a preset that takes precedence over our own preset (@evidence-dev/tailwind), so there are a few things that they can't change (e.g. the content field is integrated with our plugins system so it can't be touched), but other than that they can define their own configuration

archiewood commented 3 months ago

LGTM

archiewood commented 3 months ago

And I guess the big PR that follows involves basically renaming all the colours to things like primary rather than blue across all our components, and in the default tailwind config

ItsMeBrianD commented 3 months ago

And I guess the big PR that follows involves basically renaming all the colours to things like primary rather than blue across all our components, and in the default tailwind config

Correct

mcrascal commented 3 months ago

AFAICT this doesn't work with @apply? I think we need to maintain support for it.

ItsMeBrianD commented 3 months ago

AFAICT this doesn't work with @apply? I think we need to maintain support for it.

@apply gets baked in, so unless it is baking in as CSS Variables we can't touch those values

ItsMeBrianD commented 3 months ago

Is there a way to do this that doesn't break @apply, or what would be our migration plan away from it?

I think we'd need to look at how @apply is baking the values in. If we can get it to bake in css variables rather than hard-coded colors then we should be okay. If not, then we won't be able to use this anymore for anything themeable.

There are a lot of places in the docs where the usage of @apply is discouraged though; so this may be a good change overall

hughess commented 3 months ago

@ItsMeBrianD I remember you mentioning a challenge to set hover colours as something relative to the primary colour. Not sure if that applies to this PR specifically or themes more generally, but this package @archiewood found might have a solution:

https://github.com/Qix-/color

It includes functions to manipulate colours, so we could potentially have some sort of darkening or lightening threshold for hover states. Though I'm unsure if that will cover your point that with some colours it's actually a shift in hue that's required vs. a change in lightness

archiewood commented 2 months ago

Needs docs eventually. Could be after we do all the renaming of the tokens