Fluent Semantic Tokens - #33971
Closed
Mitch-At-Work wants to merge 13 commits into
Closed
Conversation
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
Collaborator
🕵 FluentUIV0 No visual regressions between this PR and main |
| @@ -272,6 +272,7 @@ packages/react-components/react-virtualizer/stories @microsoft/xc-uxe @Mitch-At- | |||
| packages/react-components/react-skeleton/library @microsoft/cxe-prg | |||
Collaborator
There was a problem hiding this comment.
🕵🏾♀️ visual regressions to review in the fluentuiv8 Visual Regression Report
Callout 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| Callout.No callout width specified.default.chromium.png | 2319 | Changed |
Keytip 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| Keytip.Offset.default.chromium.png | 121 | Changed |
react-charting-LineChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| react-charting-LineChart.Gaps.default.chromium.png | 1 | Changed |
| @@ -272,6 +272,7 @@ packages/react-components/react-virtualizer/stories @microsoft/xc-uxe @Mitch-At- | |||
| packages/react-components/react-skeleton/library @microsoft/cxe-prg | |||
Collaborator
There was a problem hiding this comment.
🕵🏾♀️ 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_1.png | 305 | Changed |
Badge 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| Badge. - Dark Mode.normal.chromium.png | 511 | Changed |
Checkbox 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| Checkbox. - Dark Mode.normal.chromium_1.png | 3 | Changed |
MenuList 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| MenuList. - RTL.2nd selected.chromium.png | 17 | Changed |
| MenuList. - RTL.normal.chromium_1.png | 39102 | Changed |
Comment on lines
+364
to
+368
| export const textStyleDefaultHeaderFontfamily = `var(${textStyleDefaultHeaderFontfamilyRaw}, ${textStyleDefaultRegularFontfamilyRaw})`; | ||
| export const textStyleDefaultHeaderLetterspacing = `var(${textStyleDefaultHeaderLetterspacingRaw}, ${textStyleDefaultRegularLetterspacingRaw})`; | ||
| export const textStyleAiRegularFontfamily = `var(${textStyleAiRegularFontfamilyRaw}, ${textStyleDefaultRegularFontfamilyRaw})`; | ||
| export const textStyleAiRegularWeight = `var(${textStyleAiRegularWeightRaw}, ${textStyleDefaultRegularWeightRaw})`; | ||
| export const textStyleAiRegularLetterspacing = `var(${textStyleAiRegularLetterspacingRaw}, ${textStyleDefaultRegularLetterspacingRaw})`; |
Contributor
There was a problem hiding this comment.
Looks like theres's a bug here in the whole file. The second token in the chain should be pointing to the variable string not the raw token string. Credit Zacky Ma (@marchbox) for spotting it.
- export const tokenA = `var(${tokenARaw}, ${tokenBRaw})`
+ export const tokenA = `var(${tokenARaw}, ${tokenB})`Not sure where else this is a problem.
Contributor
There was a problem hiding this comment.
investigating. I also fixed an issue where there were some times we needed a var() function wrapper and others we didn't. This should be fixed now. I'll see what's up with this now.
Contributor
There was a problem hiding this comment.
So looking at this, not sure we want to do this actually. We ideally build the template string literal through the algo and raws vs the actual token values. Otherwise, we have a pretty confusing situation and then we also lock tokens into other tokens when we need the chain to be dependent on the specific rule it's applied to. We did have a situation where the vars at the end of the chain didn't have a proper var() function call at the end. We are also missing F2 fallbacks for some locations because we haven't defined them in fluentOverrides.ts which might be why this seems like it might require using the non-raw version. Those will be filled out as we go though.
Collaborator
🕵 fluentuiv9 No visual regressions between this PR and main |
updated tsconfig to lint our scripts folder converted fluentOverrides to ts
updating API file
brandonthomas
force-pushed
the
user/mifraser/fluent-semantic-tokens
branch
from
March 18, 2025 22:23
e4845b6 to
b15073a
Compare
updating api
Mitch-At-Work
force-pushed
the
user/mifraser/fluent-semantic-tokens
branch
2 times, most recently
from
March 27, 2025 18:33
c9ea8ca to
00c7cda
Compare
Mitch-At-Work
marked this pull request as ready for review
March 27, 2025 21:52
Mitch-At-Work
requested review from
a team,
Justin Slone (JustSlone),
Juraj Kapsiar (jurokapsiar) and
Tudor Popa (tudorpopams)
as code owners
March 27, 2025 21:52
Contributor
Author
|
We have now merged this functionality via separate PRs into the extended-tokens feature branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preview release of Fluent Semantic Tokens.
unsetvalues will result in existing CSS to cascade properly.Further token changes incoming as tokens.json evolves.
TODO:
Handle 'null' tokens -> Either setting these to 'unset' in CSS, providing the fluent 2 fallback, or specific cases where it may resolve to 'transparent'fluentOverrides.jsonwill house these relationships so the script can auto gen.fluentOverrides.jsonas well.Import fluent 2 tokens from a flat-exported package so that we avoid importing the entire tokens object (to enable tree shaking and reduce bundle size).Next Steps: