-
Notifications
You must be signed in to change notification settings - Fork 153
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
Fix whitelabeling support #4573
base: master
Are you sure you want to change the base?
Conversation
Storybook staging is available at https://kiwicom-orbit-dsil-fix-whitelabeling-support.surge.sh |
Size Change: +49 B (+0.01%) Total Size: 459 kB
ℹ️ View Unchanged
|
Deploying orbit with Cloudflare Pages
|
139c7ca
to
bd6d5ba
Compare
"button-primary-subtle-foreground": `var(--button-primary-subtle-foreground, ${tokens.buttonPrimarySubtleForeground})`, | ||
"button-primary-subtle-foreground-hover": `var(--button-primary-subtle-foreground-hover, ${tokens.buttonPrimarySubtleForegroundHover})`, | ||
"button-primary-subtle-foreground-active": `var(--button-primary-subtle-foreground-active, ${tokens.buttonPrimarySubtleForegroundActive})`, | ||
"button-primary-foreground": `rgba(var(--button-primary-foreground, ${tokens.buttonPrimaryForeground}), <alpha-value>)`, |
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.
How does the <alpha-value>
work here? 🤔
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.
Honestly, it's a bit of tailwind magic that seems undocumented buy widely used by the community 😬 I also believe it will change with Tailwind v4, but that is a matter for another time
Object.keys(theme).reduce((acc, key) => { | ||
if (key.startsWith("palette")) { | ||
if (key.startsWith("palette") || key.startsWith("buttonPrimary") || key === "borderRadius100") { |
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.
Is this because all the eligible tokens to be overridden start with these prefixes?
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.
For now, yes. I just wanted to make it work. If we ever consider expanding the list we can think of a more elegant solution
CSS vars were not being injected in the test environment
Small test adjustments from the CSS vars changes
bd6d5ba
to
beef768
Compare
Some tokens were configurable for whitelabels but were not being converted in CSS Vars, so they were really no themeable.
Also, the playwright wrapper was not having access to any of the CSS Vars. This is now fixed.
Closes FEPLT-2197
✨
Description by Callstackai
This PR fixes whitelabeling support by ensuring that certain tokens are converted into CSS variables, making them themeable. It also resolves issues with the Playwright wrapper's access to CSS variables.
Diagrams of code changes
Files Changed
getCssVarsForWL
function to include additional tokens for whitelabeling.kebabCase
function to handle uppercase letters and numbers.generateRgba
function for use in other modules.generateRgba
function.This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions
.snap
. See list of supported languages.