bloomer icon indicating copy to clipboard operation
bloomer copied to clipboard

Allow custom colors with isColor

Open Heanthor opened this issue 7 years ago • 5 comments

I noticed that the isColor prop only works with built-in colors like primary. But by customizing bulma's $colors variable, you can use new colors with the is- prefix.

For example:

$colors: (
   myColor: (#ffffff, #000000)
);

will style this correctly

<div class="is-myColor">

but this does not work in Bloomer

<Notification isColor="myColor">

Does this feature make sense to add? Right now, I'm having to manually set the colors like so

<Notification className="is-myColor">

which removes some of the convenience of this awesome library.

Heanthor avatar Feb 03 '18 14:02 Heanthor

Yes, it makes sense. Right now is not allowed because the nature of typescript definitions written in bulma.ts

I'm thinking that we can solve that by allowing a custom ts file to add new colors and to be imported with Bloomer.

I haven't done anything like that because I'll be working on the new version of Bloomer with style-components and this feature is going to be added as a Must Have 😁

If you want, you can start working on something for this version and I can help.

AlgusDark avatar Feb 03 '18 17:02 AlgusDark

I haven't worked with typescript before, but I could certainly learn a little and try to contribute.

So you're suggesting a file like colors.ts that the user adds to define their own colors, instead of using scss? Then just check if it's defined in the main bulma.ts?

Heanthor avatar Feb 03 '18 22:02 Heanthor

All colors are in bulma.tsx Colors type. Maybe we can implement a default for isColor that is used in getColorModifiers like:

export function getColorModifiers({ isColor: color }: Bulma.Color) {
    return [`is-${color}`]: true;
}

That way we can have Typescript intellisense (only for default ones) and we can allow new colors. The best solution should be to extend Color interface to have the custom colors in the intellisense, but I think is a little Overengineering for now. If you think of a better approach, I'm open to discuss :)

This approach is better for this Bloomer Version, and what I was suggesting (a file like colors.ts) is going to be better approach for the new Bloomer Version since we would allow the override of almost anything as if we were using SASS.

Can you try the snippet I showed you and test? I can check your PR if everything is good :)

PS: You had a really good understanding on the suggestion, but I believe that it's going to be a pain to do it and it's going to force people to keep track of several stuff right now.

AlgusDark avatar Feb 04 '18 22:02 AlgusDark

So you're suggesting we remove the isColor check from getColorModifiers to allow any color to work with the function, in which case would it look like this? :

export function getColorModifiers({ isColor: color }: Bulma.Color) {
    return  { [`is-${color}`]: true };
}

I understand now why I'll need to add a default to the Colors type to let me pass an arbitrary string as a Color. This change seems to make the isColor function unused, but I suppose that's okay.

Might we also want to modify getColorModifier in the same way?

I think for now this is a good solution, if your new version is on its way there seems to be less incentive to make a more involved addition.

If this sounds good I'll gladly update the tests and PR it. Might have a few more questions as I'm going 🙂

Heanthor avatar Feb 05 '18 04:02 Heanthor

Yes, I believe that the best way should be to remove the isColor constraint and accept strings, at the end this way we can have the "default" as a Color. So we should remove isColor and make the functions receive color type (like we do now) and return whatever the user pass.

So, if a user pass something like <Button isColor="custom">, the function getColorModifiers is going to return "is-custom" to Button component and problem solve.

We should check for all isColor implementations, like the one with has-text-${modifier}. At first I was doing all these to help the user so it was rare to have errors, but we need to leave the user the power to do anything since at the end, if we do something like <Component className="custom"> and we don't have .custom in our css, React renders our Component with that className.

So, check for those constrains, maybe we can modify another constraints until the release of the new Bloomer Library 😃

AlgusDark avatar Feb 05 '18 04:02 AlgusDark