Skip to content

feat(wc): Add v9-compatible themes #25660

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.

Sign up for GitHub

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

Merged
miroslavstastny merged 9 commits into from
Dec 21, 2022
Merged

feat(wc): Add v9-compatible themes #25660

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c202364
bump FAST dependencies
miroslavstastny Nov 14, 2022
32f5de1
Add theme tokens (from react-theme)
miroslavstastny Nov 14, 2022
182880e
Add theme switcher to storybook
miroslavstastny Nov 14, 2022
0bb5c9f
deduplicate packages
miroslavstastny Nov 15, 2022
59b529e
fix yarn.lock
miroslavstastny Nov 15, 2022
e8aa93d
Replace @fluentui/react-theme with @fluentui/tokens
miroslavstastny Dec 21, 2022
4cfb168
Merge remote-tracking branch 'upstream/web-components-v3' into feat/w…
miroslavstastny Dec 21, 2022
9f3bcc0
change file
miroslavstastny Dec 21, 2022
8c9a0f3
address PR comments
miroslavstastny Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/web-components/.storybook/preview-body.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<div id="switches-container">
Copy link
Contributor

@Hotell Hotell Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good opportunity to leverage here our theme picker (storybook addon) , later

<label for="theme-switch">Theme</label>
<select id="theme-switch" name="theme">
<option value="web-light" selected>Web Light</option>
<option value="web-dark">Web Dark</option>
<option value="teams-light">Teams Light</option>
<option value="teams-dark">Teams Dark</option>
</select>
</div>
37 changes: 9 additions & 28 deletions packages/web-components/.storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { addons } from '@storybook/addons';
import { DOCS_RENDERED } from '@storybook/core-events';
import * as Fluent from '../src/index-rollup';
import webcomponentsTheme from './theme';
import { switchTheme } from '../public/theme-switch';

Fluent;

function changeTheme(e) {
switchTheme(e.target.value);
}

document.getElementById('theme-switch').addEventListener('change', changeTheme, false);
switchTheme('web-light');

export const parameters = {
layout: 'fullscreen',
controls: { expanded: true },
Expand All @@ -14,36 +20,11 @@ export const parameters = {
},
options: {
storySort: {
order: [
Copy link
Member Author

@miroslavstastny miroslavstastny Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed legacy

'Getting Started',
['Overview', 'Installation', 'Styling'],
'Components',
'Integrations',
['Introduction'],
'Design System',
['Design Tokens', 'Color Explorer', 'High Contrast'],
'Resources',
['Browser Support', 'FAQ', 'License', 'Security'],
'*',
],
order: [],
method: 'alphabetical',
},
},
docs: {
theme: webcomponentsTheme, // override the default Storybook theme with a custom fluent theme
},
};

addons.getChannel().addListener(DOCS_RENDERED, name => {
if (name.toLowerCase().includes('accordion') || name.toLowerCase().includes('card')) {
fillColor.setValueFor(document.body, neutralLayer2);
} else {
fillColor.setValueFor(document.body, neutralLayer1);
}

if (name.toLowerCase().includes('color-explorer')) {
document.body.classList.add('custom-fullscreen');
} else {
document.body.classList.remove('custom-fullscreen');
}
});
8 changes: 4 additions & 4 deletions packages/web-components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@
"mocha": "7.2.0"
},
"dependencies": {
"@microsoft/fast-colors": "^5.3.0",
"@microsoft/fast-element": "^1.11.0",
"@microsoft/fast-foundation": "^2.47.0",
"@microsoft/fast-web-utilities": "^5.4.0",
"@microsoft/fast-element": "^2.0.0-beta.17",
"@microsoft/fast-foundation": "^3.0.0-alpha.21",
"@microsoft/fast-web-utilities": "^6.0.0",
"@fluentui/react-theme": "^9.1.1",
Copy link
Member Author

@miroslavstastny miroslavstastny Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds dependency on a react-theme package 💣
There is nothing React related in that package besides two things:

  1. It has react in the package name.
  2. It adds react as a peer dependency.

If we decide to go this way, we should do one of the following things:

  • Rename react-theme to theme, get rid of the peer dependency (might be breaking change for FUIRv9).
  • Hoist the tokens from react-theme to theme, re-export in react-theme.
  • Use token pipeline to generate the same theme files for web-components.

Copy link
Contributor

@Hotell Hotell Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally we should use same fluent tokens and themes in all web experiences, thus

if react-theme doesn't contain anything react specific besides the peerDep

  • deprecate react-theme on npm
  • rename react-theme to react (removal of peer Deps )
    • having react-theme with re-exports of theme adds unnecessary maintenance

might be breaking change for FUIRv9

from react-components api POV there will be no breaking change. once someone installs new feature bump of react-components they will get theme instead of react-theme in they node_modules

Copy link
Member

@levithomason levithomason Nov 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no use of React in react-theme why can't we just create a theme package and use that directly in react-components and get rid of the react-theme all together?

Copy link
Member Author

@miroslavstastny miroslavstastny Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what @Hotell is proposing (+ npm deprecation). I was not sure whether we would consider it a breaking change or not.

Copy link
Contributor

@spmonahan spmonahan Nov 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that the change is not breaking.

Copy link
Contributor

@EisenbergEffect EisenbergEffect Nov 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was just chatting with @chrisdholt today and we'd definitely love to have a shared, library-agnostic theme package that we can base both React and Web Components on. Sounds like folks here arrived at the same conclusion. +1 from me.

Copy link
Member

@chrisdholt chrisdholt Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 lets start from here

"tslib": "^1.13.0"
}
}
13 changes: 13 additions & 0 deletions packages/web-components/public/theme-switch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { teamsDarkTheme, teamsLightTheme, webDarkTheme, webLightTheme } from '@fluentui/react-theme';
import { setTheme } from '../src/theme';

const themes = {
'web-light': webLightTheme,
Copy link
Member Author

@miroslavstastny miroslavstastny Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, there are no theme exports from web components.
If we decide to go this way, we need to discuss the export contract.

Copy link
Member

@levithomason levithomason Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for Storybook only? Named exports are treeshakable but an object is not, so all themes will always be included here.

Copy link
Member Author

@miroslavstastny miroslavstastny Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, storybook only for now.
Once we know the packages structure, we will decide whether partners will import the theme directly from theme package or web-components will re-export the themes and how.

Copy link
Member

@chrisdholt chrisdholt Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can discuss later - may be ideal to export from the package though given we’re using DesignToken.

'web-dark': webDarkTheme,
'teams-light': teamsLightTheme,
'teams-dark': teamsDarkTheme,
};

export function switchTheme(themeName: keyof typeof themes) {
setTheme(themes[themeName]);
}
383 changes: 383 additions & 0 deletions packages/web-components/src/theme/design-tokens.ts
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions packages/web-components/src/theme/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './design-tokens';
Copy link
Member Author

@miroslavstastny miroslavstastny Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to add bundle-size / tree-shaking tests. After that we might reconsider the way how we export and consume tokens.

export { setTheme } from './set-theme';
12 changes: 12 additions & 0 deletions packages/web-components/src/theme/set-theme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import type { Theme } from '@fluentui/react-theme';
import * as tokens from './design-tokens';

/**
* Sets the theme tokens on defaultNode.
* @param theme Flat object of theme token values.
*/
export const setTheme = (theme: Theme) => {
Copy link
Member Author

@miroslavstastny miroslavstastny Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only defaultNode (global) for now.
If agreed, we will add setValueFor version for scoped theming.

for (const t of Object.keys(tokens)) {
tokens[t].withDefault(theme[t]);
}
};
24 changes: 24 additions & 0 deletions packages/web-components/src/theme/theme.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { DesignToken } from '@microsoft/fast-foundation';
import * as tokens from '../theme/design-tokens';

DesignToken.registerDefaultStyleTarget();

export default {
title: 'Theme/Tokens',
};

export const Tokens = () => `
<div>
<h3>Theme Tokens</h3>
<p>Debug story which uses theme tokens to style the element below.</p>
<div style="
font-family: ${tokens.fontFamilyBase.createCSS()};
Copy link
Contributor

@EisenbergEffect EisenbergEffect Dec 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicholasrice What do you think about adding a toString() and valueOf() implementation to CSSDesignToken? That would make this syntax nicer if used outside of a FAST html or css template.

Copy link
Contributor

@nicholasrice nicholasrice Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toString() seems fine to me. Can you elaborate more on valueOf()? It's not clear to me what that would return.

Copy link
Contributor

@EisenbergEffect EisenbergEffect Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I don't think we need valueOf(). I just checked and the JavaScript runtime prefers toString() for string conversions over valueOf(). Since we don't need to convert to a numerical type of some non-string type we should be fine with toString().

Copy link
Contributor

@nicholasrice nicholasrice Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, I didn't realize that was an object prototype method. I think adding toString() to CSSDesignToken makes sense.

font-size: ${tokens.fontSizeBase300.createCSS()};
background: var(${tokens.colorBrandBackground.cssCustomProperty});
color: ${tokens.colorNeutralForegroundOnBrand.createCSS()};
border: ${tokens.strokeWidthThicker.createCSS()} solid ${tokens.colorNeutralStroke1.createCSS()};
padding: ${tokens.spacingVerticalS.createCSS()} ${tokens.spacingHorizontalM.createCSS()};
box-shadow: ${tokens.shadow28.createCSS()};
">colorNeutralForegroundOnBrand on colorBrandBackground with shadow28</div>
</div>
`;
53 changes: 27 additions & 26 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1604,17 +1604,17 @@
minimatch "^3.0.4"
strip-json-comments "^3.1.1"

"@floating-ui/core@^1.0.0":
version "1.0.0"
resolved "https://registry.yarnpkg.com/@floating-ui/core/-/core-1.0.0.tgz#ec1d31f54c72dd0460276e2149e59bd13c0f01f6"
integrity sha512-sm3nW0hHAxTv3gRDdCH8rNVQxijF+qPFo5gAeXCErRjKC7Qc28lIQ3R9Vd7Gw+KgwfA7RhRydDFuGeI0peGq7A==
"@floating-ui/core@^1.0.1":
version "1.0.1"
resolved "https://registry.yarnpkg.com/@floating-ui/core/-/core-1.0.1.tgz#00e64d74e911602c8533957af0cce5af6b2e93c8"
integrity sha512-bO37brCPfteXQfFY0DyNDGB3+IMe4j150KFQcgJ5aBP295p9nBGeHEs/p0czrRbtlHq4Px/yoPXO/+dOCcF4uA==

"@floating-ui/dom@^1.0.0":
version "1.0.0"
resolved "https://registry.yarnpkg.com/@floating-ui/dom/-/dom-1.0.0.tgz#66923a56755b6cb7a5958ecf25fe293912672d65"
integrity sha512-PMqJvY5Fae8HVQgUqM+lidprS6p9LSvB0AUhCdYKqr3YCaV+WaWCeVNBtXPRY2YIdrgcsL2+vd5F07FxgihHUw==
"@floating-ui/dom@^1.0.0", "@floating-ui/dom@^1.0.3":
version "1.0.4"
resolved "https://registry.yarnpkg.com/@floating-ui/dom/-/dom-1.0.4.tgz#cc0f2a03db7193b1b932b90d09c5c81235682a60"
integrity sha512-maYJRv+sAXTy4K9mzdv0JPyNW5YPVHrqtY90tEdI6XNpuLOP26Ci2pfwPsKBA/Wh4Z3FX5sUrtUFTdMYj9v+ug==
dependencies:
"@floating-ui/core" "^1.0.0"
"@floating-ui/core" "^1.0.1"

"@fluent-blocks/[email protected]":
version "9.2.0"
Expand Down Expand Up @@ -3011,30 +3011,31 @@
eslint-plugin-react "7.24.0"
eslint-plugin-security "1.4.0"

"@microsoft/fast-colors@^5.3.0":
version "5.3.1"
resolved "https://registry.yarnpkg.com/@microsoft/fast-colors/-/fast-colors-5.3.1.tgz#defc59874176e42316be7e6d24c31885ead8ca56"
integrity sha512-72RZXVfCbwQzvo5sXXkuLXLT7rMeYaSf5r/6ewQiv/trBtqpWRm4DEH2EilHw/iWTBKOXs1qZNQndgUMa5n4LA==

"@microsoft/fast-element@^1.10.4", "@microsoft/fast-element@^1.11.0":
"@microsoft/fast-element@^1.10.4":
version "1.11.0"
resolved "https://registry.yarnpkg.com/@microsoft/fast-element/-/fast-element-1.11.0.tgz#494f6c87057bcbb42406982d68a92887d6b5acb1"
integrity sha512-VKJYMkS5zgzHHb66sY7AFpYv6IfFhXrjQcAyNgi2ivD65My1XOhtjfKez5ELcLFRJfgZNAxvI8kE69apXERTkw==

"@microsoft/fast-foundation@^2.47.0":
version "2.47.0"
resolved "https://registry.yarnpkg.com/@microsoft/fast-foundation/-/fast-foundation-2.47.0.tgz#a4cc8c5277e21d080215f5adcaed4266a8dd8a8e"
integrity sha512-EyFuioaZQ9ngjUNRQi8R3dIPPsaNQdUOS+tP0G7b1MJRhXmQWIitBM6IeveQA6ZvXG6H21dqgrfEWlsYrUZ2sw==
"@microsoft/fast-element@^2.0.0-beta.17":
version "2.0.0-beta.17"
resolved "https://registry.yarnpkg.com/@microsoft/fast-element/-/fast-element-2.0.0-beta.17.tgz#46f9b1feaf57e0cb841d31a80574537257b2239f"
integrity sha512-R9NLnH0rh/btfBIdDicHGblw4uk4U7TNTnfpZ25W5IC52cWg7DLr0ky5GujH7wXge8F4ze/Z8anABzhC9eHYrA==

"@microsoft/fast-foundation@^3.0.0-alpha.21":
version "3.0.0-alpha.21"
resolved "https://registry.yarnpkg.com/@microsoft/fast-foundation/-/fast-foundation-3.0.0-alpha.21.tgz#b867b3483d9e2ed496b301e6a6c4017adfa333f4"
integrity sha512-hKY2aI7OI54SN+h2ayJVh4CLD8dcbCuaOWihdiUTBnwNVgl/xANazO3q+Cs2P/5gHHOy2lIgJOZvsK9ilBsCQQ==
dependencies:
"@microsoft/fast-element" "^1.11.0"
"@microsoft/fast-web-utilities" "^5.4.1"
"@floating-ui/dom" "^1.0.3"
"@microsoft/fast-element" "^2.0.0-beta.17"
"@microsoft/fast-web-utilities" "^6.0.0"
tabbable "^5.2.0"
tslib "^1.13.0"
tslib "^2.4.0"

"@microsoft/fast-web-utilities@^5.4.0", "@microsoft/fast-web-utilities@^5.4.1":
version "5.4.1"
resolved "https://registry.yarnpkg.com/@microsoft/fast-web-utilities/-/fast-web-utilities-5.4.1.tgz#8e3082ee2ff2b5467f17e7cb1fb01b0e4906b71f"
integrity sha512-ReWYncndjV3c8D8iq9tp7NcFNc1vbVHvcBFPME2nNFKNbS1XCesYZGlIlf3ot5EmuOXPlrzUHOWzQ2vFpIkqDg==
"@microsoft/fast-web-utilities@^6.0.0":
version "6.0.0"
resolved "https://registry.yarnpkg.com/@microsoft/fast-web-utilities/-/fast-web-utilities-6.0.0.tgz#7678c2b2cd12aeef785f4e2288da93a4db9e38a6"
integrity sha512-ckCA4Xn91ja1Qz+jhGGL1Q3ZeuRpA5VvYcRA7GzA1NP545sl14bwz3tbHCq8jIk+PL7mkSaIveGMYuJB2L4Izg==
dependencies:
exenv-es6 "^1.1.1"

Expand Down