redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.09k stars 980 forks source link

A case for a different css strategy in scaffolds #610

Closed Terris closed 4 years ago

Terris commented 4 years ago

I’m hoping to use Redwood often ;) and when I do I will also often want to implement a custom design in the Admin section while still taking advantage of CLI scaffold generators. However, I feel the current implementation of Tailwind-fashion utility classes is less than ideal for this usage. I have nothing against Tailwind—it’s a fine tool. But I don’t feel it’s the best implementation in this case as the generated components contain (in my opinion) an exorbitant amount of className attributes and classes.

Presently, if I want to implement any other styles—be it another framework or a unique design—I will always spend a great deal of time scrubbing every file that gets generated, every time I generate a scaffold. By nature, the generated files are meant to output the same thing (Of course! That’s what makes generators useful) so implementing a css strategy that minimizes class scrubbing within those many components and pages and instead lends itself to reusability via a single css file or set of css files, seems far more ideal in this instance.

If there is any sense of persuasion here, I’ve put together a list of actionable options, any of which I will happily instigate and contribute code for myself. Also, I’m wide open to further discussion and suggestions. We could, for instance:

Have I made my case and, if so, do any of these options fit the Redwood-uniform?

p.s. Many, many thanks to the core team and to all the contributors. This thing is awesome!

cannikin commented 4 years ago

Thanks for bringing this up!

We had another issue recently where someone brought up the utility-based classes used in the scaffolded pages. My plan was to turn the classes into good old fashioned semantic names so at least they would be easier to customize if someone wanted to do so. I'd just prefix them with something like rw- to avoid future name clashes.

I love extracting to component classes in my own sites, but we don't want to include the full TailwindCSS and/or PostCSS packages for the @apply directive to be supported. And I really don't want to write our own version!

We do plan to add support for custom generator templates. So once you decide what HTML/CSS you want your admin screens to include you can be sure that all future scaffolded pages will have the same layout. We don't have an ETA for that feature though.

If you'd like to take a stab at a PR that switches to semantic names, please feel free! The only requirements would be that the final output visually matches as closely as possible to what's currently generated.

brianespinosa commented 4 years ago

@Terris I had similar thoughts when looking at the generators for what gets output for what can be used for an admin area... and I love the suggestion of a CLI --no-css flag. I guess that becomes irrelevant though once support for custom generator templates come around. Is that the idea at least @cannikin for not necessarily having a bare version with a flag?

Terris commented 4 years ago

@cannikin My apologies for the duplicate issue and thanks for the insight.

Yes, I'd absolutely like to open up a PR to switch to semantic names. My goal will be to make it match, visually, the current design. My plan of action is to copy over exactly the styles of each element/component to the related class name.

Shall I assign you as a reviewer on the PR?

cannikin commented 4 years ago

@brianespinosa For the bare version, wouldn't you just have the opposite problem—instead of stripping out all the existing classes, you now need to add classes to everything? I wonder how helpful that would be for most people? But once custom generators are ready you can have them be bare or anything else you want!

@Terris no problem on the duplicate, I can't even find the original issue and can't remember if it was even here on GitHub or in our discussion forum...

Yes please, add me as a reviewer when you're ready! 😀

Tobbe commented 4 years ago

For the bare version, wouldn't you just have the opposite problem—instead of stripping out all the existing classes, you now need to add classes to everything? I wonder how helpful that would be for most people?

--no-css to me just means "no css styles", not "no classes". Leaving classes to be able to target with custom css would be perfectly fine to me (maybe even expected). But yes, I would expect it to remove stuff like "md:flex-shrink-0" as to me that's just css disguised as a class.

cannikin commented 4 years ago

Ahhh so you just want to skip generating the scaffold.css file. I'm okay with that option!

I'd vote that the flag be something like --skip-stylesheet to maybe make it a little more clear as to what's getting skipped. I've always liked Rails's options being prefixed with skip when you don't want something generated.

Tobbe commented 4 years ago

With the way it's currently done there would be no easy way to implement --no-css. But, depending on how Terris' PR evolves, then, yes, maybe just skipping scaffold.css will be enough.

But if that's all it's going to do, I don't think it's worth the extra LOCs to have it. Easy enough to just remove that file manually in that case.