-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat(vanilla-extract): remove sprinkles usages from TS files #1257
Conversation
Size stats
|
Accessibility report ℹ️ You can run this locally by executing |
paddingRight: typeof paddingRight === 'object' ? paddingRight.mobile : paddingRight, | ||
}; | ||
} | ||
const paddingTopValues = |
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.
I couldn't come up with a way to use sprinkles in Box and Grid components (because I don't know how to replicate the fallback logic with pure CSS).
The only way I was able to make this work is to use CSS vars and style()
instead of sprinkles()
in the corresponding CSS files.
Deploy preview for mistica-web ready! ✅ Preview Built with commit 6dd3540. |
Screenshot tests report ✔️ All passing |
@@ -619,15 +618,15 @@ const Tooltip = ({ | |||
return ( | |||
<BaseTooltip | |||
content={ | |||
<Box className={styles.content}> |
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.
This was working just by luck. The Box component sets padding to 0 by default, and in this case the padding is being applied also through className prop.
When changing the logic in the Box CSS file to use vars instead of sprinkles, the padding from props took precedence over the one passed through className (I guess it depends on how vanilla determines the one that "wins").
In this case, given that we are not using padding props at all, it doesn't actually makes sense to use Box here. I've replaced it with a div to avoid the issue I've mentioned before
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.
I've also detected some use cases in webapp where padding is being applied through className instead of props. These will fail because of the same reason, so we may need to fix them after merging this PR:
- web/src/pages/cancellation-with-retention-v3/v1/components/list-offers/index.tsx
- web/src/pages/mv-forms/forms/cancelamento-sva/commons/infos/index.tsx
- web/src/pages/mv-forms/forms/problemas-cobrancas/commons/infos/index.tsx
- web/src/pages/mv-forms/forms/suporte-tecnico/commons/infos/index.tsx
- web/src/pages/mv-forms/forms/suspensao-temporaria/commons/choice/index.tsx
- web/src/pages/mv-forms/forms/suspensao-temporaria/commons/infos/index.tsx
- web/src/pages/update-contacts-vivo-code/components/update-contact-parcial/index.tsx
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.
As commented in teams, I'd deprecate the className prop in the Box component. If someone wants a div with styles, then they can use a div.
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.
Done, deprecated className
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 is possible to add an eslint rule to forbid sprinkes imports in .tsx files?
Yes, I've added it here :) |
# [16.2.0](v16.1.1...v16.2.0) (2024-10-08) ### Bug Fixes * **Tooltip, Popover:** allow styling target wrapper ([#1262](#1262)) ([38abf25](38abf25)) * **Touchable:** make content focusable when using onPress + as="a" ([#1255](#1255)) ([5e297d5](5e297d5)) ### Features * **Align:** create component ([#1254](#1254)) ([86fb294](86fb294)) * **Logo:** allow color override ([#1263](#1263)) ([4d16939](4d16939)) * **vanilla-extract:** remove sprinkles usages from TS files ([#1257](#1257)) ([b0d6040](b0d6040))
🎉 This PR is included in version 16.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Issue: Link
Before:
After: