-
Notifications
You must be signed in to change notification settings - Fork 289
feat: syncs and adds support for button and button-link in site theme #2190
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: integrations/makeswift
Are you sure you want to change the base?
feat: syncs and adds support for button and button-link in site theme #2190
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
d4765d8
to
ce302f8
Compare
⚡️🏠 Lighthouse reportLighthouse ran against https://catalyst-canary-qxq09w3d8-bigcommerce-platform.vercel.app 🖥️ DesktopWe ran Lighthouse against the changes on a desktop and produced this report. Here's the summary:
📱 MobileWe ran Lighthouse against the changes on a mobile and produced this report. Here's the summary:
|
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.
Looks good! Can we please get a changeset with details on the change, what to expect, what to do to migrate, etc.?
@@ -17,6 +17,7 @@ export const SiteTheme = ({ fontTokens, ...theme }: TokensProps & ThemeProps) => | |||
${fontTokensToCssVars(fontTokens).join('\n')} | |||
${themeToCssVars(theme).join('\n')} | |||
--font-family-mono: var(--font-family-accent); | |||
--button-primary-text: var(--button-primary-foreground); |
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.
We could change all the foreground
prop names to text
in core/lib/makeswift/components/site-theme/components/button.ts to avoid needing to declare a new --button-primary-text
css variable here. We're already building the css variables from the Makeswift control values via themeToCssVars(), so I would prefer to see that functionality not overridden if we can avoid the extra complexity of doing so.
What/Why?
Button
andButtonLink
with SoulForeground
toText
in Site ThemeTesting
CleanShot.2025-04-03.at.14.32.16.mp4