-
Notifications
You must be signed in to change notification settings - Fork 2
Fix Type errors and expose themePropertiesConfig option #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Hi @ankarhem, just following up about this PR :) |
| this.userPrefix = options?.prefix; | ||
| this.selector = options?.selector || this.selector; | ||
|
|
||
| if (options?.selector) this.selector = options.selector; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (options?.selector) this.selector = options.selector;
Please revert this syntax or add curly braces.
|
Hey @kaelansmith, sorry about missing this. I'm not sure I follow why this customization is needed? If the package requires some specific css variable, couldn't you just alias the one generated from the theme? Something like: import { Theme, ColorProperty } from "tailwind-easy-theme";
const theme = new Theme(
{
textColor: "#000000",
scrollbarColor: "#FEFEFE", // custom theme property (see config below)
}
);// input.css
@tailwind base;
@tailwind components;
@tailwind utilities;
body {
--scrolly-scroll-prop: hsl(var(--color-scrollbar-color));
} |
|
@ankarhem thanks for following up on this :) Without this customization, const defaultThemePropertiesConfig: ThemePropertiesConfig = {
colors: { prefix: "", type: ColorProperty },
backgroundColor: { prefix: "bg", type: ColorProperty },
textColor: { prefix: "text", type: ColorProperty },
borderColor: { prefix: "border", type: ColorProperty },
accentColor: { prefix: "accent", type: ColorProperty },
ringColor: { prefix: "ring", type: ColorProperty },
caretColor: { prefix: "caret", type: ColorProperty },
divideColor: { prefix: "divide", type: ColorProperty },
outlineColor: { prefix: "outline", type: ColorProperty },
boxShadowColor: { prefix: "box-shadow", type: ColorProperty },
ringOffsetColor: { prefix: "ring-offset", type: ColorProperty },
placeholderColor: { prefix: "placeholder", type: ColorProperty },
textDecorationColor: { prefix: "text-decoration", type: ColorProperty },
gradientColorStops: { prefix: "gradient", type: ColorProperty },
fill: { prefix: "fill", type: ColorProperty },
stroke: { prefix: "stroke", type: ColorProperty },
};By exposing this new API, users can extend/modify/override that default config as they wish, unlocking all kinds of flexibility in terms of how certain theme properties get converted into CSS variables. Does that make sense? |
|
Alright, can you rebase this and I'll approve. |
|
@kaelansmith ping |
This PR adds a user-facing option when instantiating a Theme instance, called
themePropertiesConfig. This allows users to customize the default/internal themePropertiesConfig, providing control over how theme properties' CSS variables get generated (i.e. customize theprefixand/or provide a customtypeclass to handle custom value conversion logic, like how the internal ColorProperty class adds the HSL conversion logic). I came across the need for this because I'm using thetailwind-scrollbarpackage, and needed to add a scrollbar-specific color palette via the plugin's customscrollbarColortailwind theme property, while ensuring it used the ColorProperty logic -- see below:This PR also fixes some TS errors.