-
Notifications
You must be signed in to change notification settings - Fork 257
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(ui): component theming #5170
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Caleb Pollman <cpollman@amazon.com>
🦋 Changeset detectedLatest commit: f937f27 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/ui/src/theme/createTheme/__tests__/createComponentClasses.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentClasses.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentClasses.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentClasses.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Fixed
Show fixed
Hide fixed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Fixed
Show fixed
Hide fixed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Fixed
Show fixed
Hide fixed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Fixed
Show fixed
Hide fixed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Fixed
Show fixed
Hide fixed
packages/ui/src/theme/createTheme/__tests__/createComponentClasses.test.ts
Fixed
Show fixed
Hide fixed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentClasses.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/ui/src/theme/createTheme/__tests__/createComponentClasses.test.ts
Dismissed
Show dismissed
Hide dismissed
@@ -13,6 +13,11 @@ | |||
"import": "./dist/esm/internal.mjs", | |||
"require": "./dist/internal.js" | |||
}, | |||
"./server": { |
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.
New RSC-path for any RSC components we will export in the future
* | ||
* We should see if there is a way to share this logic with style dictionary... | ||
*/ | ||
const setupToken: SetupToken<WebDesignToken> = ({ token, path }) => { |
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.
Moved these functions to utils.ts
@@ -46,7 +46,6 @@ | |||
"dependencies": { | |||
"csstype": "^3.1.1", | |||
"lodash": "4.17.21", | |||
"style-dictionary": "3.9.1", |
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.
Removed the direct dependency on style-dictionary and opted to copy + paste the code into our source code. This was to get the bundle size down within our limits.
@@ -1,19 +0,0 @@ | |||
import { cssNameTransform } from '../utils'; |
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.
Moved these tests and the source code around a bit. This should be in src/theme/createTheme/tests now
@@ -0,0 +1,13 @@ | |||
import { BaseProperties, Elements } from './utils'; | |||
|
|||
export type AccordionTheme<Required extends boolean = false> = BaseProperties & |
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 Required
optional generic is for ourselves if/when we write our styles with createComponentTheme
to force ourselves to use all the elements/modifiers in the component theme.
@@ -0,0 +1,285 @@ | |||
import kebabCase from 'lodash/kebabCase.js'; |
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.
All of this file until the next comment was just moving the file into the createTheme
directory.
} | ||
|
||
// Internal Style Dictionary methods | ||
// copied from amzn/style-dictionary with the owner's permission |
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.
From here down, I copy/pasted the necessary style dictionary methods we use (and made it so TS didn't hate them)
packages/react/src/components/ThemeProvider/__tests__/ThemeStyle.test.tsx
Fixed
Show fixed
Hide fixed
packages/ui/src/theme/createTheme/__tests__/createComponentTheme.test.ts
Dismissed
Show dismissed
Hide dismissed
deepExtend, | ||
} from './utils'; | ||
import { WebDesignToken } from '../tokens/types/designToken'; | ||
import { createComponentCSS, propsToString } from './createComponentCSS'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
} from './utils'; | ||
import { WebDesignToken } from '../tokens/types/designToken'; | ||
import { createComponentCSS, propsToString } from './createComponentCSS'; | ||
import { isFunction, isString } from '../../utils'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
import { isFunction, isString } from '../../utils'; | ||
import { createColorPalette } from './createColorPalette'; | ||
import { WebTokens } from '../tokens'; | ||
import { CSSProperties } from '../components/utils'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Customer problems
1. Cannot theme a component when there is no design token available
Developers can theme any part of our components in any way, but only in CSS. If they use a theme object they are constrained by the design tokens we expose. For example, a developer cannot add a box shadow on our buttons at the theme level without using plain CSS. It is cumbersome to do some stuff with the theme object and other stuff with plain CSS.
2. No intellisense/type-safety for referencing tokens
Token references in a theme are just strings and do not have intellisense or autocomplete. The only way to know if you referenced a token correctly is to run the app locally and see if it is being styled correctly.
3. No easy way to create custom primitives
#2376
Customers basically have to write their own CSS and use our CSS variables if they want to integrate with our theming system.
4. No RSC support
Not a huge deal, but really since the theme is just CSS there is no reason that it shouldn’t be able to be a server component.
5. Splitting up a theme is not possible
For larger themes with a lot of component customization, a theme object can get very long. Developers might want to split up the theme into component themes like a button theme. Today that is not possible in our types.
DX
Type-safety for theming components in JS
Type-safety for custom component themes/classnames
Description of changes
These are all additive changes so this PR has no breaking changes.
Adding next-app-router example app
This is used to verify RSC-compliant code and is a testing ground for the new theming capabilities and any future RSCs we build.
Adding a
/server
export path in the ui-react package for RSCImporting anything as an RSC will look at the entire module whether or not code is used, so we have to have a separate entry point for any RSC-compliant code. Right now that is only this new theming stuff, but we could expand it in the future. It is currently re-exporting the theming functions from the ui package and a new
<Theme />
RSC.<Theme />
RSCThe Theme RSC acts like the ThemeProvider without the React context. It takes the same props as ThemeProvider. Because this is an RSC the CSS is generated on the server (or at build time) and shipped to the client.
defineComponentTheme
This function is used to: fully theme built-in components, as well as define the theme/styles for custom components.
The
defineComponentTheme
function doesn't really do anything itself. You will need to pass it to thecreateTheme
function. (keep reading)Add
components
attribute increateTheme
Removing direct dependency on style-dictionary
This was a tough one for me, but I wanted to keep this PR within our existing bundle size limits. We were only importing specific functions from style-dictionary which also included an ES6 polyfill, which we don't need. So I copied the code over and removed the ES6 polyfill stuff. This change actually decreased the overall bundle size, so thats nice.
Issue #, if available
Description of how you validated changes
Added a Next app router example to test the changes on. Additionally, tested the docs changes.
Checklist
yarn test
passes and tests are updated/addedsideEffects
field updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.