refactor(Color): Update shadow tokens' names and values #3036
Conversation
|
View your CI Pipeline Execution ↗ for commit e64de76. ☁️ Nx Cloud last updated this comment at |
| @@ -129,7 +129,7 @@ export const coreTheme = createTheme({ | |||
| }, | |||
| }, | |||
| }) | |||
| .addScale('borders', ({ colors }) => ({ | |||
| .addScale('borders', ({ colors }: { colors: Record<string, string> }) => ({ | |||
There was a problem hiding this comment.
Is this ok? TS was complaining that colors was implicitly typed as any (and this is what Copilot suggested) .
LinKCoding
changed the title
| @@ -28,20 +28,20 @@ const DynamicCardWrapper = styled(Box)<CardWrapperProps>( | |||
| prop: 'shadow', | |||
| base: { | |||
| position: 'relative', | |||
| boxShadow: `0px 0px 0 currentColor`, | |||
There was a problem hiding this comment.
Wondering if it's actually better to remove this altogether?
it seems like values for no shadow?
There was a problem hiding this comment.
aere we sure shadow actually get overwritten when this value is removed?
There was a problem hiding this comment.
Double checked on local + preview by removing the code and I don't see any visible difference for shadow
| @@ -54,20 +54,20 @@ const shadowVariants = (mode: ColorModes) => | |||
| prop: 'shadow', | |||
| base: { | |||
| position: 'relative', | |||
| boxShadow: `0px 0px 0 currentColor`, | |||
There was a problem hiding this comment.
same thought here on removal?
📬Published Alpha Packages:@codecademy/[email protected] |
|
🚀 Styleguide deploy preview ready! |
Overview
Updating the names of shadow tokens (colors that use
shadow-*) and updating components that have been usingshadow-*tokens incorrectly, i.e. not semantically correct, like setting a background toshadow-solidinstead ofbackground-contrast.Note:
PR Checklist
Testing Instructions
For Gamut:
?path=/docs/foundations-colormode--docsto see thatshadow-solidis nowshadow-primaryandshadow-opaqueis nowshadow-secondary.polylineshould be roughly the same.PR Links and Envs