USACE / groundwork

React Components for USACE Applications
https://usace.github.io/groundwork
4 stars 0 forks source link

Inconsistent default vs props-passed className precedence #31

Open jbkolze opened 2 months ago

jbkolze commented 2 months ago

Within groundwork there is some inconsistency with application of default classNames vs. props-passed classNames. Ideally, props-passed classNames would always take precedence, but this is not always the case. Example: If you pass a custom width to a Card component it still maintains its default full-width.

This can be bypassed by using a "!" operator in front of the class, e.g. className="!gw-w-48". However, if you do this then the user also has to use a "!" operator in order to adjust the class you modified, e.g. className-"!w-36". So, you're somewhat passing the buck to the user end, which isn't ideal.

May be worth investigating other solutions, e.g. tailwind-merge

adamscarberry commented 2 months ago

Observation. gw-w-10, gw-w-24 and a few others work. Most do not. Still feels like it's only using what is already built by the library.

adamscarberry commented 2 months ago

However this is implemented, I think we need something like the following for the config:

import { extendTailwindMerge } from 'tailwind-merge';

export const twMerge = extendTailwindMerge({
  prefix: 'gw-',
});

Although I don't know what happens when we pass in normal tailwind classnames from the consuming app.

adamscarberry commented 1 month ago

@jbkolze is this still an issue?

jbkolze commented 1 month ago

@adamscarberry Yes, still an issue as far as I can tell.

Example: Try setting className="gw-w-96" on a Card component in the docs.

Some widths work, some don't. Think it all depends on order of compilation.

willbreitkreutz commented 1 month ago

@jbkolze @adamscarberry I'm playing with tailwind-merge now, It looks like it works well as long as the classes you pass in are prefixed matching the config, the issue is when we build, only the used classes are bundled so we still run into the missing classes issue, even if we tell consumers to use the gw- prefix to override. I'm going to dig a little deeper and see if I can come up with a way to let you pass in a normal tailwind class, and still have it override the prefixed one.