-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Semantic Tokens #33831
base: master
Are you sure you want to change the base?
Semantic Tokens #33831
Conversation
Todo:
|
@@ -24,6 +24,7 @@ | |||
"@fluentui/react-shared-contexts": "^9.21.2", | |||
"@fluentui/react-tabster": "^9.23.3", | |||
"@fluentui/react-theme": "^9.1.24", | |||
"@fluentui/semantic-tokens": "^0.0.0-alpha.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.
Should we be importing this via react-theme for Fluentui component usage?
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.
like re-export through the react-theme
package? I think the less we tangle dependencies the better IMHO.
Pull request demo site: URL |
"@fluentui/scripts-api-extractor": "*" | ||
}, | ||
"dependencies": { | ||
"@swc/helpers": "^0.5.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.
Do we actually need this as a dep vs a devdep?
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.
Not 100% certain but following the same dependency schema as our other top level packages here
7d5e1fb
to
b4c5a56
Compare
@@ -272,6 +272,7 @@ packages/react-components/react-virtualizer/stories @microsoft/xc-uxe @Mitch-At- | |||
packages/react-components/react-skeleton/library @microsoft/cxe-prg |
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.
🕵🏾♀️ visual regressions to review in the fluentui-web-components-v3 Visual Regression Report
Accordion 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Accordion. - Dark Mode.normal.chromium_1.png | 2660 | Changed |
Avatar 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Avatar. - Dark Mode.normal.chromium.png | 10569 | Changed |
Switch 2 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Switch. - Dark Mode.hover.chromium_2.png | 92 | Changed |
Switch. - Dark Mode.normal.chromium_1.png | 92 | Changed |
📊 Bundle size reportUnchanged fixtures
|
🕵 FluentUIV0 No visual regressions between this PR and main |
2b7463b
to
22b1d58
Compare
6ee8605
to
1d6d672
Compare
🕵 fluentuiv9 No visual regressions between this PR and main |
🕵 fluentuiv8 No visual regressions between this PR and main |
export const ctrlLinkForegroundBrandHover: string; | ||
|
||
// @public (undocumented) | ||
export const ctrlLinkForegroundBrandHoverRaw = "--ctrl-link-foreground-brand-hover"; |
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 shipped prefixing our tokens with --smtc-
, we shoudl probably align on that.
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'm a little concerned this isn't matching the JSON I've received, it's easy enough for us to add but need to be sure we are all synced on this - can discuss this at our meeting today.
|
||
// Text | ||
export const textStyleDefaultRegularFontfamily = `var(--text-style-default-regular-fontfamily, ${tokens.fontFamilyBase})`; | ||
export const textGlobalBody3Fontsize = `var(--text-global-body3-fontsize, ${tokens.fontSizeBase300})`; |
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.
Will need a system for layering in optional tokens.
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.
My understanding is that these tokens do not have an optional token layer, based on the JSON I recieved - we can add multiple layers if needed, but not sure we want multiple layers of optional tokens if not needed (for optimized bundle size) and some of these tokens are provided as component level tokens which will have an additional component-specific targeted CSSVar.
Consuming users can override '--text-style-default-regular-fontfamily' if looking to replace any preconfigured CSS vars (but in Fluent 2 this will be null and resolve to existing tokens.fontFamilyBase).
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.
One thing to call out is that sometimes, the value is a calc
function trying to coerce a value out of two F2 variables or maybe just a random static value that's unvariablized. But those values will be available as a single semantic token. So there are times where a token value might look more like...
export const someToken = `var(--someToken, calc(${tokens.somef2token} + ${tokens.someOtherF2Token}))`;
I think we only have a couple instances where this happens. If values like this in your exports (which is totally normal), you end up having to create an internal variable in the component to resolve it. I'd have to take a deeper look but it's also possible your *Raw
variable exports are a better workaround for this scenario.
DO NOT MERGE: This is a proof of concept draft only (for now).
Previous Behavior
Tokens provided a direct token -> CSS variable link
New Behavior
New semantic-tokens top level library
Tokens are now semantically enabled, providing options for fallbacks and component-level styling.