From 49841c9692b30633cd6468cda7e1abdaeb0d6e47 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Thu, 18 Nov 2021 15:03:26 +0100 Subject: [PATCH 1/4] rfc: no functions in makeStyles() --- rfcs/convergence/make-styles-no-functions.md | 94 ++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 rfcs/convergence/make-styles-no-functions.md diff --git a/rfcs/convergence/make-styles-no-functions.md b/rfcs/convergence/make-styles-no-functions.md new file mode 100644 index 00000000000000..617b2a7d6558ed --- /dev/null +++ b/rfcs/convergence/make-styles-no-functions.md @@ -0,0 +1,94 @@ +# Problem + +Currently `makeStyles()` implements two on how styles can be defined: + +```ts +makeStyles({ + rootA: { color: 'red' }, + rootB: theme => ({ color: theme.tokenB }), +}); +``` + +`theme` is typed strictly and offers tokens from `@fluentui/react-theme`. This tokens shape is not extensible for customers. + +# Solution + +## Export tokens separately + +Tokens are just CSS variables, we should be clear with customers on what they are using. This also removes need in `useTheme()` hook as tokens can be accessed directly. + +```tsx +import { tokens } from '@fluentui/react-theme'; + +function CustomComponent() { + return
; +} +``` + +`tokens` is just a plain object: + +```ts +import type { Theme } from '@fluentui/react-theme'; + +const tokens: Theme = { + borderRadiusNone: 'var(--borderRadiusNone)', + borderRadiusSmall: 'var(--borderRadiusSmall)', + /* ... */ +}; +``` + +## Remove functions + +Once `tokens` are available there is no sense to keep functions in `makeStyles()`: + +```diff +import { makeStyles } from '@fluentui/react-make-styles'; ++import { tokens } from '@fluentui/react-theme'; + +makeStyles({ +- root: theme => ({ color: theme.tokenB }), ++ root: { color: tokens.tokenB }, +}); +``` + +## Simplify types in `FluentProvider` + +Currently `FluentProvider` accepts `theme` only as `Theme` type from `@fluentui/react-theme`. If we will simplify it to `Record` this will allow customers to extend our theme. + +```tsx +import { FluentProvider } from '@fluentui/react-provider'; +import { mergeThemes, teamsLightTheme, Theme } from '@fluentui/react-theme'; + +type CustomTokens = { + tokenA: string; +}; +type CustomTheme = CustomTokens & Theme; + +const extendedTheme: CustomTheme = mergeThemes(teamsLightTheme, { tokenA: 'red' }); + +function App() { + return ; +} +``` + +Usage of `FluentProvider` will inject all customer tokens properly and will make them available on React Portals. + +## Using custom tokens in `makeStyles()` + +Once tokens are injected on `FluentProvider` they could be used in `makeStyles()`. + +```tsx +import { tokens } from '@fluentui/react-theme'; +import type { CustomTokens } from './custom-theme'; + +const customTokens: CustomTokens = { + tokenA: 'var(--tokenA)', +}; + +makeStyles({ + root: { + backgroundColor: customTokens.tokenA, + color: tokens.tokenB, + }, +}); +``` From 240db23fd2e33607d4c7c786834f201398c12c38 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Thu, 18 Nov 2021 15:44:29 +0100 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: ling1726 --- rfcs/convergence/make-styles-no-functions.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rfcs/convergence/make-styles-no-functions.md b/rfcs/convergence/make-styles-no-functions.md index 617b2a7d6558ed..8ab2aed3ee5676 100644 --- a/rfcs/convergence/make-styles-no-functions.md +++ b/rfcs/convergence/make-styles-no-functions.md @@ -1,6 +1,6 @@ # Problem -Currently `makeStyles()` implements two on how styles can be defined: +Currently `makeStyles()` can define style rules in two different ways ```ts makeStyles({ @@ -9,13 +9,13 @@ makeStyles({ }); ``` -`theme` is typed strictly and offers tokens from `@fluentui/react-theme`. This tokens shape is not extensible for customers. +`theme` is typed and coupled to tokens from `@fluentui/react-theme`. This tokens shape is not extensible for customers. # Solution ## Export tokens separately -Tokens are just CSS variables, we should be clear with customers on what they are using. This also removes need in `useTheme()` hook as tokens can be accessed directly. +Tokens are just CSS variables, we communicate this fact to customers so that they understand clearly what they are using. This also removes need in `useTheme()` hook as tokens can be accessed directly. ```tsx import { tokens } from '@fluentui/react-theme'; @@ -39,7 +39,7 @@ const tokens: Theme = { ## Remove functions -Once `tokens` are available there is no sense to keep functions in `makeStyles()`: +Once `tokens` are available there is no more need for functional style rules in `makeStyles()`: ```diff import { makeStyles } from '@fluentui/react-make-styles'; @@ -53,7 +53,7 @@ makeStyles({ ## Simplify types in `FluentProvider` -Currently `FluentProvider` accepts `theme` only as `Theme` type from `@fluentui/react-theme`. If we will simplify it to `Record` this will allow customers to extend our theme. +Currently `FluentProvider` only supports the `Theme` type from `@fluentui/react-theme`. If we simplify this to `Record` , we enable consumers to extend the default theme. ```tsx import { FluentProvider } from '@fluentui/react-provider'; @@ -71,11 +71,11 @@ function App() { } ``` -Usage of `FluentProvider` will inject all customer tokens properly and will make them available on React Portals. +`FluentProvider` will inject all customer tokens properly. These tokens will also be available on React Portals. ## Using custom tokens in `makeStyles()` -Once tokens are injected on `FluentProvider` they could be used in `makeStyles()`. +Once tokens are injected through the `FluentProvider` they can be used in `makeStyles()`. ```tsx import { tokens } from '@fluentui/react-theme'; From 3688dc936c73a8f5e944fb97a2e7409ac6fd4f5d Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Thu, 9 Dec 2021 13:37:42 +0100 Subject: [PATCH 3/4] minor updates and cleanups --- rfcs/convergence/make-styles-no-functions.md | 49 +++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/rfcs/convergence/make-styles-no-functions.md b/rfcs/convergence/make-styles-no-functions.md index 8ab2aed3ee5676..10c990fcbded2a 100644 --- a/rfcs/convergence/make-styles-no-functions.md +++ b/rfcs/convergence/make-styles-no-functions.md @@ -1,31 +1,34 @@ -# Problem +# RFC: No functions in `makeStyles()` -Currently `makeStyles()` can define style rules in two different ways +--- + +@layershifter @khmakoto + +## Summary/problem + +Currently `makeStyles()` allows to define style rules in two different ways: ```ts makeStyles({ + // 👇 as an object rootA: { color: 'red' }, + // 👇 as a function rootB: theme => ({ color: theme.tokenB }), }); ``` -`theme` is typed and coupled to tokens from `@fluentui/react-theme`. This tokens shape is not extensible for customers. +- `rootB` uses a function where `theme` is typed and coupled to tokens from `@fluentui/react-theme`. This tokens shape is not extensible for customers. -# Solution - -## Export tokens separately +# Proposed solution -Tokens are just CSS variables, we communicate this fact to customers so that they understand clearly what they are using. This also removes need in `useTheme()` hook as tokens can be accessed directly. +- Remove functions from `makeStyles()` calls +- Export the `tokens` object with CSS variables -```tsx -import { tokens } from '@fluentui/react-theme'; +## Export tokens separately -function CustomComponent() { - return
; -} -``` +Initially we planed support IE11 via runtime tricks, but with the deprecation of IE11, we are now able to leverage CSS Variables for tokens and theming purposes. We communicate this fact to customers so that they understand clearly what they are using. -`tokens` is just a plain object: +The proposal is to export `tokens` as a plain object: ```ts import type { Theme } from '@fluentui/react-theme'; @@ -37,7 +40,19 @@ const tokens: Theme = { }; ``` -## Remove functions +This also removes need in `useTheme()` hook for customers as tokens can be accessed directly, for example: + +```tsx +import { tokens } from '@fluentui/react-theme'; + +function CustomComponent() { + return
; +} +``` + +That simplifies also integration with other CSS-in-JS from Fluent UI v8 or Fluent UI Northstar. + +## Remove functions in `makeStyles()` Once `tokens` are available there is no more need for functional style rules in `makeStyles()`: @@ -53,7 +68,7 @@ makeStyles({ ## Simplify types in `FluentProvider` -Currently `FluentProvider` only supports the `Theme` type from `@fluentui/react-theme`. If we simplify this to `Record` , we enable consumers to extend the default theme. +Currently `FluentProvider` only supports the `Theme` type from `@fluentui/react-theme`. If we simplify this to `Record`, we enable consumers to extend the default theme: ```tsx import { FluentProvider } from '@fluentui/react-provider'; @@ -71,7 +86,7 @@ function App() { } ``` -`FluentProvider` will inject all customer tokens properly. These tokens will also be available on React Portals. +`FluentProvider` will inject all customer tokens properly including scenarios with React Portals. ## Using custom tokens in `makeStyles()` From b8bef769f38c4f251ac653b944e321b3b9f9a6b5 Mon Sep 17 00:00:00 2001 From: Makoto Morimoto Date: Thu, 9 Dec 2021 20:38:31 +0000 Subject: [PATCH 4/4] Updating with separate approach to extending theme accepted by FluentProvider. --- rfcs/convergence/make-styles-no-functions.md | 42 ++++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/rfcs/convergence/make-styles-no-functions.md b/rfcs/convergence/make-styles-no-functions.md index 10c990fcbded2a..7f1b7eed7c06d1 100644 --- a/rfcs/convergence/make-styles-no-functions.md +++ b/rfcs/convergence/make-styles-no-functions.md @@ -26,7 +26,7 @@ makeStyles({ ## Export tokens separately -Initially we planed support IE11 via runtime tricks, but with the deprecation of IE11, we are now able to leverage CSS Variables for tokens and theming purposes. We communicate this fact to customers so that they understand clearly what they are using. +Initially we planed to support IE11 via runtime tricks, but with the deprecation of IE11, we are now able to leverage CSS Variables for tokens and theming purposes. We communicate this fact to customers so that they understand clearly what they are using. The proposal is to export `tokens` as a plain object: @@ -40,7 +40,7 @@ const tokens: Theme = { }; ``` -This also removes need in `useTheme()` hook for customers as tokens can be accessed directly, for example: +This also removes the need of using the `useTheme()` hook for customers as tokens can be accessed directly, for example: ```tsx import { tokens } from '@fluentui/react-theme'; @@ -68,7 +68,9 @@ makeStyles({ ## Simplify types in `FluentProvider` -Currently `FluentProvider` only supports the `Theme` type from `@fluentui/react-theme`. If we simplify this to `Record`, we enable consumers to extend the default theme: +Currently `FluentProvider` only supports the `Theme` type from `@fluentui/react-theme`. + +If we simplify this to `Record`, we enable consumers to extend the default theme: ```tsx import { FluentProvider } from '@fluentui/react-provider'; @@ -86,7 +88,39 @@ function App() { } ``` -`FluentProvider` will inject all customer tokens properly including scenarios with React Portals. +We could alternatively make the theme property in `FluentProvider` extend from `PartialTheme` if we want to ensure that the theme that is passed in always has the keys for the default tokens we provide. + +The type of `FluentProviderProps` would then be: + +```ts +export interface FluentProviderProps + extends Omit, 'dir'>, + Partial { + theme?: TTheme; +} +``` + +And we could then use it as follows: + +```tsx +import { FluentProvider } from '@fluentui/react-provider'; +import { mergeThemes, teamsLightTheme, Theme } from '@fluentui/react-theme'; + +type CustomTokens = { + tokenA: string; +}; +type CustomTheme = CustomTokens & Theme; + +const extendedTheme: CustomTheme = mergeThemes(teamsLightTheme, { tokenA: 'red' }); + +function App() { + return ; +} +``` + +In both scenarios above, `FluentProvider` will still inject all customer tokens properly including scenarios with React Portals. + +Also, in case there is an ask for it, we can decide to export a primitive component in the future (named for example `TokensProvider`) whose only purpose would be to render variables for `Record`. ## Using custom tokens in `makeStyles()`