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

Replaced colors with config colors #64

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

danhab99
Copy link

I found it kind of weird that components came with default colors that require you to edit the theme itself. Tailwindcss provides a system for creating a color pallet and I think windmill should take advantage of that. I've taken the liberty of replacing every single color in the default theme with a discriptivly named color to be later resolved in tailwind.config.js.

Here's my proposal:

Config color Replaces
wm-primary purple
wm-success green
wm-danger red
wm-neutral gray
wm-warning yellow
wm-info blue
wm-darkText white
wm-darkWarning orange

This would require users to create a tailwind.config.js resembling something like this:

const windmill = require("@windmill/react-ui/config");
const colors = require("tailwindcss/colors");

module.exports = windmill({
  purge: [],
  theme: {
    extend: {
      colors: {
        "wm-primary": colors.pink, // or whatever you want.. I like pink
        "wm-success": colors.green,
        "wm-danger": colors.red,
        "wm-neutral": colors.trueGray,
        "wm-warning": colors.yellow,
        "wm-info": colors.blue,
        "wm-darkText": colors.white,
        "wm-darkWarning": colors.orange,
      },
    },
  },
  variants: {},
  plugins: [],
});

Ofcourse I'm open to suggestions and opinions. Thoughts?

@ilyasmez
Copy link

This is a very much needed PR, thank you very much @danhab99 🙇‍♂️

@estevanmaito could you please have a look? It's necessary to have an abstraction of the colors instead of using the color names directly. The current customization solution is not solid enough: For example if I want just change the color of the primary button (or my accent color in general), I'll have to rewrite all the styles of the button (like border radius...), and I find this to be not efficient at all, in one project I had to rewrite almost the whole theme.

Just a few remarks regarding the PR, @danhab99:

  • To ensure backwards compatibility and to not introduce breaking changes, I think it makes sense to update the config/wrapper to fall back to the current values if the abstractions are not set by the developer.
  • I'm not sure if it makes sense to use the shades in the theme (e.g. wm-primary-600), I think it should be up to the developer to assign the right color to the abstraction, so in the theme we always use just wm-primary, and leave the selection of the shade to the user. I know that there are cases where we will need a lighter version of a color (e.g. input border), and for these ones I would suggest to use opacity instead of shades.

One alternative solution (mainly to @estevanmaito) would be to merge the Tailwind classes as well in the wrapper, we can use tailwind-merge. This way at least we don't have to write all the rules/styles in our theme just to overwrite the color.

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