activist-org / activist

An open-source activism platform
https://activist.org
GNU Affero General Public License v3.0
234 stars 191 forks source link

Combine color attributes into light/dark mode custom Tailwind classes #751

Open andrewtavis opened 7 months ago

andrewtavis commented 7 months ago

Terms

Description

It would be great to check the custom colors we have in tailwind.config.ts to see if there are some that are being used very infrequently in the platform. If so, we can replace these with composite versions of the colors. Specifically if there's a custom color that's just a more used color with an alpha, that would indicate that we can remove it.

It'd also be great if the names of the colors could be checked to see if they're descriptive of their usage, or if a rename would be helpful in these regards :)

Contribution

Happy to discuss potential changes and support in general! 😊

andrewtavis commented 7 months ago

@bozmen, do you want to work on this?

bozmen commented 7 months ago

Yes, that sounds good to me

andrewtavis commented 7 months ago

Assigned 😊 Thanks @bozmen! Let me know if there's anything I can do to support :)

andrewtavis commented 7 months ago

Heyo @bozmen 👋 Something that came from the Vue.js meetup is whether we wanted to combine the light and dark mode versions of colors we defined in tailwind.config.ts into common ones in frontend/assets/css/tailwind.css. So rather than text-light-text dark:text-dark-text we could do something like text-brand-text, text-brand-distinct-text, bg-brand-layer-0, etc. This wouldn't be done everywhere, but just places where the both are used in the same place. This is not the case in places like the CTA buttons where the dark mode background is the CTA color with an alpha and the border/text are the CTA orange color rather than the text color.

andrewtavis commented 7 months ago

I think that maintaining the Tailwind style on this would make sense and make it more clear how to use the individual props when they're used alone :)

Let me know what you think!

bozmen commented 7 months ago

Yep, that makes perfect sense, and it solves the "doubt" that I had and discussed with you over signal, if i understood this correctly :) But I do not understand the second point that you make, what do you mean exactly?

andrewtavis commented 6 months ago

Wow sorry, totally blanked on this... You mean the CTA buttons? They're one of the elements where the styles don't mirror to just the dark mode of the other color mode. So for light mode we have black borders and text with an orange background, and for dark mode we have orange borders and text with an orange background with an low alpha. In this case we can't just drop in one of the common styles as discussed above :)

I'm have changes to remove the unused Tailwind classes locally at this point. Will send them along hopefully soon :) Renaming this to reflect that we're making the combined Tailwind classes 😊

amyrman commented 4 months ago

As discussed in the call it'd be great if this could be assigned to me, thanks!

andrewtavis commented 4 months ago

Hey @bozmen, I'm in a call with @amyrman right now and we thought that this would be a good one 😊 Maybe you can take a look at #839 and other image based one :) We can chat later as well!

@amyrman, to reiterate what we discussed, we want to do the following:

.text-brand {
  @apply text-light-text dark:text-dark-text;
}

.fill-text-brand {
  @apply fill-light-text dark:fill-dark-text;
}

If you see anything else that needs to be fixed along the way, feel free to note it!

andrewtavis commented 4 months ago

@amyrman, if you're using VS Code for your IDE, there are some suggested extensions in the readme environment setup section in the Suggested IDE setup dropdown 😊 Those will help for this one so you can see that the color classes are being applied in your IDE :)

andrewtavis commented 4 months ago

Hey @amyrman 👋 Quick checkin just to make sure that all's well here :) Let me know if a call would be helpful :)