Enhancements on Custom Themes

[thomascarman]

Describe the feature in detail (code, mocks, or screenshots encouraged)

I came across this issue that my enhancements to a custom theme were not applying after upgrading v2 following the css-js conversion instruction in documentation. I'm looking to help with the project. I will submit a pull request to add this feature to the project.

if(theme.enhancements) {
  baseStyles = { ...baseStyles, ...theme.enhancements };
}

What type of pull request would this be?

Enhancement

Provide relevant links or additional information.

While in the types.ts file for the tw-plugin it has an example for enhancements on custom elements, but the index.ts file only parses through theme properties and properties_dark.

• Custom Enhancements in Types.ts
• Spot for adding the Custom Enhancements

[endigo9740]

@thomascarman I've reached out to @AdrianGonz97 to review this. He implemented the latest iteration of the Skeleton Tailwind plugin and the new Skeleton theme structure. I believe he had some hesitation to us implementing enhancements on user-generated themes. We were intending to keep these limited to the preset themes only for now.

If he gives the thumbs up we'll move forward with this, otherwise I'd expect you'll hear back from him on why we may not wish to proceed with this right now.

We have some major plans for themes in the near future, so a lot of this may coincide with that:

• NEXT: New Theme Generator #1142

[thomascarman]

@endigo9740 Sounds great, Thanks!

[AdrianGonz97]

Thanks for raising an issue and creating a PR!

As Chris mention, I'm a bit hesitant on adding this feature. When I initially rewrote the plugin, I already had this feature implemented. But after playing with it for some time, I really didn't like the API. It felt very clunky and very easy to get wrong. There weren't any guard rails I could add to guide the user towards the right direction. I figured that the best way for users to add their own "enhancements" was to just leave them in their app.postcss file.

One of the caveats I didn't like was the requirement of prefixing each of their selectors with an attribute selector like so: [data-theme="theme-name"].

So, if you look at our skeleton theme, you'll see that to add a specific enhancement for each of the headings, we would need to prefix the attribute selector for each:

enhancements: {
	"[data-theme='skeleton'] h1, [data-theme='skeleton'] h2, [data-theme='skeleton'] h3, [data-theme='skeleton'] h4, [data-theme='skeleton'] h5, [data-theme='skeleton'] h6":
	{ fontWeight: 'bold' }
}

which is rather ugly, unwieldy, and pretty easy to make a mistake if you happen to misspell one of the attributes/theme names.

One idea I wrangled with was to prefix the attribute selector for the users, so all they would have to do is just add the selector they want along with their desired declarations:

enhancements: {
	"h1, h2, h3, h4, h5, h6": { fontWeight: 'bold' }
}

Definitely more readable, but this wouldn't work either unless I start parsing and splitting the keys for commas to prefix a [data-theme='theme-name'] to each selector. I sense that this would cause a lot of headaches down the line.

Next idea was require users to individually write their selectors like so:

enhancements: {
	"h1": { fontWeight: 'bold' },
	"h2": { fontWeight: 'bold' },
	"h3": { fontWeight: 'bold' },
	// ...
}

and prefix every key with [data-theme="theme-name"].

This works, but perhaps it's too verbose? I don't know. It's something I still want to think about and am very open to suggestions!

[endigo9740]

Great explanation @AdrianGonz97, to provide more context from my comment earlier. We do have plans to flush out themes a bit further here:

• NEXT: New Theme Generator #1142

As part of this, one of the things I'd like to do is replace enhancements (even for preset themes) with their own first class CSS custom properties. If you browse our existing preset themes there's a common pattern of what's being added. This typically includes heading font weights and the body background color and/or image.

Given this, the simpler approach might be to just implement something like:

--heading-font-weight: ...
--body-bg-image: ...
...

NOTE: these are just example names

Make these a standard part of the theme, so that we can then also build Skeleton design tokens around them as well.

Doing this, enhancements then becomes redundant and unnecessary in most cases. Though I do like the idea of extensibility - which is why we currently allow folks to implement their own custom CSS properties if they so choose. This keeps those portable within the theme, but can be consumed as needed in the app. This is undocumented, but possible right now.

Just note we're still noodling on this. These represent our first idea for how to move forward on these, but may not yet be the BEST solution. It'll take time and testing to determine the best way forward.

[thomascarman]

Thanks again much appreciated explanations. @AdrianGonz97 exactly what I end up doing last night was just reverted back to just leaving in them in the postcss file. Would be nice to have it all contained and singular format.

I have been only messing with this project for a short time. I'll do some more digging maybe help come up with some ideas.

Yall are doing an awesome job! 👍

[endigo9740]

FYI I've closed the pending PR per the discussion above. This is a topic we're still exploring alongside our future updates to the themes and theme generator. We'll keep this thread for reference though.

[endigo9740]

Per the automated message above, I'm going to close this issue and fold this request in to the group thread. Closing doesn't mean we aren't consider it, just that it'll be part of the larger initiative. Thanks!