From de90f8fbb8294e6c0166c5753da31090474b6bf1 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 13:17:34 +0200 Subject: [PATCH 01/18] i --- .../convergence/input-appearance-migration.md | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 rfcs/convergence/input-appearance-migration.md diff --git a/rfcs/convergence/input-appearance-migration.md b/rfcs/convergence/input-appearance-migration.md new file mode 100644 index 00000000000000..9dfab9d6d10f24 --- /dev/null +++ b/rfcs/convergence/input-appearance-migration.md @@ -0,0 +1,78 @@ +# RFC: Input appearance migration + +@peta-duda + +## Summary + +Fluent V9 changes the appearance of `Input` component. V0 Input component default background colour is grey-ish, whereas in V9 the background colour of the input is white. + +## Background + +Partners could change the background colour to match the previous version of Fluent by passing a prop `appearance` with value `filled-darker`, but this solution does not scale well for partners when migrating to the new version. + +## Problem statement + +This RFC explores potential solutions for partners, so they could migrate input components to V9 without the extra work of adding an additional prop to input components. + +Partners should also easily revert the decision to the default appearance value if they decide so, without changing inputs individually. + +馃憥 Cons of adding appearance to achieve the same design as V0: + +- Not scalable, partners would have to add the prop to every component +- If they decide to revert back to default appearance in future, they would have to go through the same pain again by removing the prop +- Is error prone + +## Detailed Design or Proposal + +### Composition component + +Partners could create a new composition component and modify the props in their preferred way. If the partner would like to keep the original colour, they could create the composition component and have the default component without the appearance prop renders as `filled-darker`. + +馃憤 Pros: + +- Easy to create +- Would work in iframes +- Is safe + +馃憥 Cons: + +- Creating new input that does not match the fluent V9 doc. + +### Global css selector + +Targeting all the input selectors from a partner app and change the colour with global css. + +馃憥 Cons: + +- Wouldn鈥檛 work in iframes +- Difficult to validate + +### New token alias to theme + +Adding a new theme token that the partner could override the existing one. + +馃憤 Pros: + +- Easy to do ? + +馃憥 Cons: + +- Negative impact on performance by increasing variables (as read here: [fluentui/theme-shared-colors.md at d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351 路 microsoft/fluentui (github.com)](https://github.com/microsoft/fluentui/blob/d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351/rfcs/react-components/convergence/theme-shared-colors.md)) + +### Unify design + +Discuss with designers to unify V0 and V9 design, setting the appearance to filled-dark by default. + +馃憤 Props: Will make migration easier for partners who already uses V0 + +馃憥 Cons: Inherits old design/dependency instead of inventing the best thing for future + + + + + + + + + + From 286ec1905a1462ec5e90b1467cd8e6fd1b87d2cc Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 13:37:11 +0200 Subject: [PATCH 02/18] appearance --- ...ppearance-migration.md => apperance-migration.md} | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) rename rfcs/convergence/{input-appearance-migration.md => apperance-migration.md} (83%) diff --git a/rfcs/convergence/input-appearance-migration.md b/rfcs/convergence/apperance-migration.md similarity index 83% rename from rfcs/convergence/input-appearance-migration.md rename to rfcs/convergence/apperance-migration.md index 9dfab9d6d10f24..8128b10389a131 100644 --- a/rfcs/convergence/input-appearance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -1,14 +1,14 @@ -# RFC: Input appearance migration +# RFC: Appearance migration @peta-duda ## Summary -Fluent V9 changes the appearance of `Input` component. V0 Input component default background colour is grey-ish, whereas in V9 the background colour of the input is white. +Fluent V9 changes the default appearance of input components (`Input`, `Dropdown`). V0 input components default background colour is grey-ish, whereas in V9 the background colour is white. ## Background -Partners could change the background colour to match the previous version of Fluent by passing a prop `appearance` with value `filled-darker`, but this solution does not scale well for partners when migrating to the new version. +Partners could change the background colour to match the previous version of Fluent by passing a prop `appearance` with value `filled-darker`, but this solution does not scale well for partners when migrating to the new version and is error prone. ## Problem statement @@ -36,15 +36,15 @@ Partners could create a new composition component and modify the props in their 馃憥 Cons: -- Creating new input that does not match the fluent V9 doc. +- Creating new input components (`Dropdown`, `Input`) that wouldn't match the Fluent V9 doc. ### Global css selector -Targeting all the input selectors from a partner app and change the colour with global css. +Targeting all input selectors from a partner app and change the colour with global css. 馃憥 Cons: -- Wouldn鈥檛 work in iframes +- Wouldn鈥檛 work in iframes (?) - Difficult to validate ### New token alias to theme From 19001b4fb2bc58c451db95a911d16c7aa17cfec6 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 13:39:33 +0200 Subject: [PATCH 03/18] up --- rfcs/convergence/apperance-migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 8128b10389a131..821692e4459bfa 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -65,7 +65,7 @@ Discuss with designers to unify V0 and V9 design, setting the appearance to fill 馃憤 Props: Will make migration easier for partners who already uses V0 -馃憥 Cons: Inherits old design/dependency instead of inventing the best thing for future +馃憥 Cons: Inherits old design/dependency that might not be relevant this time From d136f9e2b3689b4191a835c6ccc2c25addc4d0a6 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 13:42:55 +0200 Subject: [PATCH 04/18] txtarea --- rfcs/convergence/apperance-migration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 821692e4459bfa..cbe556b6ece1d0 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -4,7 +4,7 @@ ## Summary -Fluent V9 changes the default appearance of input components (`Input`, `Dropdown`). V0 input components default background colour is grey-ish, whereas in V9 the background colour is white. +Fluent V9 changes the default appearance of input components (`Dropdown`, `Input`, `TextArea`). V0 input components default background colour is grey-ish, whereas in V9 the background colour is white. ## Background @@ -36,7 +36,7 @@ Partners could create a new composition component and modify the props in their 馃憥 Cons: -- Creating new input components (`Dropdown`, `Input`) that wouldn't match the Fluent V9 doc. +- Creating new input components (`Dropdown`, `Input`, `TextArea`) where the props and default appearance wouldn't match our documentation. ### Global css selector From 3d9fa55bd764f5307805cbb0bff025aa05f59e8d Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 13:45:48 +0200 Subject: [PATCH 05/18] s --- rfcs/convergence/apperance-migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index cbe556b6ece1d0..5f07100b3cb81d 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -32,7 +32,7 @@ Partners could create a new composition component and modify the props in their - Easy to create - Would work in iframes -- Is safe +- Is relatively safe 馃憥 Cons: From b1aa5f380d887ff3ef90d9933b59d44869804e82 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 14:23:14 +0200 Subject: [PATCH 06/18] color --- rfcs/convergence/apperance-migration.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 5f07100b3cb81d..c53689e5b830fe 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -4,11 +4,11 @@ ## Summary -Fluent V9 changes the default appearance of input components (`Dropdown`, `Input`, `TextArea`). V0 input components default background colour is grey-ish, whereas in V9 the background colour is white. +Fluent V9 changes the default appearance of input components (`Dropdown`, `Input`, `TextArea`). V0 input components default background color is grey-ish, whereas in V9 the background color is white. ## Background -Partners could change the background colour to match the previous version of Fluent by passing a prop `appearance` with value `filled-darker`, but this solution does not scale well for partners when migrating to the new version and is error prone. +Partners could change the background color to match the previous version of Fluent by passing a prop `appearance` with value `filled-darker`, but this solution does not scale well for partners when migrating to the new version and is error prone. ## Problem statement @@ -26,7 +26,7 @@ Partners should also easily revert the decision to the default appearance value ### Composition component -Partners could create a new composition component and modify the props in their preferred way. If the partner would like to keep the original colour, they could create the composition component and have the default component without the appearance prop renders as `filled-darker`. +Partners could create a new composition component and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composition component and have the default component without the appearance prop renders as `filled-darker`. 馃憤 Pros: @@ -40,7 +40,7 @@ Partners could create a new composition component and modify the props in their ### Global css selector -Targeting all input selectors from a partner app and change the colour with global css. +Targeting all input selectors from a partner app and change the color with global css. 馃憥 Cons: From 803cf5d36a623c852701e642200e23a19320ba0b Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 14:29:59 +0200 Subject: [PATCH 07/18] update --- rfcs/convergence/apperance-migration.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index c53689e5b830fe..493602a22a6b7a 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -30,18 +30,22 @@ Partners could create a new composition component and modify the props in their 馃憤 Pros: -- Easy to create - Would work in iframes - Is relatively safe 馃憥 Cons: -- Creating new input components (`Dropdown`, `Input`, `TextArea`) where the props and default appearance wouldn't match our documentation. +- Creating new composition component for each input `Dropdown`, `Input`, `TextArea` and `DatePicker` +- Composition component apperance prop wouldn't match our Fluent V9 documentation ### Global css selector Targeting all input selectors from a partner app and change the color with global css. +馃憤 Pros: + +- Relatively easy and fast to do + 馃憥 Cons: - Wouldn鈥檛 work in iframes (?) @@ -53,7 +57,7 @@ Adding a new theme token that the partner could override the existing one. 馃憤 Pros: -- Easy to do ? +- Easy to do but creating for each input component (?) 馃憥 Cons: From dff18df1654c860cec6a8198d5ecd9e235fb155f Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 16:18:22 +0200 Subject: [PATCH 08/18] update --- rfcs/convergence/apperance-migration.md | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 493602a22a6b7a..81f12f7c8115c3 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -30,13 +30,14 @@ Partners could create a new composition component and modify the props in their 馃憤 Pros: -- Would work in iframes - Is relatively safe 馃憥 Cons: - Creating new composition component for each input `Dropdown`, `Input`, `TextArea` and `DatePicker` - Composition component apperance prop wouldn't match our Fluent V9 documentation +- Wouldn鈥檛 work in iframes +- Does not work if repos have dependencies of another project with Fluent V9 input components (Nova) ### Global css selector @@ -45,11 +46,12 @@ Targeting all input selectors from a partner app and change the color with globa 馃憤 Pros: - Relatively easy and fast to do +- Scalable 馃憥 Cons: -- Wouldn鈥檛 work in iframes (?) - Difficult to validate +- Wouldn鈥檛 work in iframes ### New token alias to theme @@ -62,21 +64,18 @@ Adding a new theme token that the partner could override the existing one. 馃憥 Cons: - Negative impact on performance by increasing variables (as read here: [fluentui/theme-shared-colors.md at d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351 路 microsoft/fluentui (github.com)](https://github.com/microsoft/fluentui/blob/d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351/rfcs/react-components/convergence/theme-shared-colors.md)) +- Divergince the themes +- Wouldn't work with iframes ### Unify design Discuss with designers to unify V0 and V9 design, setting the appearance to filled-dark by default. -馃憤 Props: Will make migration easier for partners who already uses V0 +馃憤 Props: -馃憥 Cons: Inherits old design/dependency that might not be relevant this time +- Will make migration easier for partners who already uses V0 +- Would work with iframes (if all the teams have the unified design) - - - - - - - +馃憥 Cons: - +- Inherits design from old V0 package that does not meet our needs/goals From f1c0e540302dd18c8f8fafa9ab97d64db5f99c22 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Tue, 2 Aug 2022 16:21:35 +0200 Subject: [PATCH 09/18] m --- rfcs/convergence/apperance-migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 81f12f7c8115c3..a18eff3f3eefae 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -64,7 +64,7 @@ Adding a new theme token that the partner could override the existing one. 馃憥 Cons: - Negative impact on performance by increasing variables (as read here: [fluentui/theme-shared-colors.md at d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351 路 microsoft/fluentui (github.com)](https://github.com/microsoft/fluentui/blob/d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351/rfcs/react-components/convergence/theme-shared-colors.md)) -- Divergince the themes +- Divergence themes from the original - Wouldn't work with iframes ### Unify design From 4bcf5f4392eabbb2d207336bedabbdbdeb2488d7 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Thu, 4 Aug 2022 11:09:51 +0200 Subject: [PATCH 10/18] Update rfcs/convergence/apperance-migration.md Co-authored-by: Miroslav Stastny --- rfcs/convergence/apperance-migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index a18eff3f3eefae..fa8d3b0c23d51a 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -24,7 +24,7 @@ Partners should also easily revert the decision to the default appearance value ## Detailed Design or Proposal -### Composition component +### Composed component Partners could create a new composition component and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composition component and have the default component without the appearance prop renders as `filled-darker`. From 5b8838bc33ff3058448b8d147548732be76dd84e Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Thu, 4 Aug 2022 11:10:06 +0200 Subject: [PATCH 11/18] Update rfcs/convergence/apperance-migration.md Co-authored-by: Miroslav Stastny --- rfcs/convergence/apperance-migration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index fa8d3b0c23d51a..fc6a40e0819023 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -38,6 +38,7 @@ Partners could create a new composition component and modify the props in their - Composition component apperance prop wouldn't match our Fluent V9 documentation - Wouldn鈥檛 work in iframes - Does not work if repos have dependencies of another project with Fluent V9 input components (Nova) +- Composition approach is currently marked as unstable in v9 ### Global css selector From 537026ffe91faf4ad28b169dec02ece20ed58fb6 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Thu, 4 Aug 2022 11:15:23 +0200 Subject: [PATCH 12/18] Update rfcs/convergence/apperance-migration.md Co-authored-by: Miroslav Stastny --- rfcs/convergence/apperance-migration.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index fc6a40e0819023..69fe05a530bedf 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -24,6 +24,8 @@ Partners should also easily revert the decision to the default appearance value ## Detailed Design or Proposal +### Wrap the library components on application side + ### Composed component Partners could create a new composition component and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composition component and have the default component without the appearance prop renders as `filled-darker`. From 6ddb6f419f7fb3b3fe2f928dd5f61333d27e8885 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Thu, 4 Aug 2022 11:50:40 +0200 Subject: [PATCH 13/18] token alias --- rfcs/convergence/apperance-migration.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 69fe05a530bedf..f86a76e2e5099f 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -58,17 +58,25 @@ Targeting all input selectors from a partner app and change the color with globa ### New token alias to theme -Adding a new theme token that the partner could override the existing one. +Adding a new alias color token (let's say `colorInputBackground`) and use it for all inputs. 馃憤 Pros: -- Easy to do but creating for each input component (?) +- Easy to do and would work for all inputs 馃憥 Cons: - Negative impact on performance by increasing variables (as read here: [fluentui/theme-shared-colors.md at d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351 路 microsoft/fluentui (github.com)](https://github.com/microsoft/fluentui/blob/d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351/rfcs/react-components/convergence/theme-shared-colors.md)) - Divergence themes from the original -- Wouldn't work with iframes +- Wouldn't work in iframes + +Another option is to add a possibility to override the background using a CSS variable: +We can use `backgroundColor: var(--inputBackgroundOverride, ${tokens.colorNeutralBackground1})` without setting the `--inputBackgroundOverride` anywhere. Then an application can set that variable if it needs to override the background. + +馃憤 no additional tokens unless needed + +馃憥 new concept +馃憥 one-off just for the Input background ### Unify design @@ -82,3 +90,4 @@ Discuss with designers to unify V0 and V9 design, setting the appearance to fill 馃憥 Cons: - Inherits design from old V0 package that does not meet our needs/goals +- According to the design team, this is currently a no-go From 4c074b3a25b0edf7476c20df90afd09a55bacbf6 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Wed, 10 Aug 2022 13:47:25 +0200 Subject: [PATCH 14/18] example --- rfcs/convergence/apperance-migration.md | 28 ++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index f86a76e2e5099f..bb3973e89a868d 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -24,11 +24,9 @@ Partners should also easily revert the decision to the default appearance value ## Detailed Design or Proposal -### Wrap the library components on application side - ### Composed component -Partners could create a new composition component and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composition component and have the default component without the appearance prop renders as `filled-darker`. +Partners could create a new composed component in fluent and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composed component and have the default component without the appearance prop renders as `filled-darker`. 馃憤 Pros: @@ -36,25 +34,41 @@ Partners could create a new composition component and modify the props in their 馃憥 Cons: -- Creating new composition component for each input `Dropdown`, `Input`, `TextArea` and `DatePicker` -- Composition component apperance prop wouldn't match our Fluent V9 documentation -- Wouldn鈥檛 work in iframes +- Creating new composed component for each input `Dropdown`, `Input`, `TextArea` and `DatePicker` +- Composed component apperance prop wouldn't match our Fluent V9 documentation - Does not work if repos have dependencies of another project with Fluent V9 input components (Nova) - Composition approach is currently marked as unstable in v9 +### Wrap the library components on application side + +Partners could a new component wrapper that will wrap input components and modify the appearance prop on the application side. + +馃憤 Pros: + +- Is safe and stable + +馃憥 Cons: + +- Creating new composed component for each input `Dropdown`, `Input`, `TextArea` and `DatePicker` +- Wrapper component apperance wouldn't match our Fluent V9 documentation +- Would work in iframes only within the TMP repository +- Does not work if repos have dependencies of another project with Fluent V9 input components (Nova) + ### Global css selector Targeting all input selectors from a partner app and change the color with global css. +Example: https://codesandbox.io/s/rfc-inputs-ew821q?file=/app.tsx + 馃憤 Pros: - Relatively easy and fast to do - Scalable +- Would work for dependencies such as Nova (but depending where we inject the CSS rule and if they have their own `FluentProvider`) 馃憥 Cons: - Difficult to validate -- Wouldn鈥檛 work in iframes ### New token alias to theme From efd09db99efd2ea1d4b3750ccadb4447448054e0 Mon Sep 17 00:00:00 2001 From: Petr Duda Date: Wed, 10 Aug 2022 13:56:09 +0200 Subject: [PATCH 15/18] iframes --- rfcs/convergence/apperance-migration.md | 1 - 1 file changed, 1 deletion(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index bb3973e89a868d..1596fb4f0db529 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -82,7 +82,6 @@ Adding a new alias color token (let's say `colorInputBackground`) and use it for - Negative impact on performance by increasing variables (as read here: [fluentui/theme-shared-colors.md at d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351 路 microsoft/fluentui (github.com)](https://github.com/microsoft/fluentui/blob/d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351/rfcs/react-components/convergence/theme-shared-colors.md)) - Divergence themes from the original -- Wouldn't work in iframes Another option is to add a possibility to override the background using a CSS variable: We can use `backgroundColor: var(--inputBackgroundOverride, ${tokens.colorNeutralBackground1})` without setting the `--inputBackgroundOverride` anywhere. Then an application can set that variable if it needs to override the background. From e8ba998a33ff109eb74f0e00b40483693444d3eb Mon Sep 17 00:00:00 2001 From: Miroslav Stastny Date: Mon, 12 Sep 2022 09:17:24 +0200 Subject: [PATCH 16/18] Apply suggestions from code review Co-authored-by: Oleksandr Fediashov --- rfcs/convergence/apperance-migration.md | 37 ++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 1596fb4f0db529..6770637ed4226b 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -1,6 +1,6 @@ # RFC: Appearance migration -@peta-duda +[@petr-duda](https://github.com/petr-duda) ## Summary @@ -28,6 +28,16 @@ Partners should also easily revert the decision to the default appearance value Partners could create a new composed component in fluent and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composed component and have the default component without the appearance prop renders as `filled-darker`. +#### Example + +```ts +export const Input: ForwardRefComponent = React.forwardRef((props, ref) => { + const state = useInput_unstable({ appearance: 'filled-darker', ...props }, ref); + + useInputStyles_unstable(state); + return renderInput_unstable(state); +}); +``` 馃憤 Pros: - Is relatively safe @@ -43,6 +53,14 @@ Partners could create a new composed component in fluent and modify the props in Partners could a new component wrapper that will wrap input components and modify the appearance prop on the application side. +#### Example + +```tsx +export const Input: ForwardRefComponent = React.forwardRef((props, ref) => { + return +}) +``` + 馃憤 Pros: - Is safe and stable @@ -50,6 +68,7 @@ Partners could a new component wrapper that will wrap input components and modif 馃憥 Cons: - Creating new composed component for each input `Dropdown`, `Input`, `TextArea` and `DatePicker` +- Wrapper components in React's i.e. bigger React tree - Wrapper component apperance wouldn't match our Fluent V9 documentation - Would work in iframes only within the TMP repository - Does not work if repos have dependencies of another project with Fluent V9 input components (Nova) @@ -104,3 +123,19 @@ Discuss with designers to unify V0 and V9 design, setting the appearance to fill - Inherits design from old V0 package that does not meet our needs/goals - According to the design team, this is currently a no-go + +### Use React context + +Another option is to use React Context to override `appearance` defaults in `FluentProvider`. + +#### Example + +```tsx +function App() { + return ( + + {/* has "filled-darker" */} + {/* has "underline" */} + + ) +} From e3484b4332e9bf51ac3de3171e071c07fa2ecc40 Mon Sep 17 00:00:00 2001 From: Miroslav Stastny Date: Mon, 12 Sep 2022 09:23:21 +0200 Subject: [PATCH 17/18] Address PR comments --- rfcs/convergence/apperance-migration.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index 6770637ed4226b..c1deeb6eac19ea 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -24,9 +24,9 @@ Partners should also easily revert the decision to the default appearance value ## Detailed Design or Proposal -### Composed component +### Compose components on application side -Partners could create a new composed component in fluent and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composed component and have the default component without the appearance prop renders as `filled-darker`. +Partners could create a new composed component on application side. and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composed component and have the default component without the appearance prop renders as `filled-darker`. #### Example @@ -38,6 +38,7 @@ export const Input: ForwardRefComponent = React.forwardRef((props, r return renderInput_unstable(state); }); ``` + 馃憤 Pros: - Is relatively safe @@ -57,8 +58,8 @@ Partners could a new component wrapper that will wrap input components and modif ```tsx export const Input: ForwardRefComponent = React.forwardRef((props, ref) => { - return -}) + return ; +}); ``` 馃憤 Pros: @@ -102,13 +103,15 @@ Adding a new alias color token (let's say `colorInputBackground`) and use it for - Negative impact on performance by increasing variables (as read here: [fluentui/theme-shared-colors.md at d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351 路 microsoft/fluentui (github.com)](https://github.com/microsoft/fluentui/blob/d5d510bf1ffcc1a4ed2067e9eb009c84e7beb351/rfcs/react-components/convergence/theme-shared-colors.md)) - Divergence themes from the original +### New optional token with fallback to a theme token + Another option is to add a possibility to override the background using a CSS variable: We can use `backgroundColor: var(--inputBackgroundOverride, ${tokens.colorNeutralBackground1})` without setting the `--inputBackgroundOverride` anywhere. Then an application can set that variable if it needs to override the background. 馃憤 no additional tokens unless needed 馃憥 new concept -馃憥 one-off just for the Input background +馃憥 one-off just for the Input background (although it would be used in multiple input components) ### Unify design @@ -139,3 +142,9 @@ function App() { ) } + +馃憤 more universal solution than a custom token - can be used to override different concepts, not only tokens - props, icons, etc. + +馃憥 new concept +馃憥 one-off just for the Input background (although it would be used in multiple input components) +``` From 5040d9c67c2ee237e7b174d065765cf98b5bbf80 Mon Sep 17 00:00:00 2001 From: Miroslav Stastny Date: Mon, 8 May 2023 14:28:34 -0700 Subject: [PATCH 18/18] Use react context --- rfcs/convergence/apperance-migration.md | 46 +++++++++++++------------ 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/rfcs/convergence/apperance-migration.md b/rfcs/convergence/apperance-migration.md index c1deeb6eac19ea..6f154b11256a8f 100644 --- a/rfcs/convergence/apperance-migration.md +++ b/rfcs/convergence/apperance-migration.md @@ -24,6 +24,30 @@ Partners should also easily revert the decision to the default appearance value ## Detailed Design or Proposal +### Use React context + +Another option is to use React Context to override `appearance` defaults in `FluentProvider`. + +#### Example + +```tsx +function App() { + return ( + + {/* has "filled-darker" */} + {/* has "underline" */} + + ) +} + +馃憤 more universal solution than a custom token - can be used to override different concepts, not only tokens - props, icons, etc. + +馃憥 new concept +馃憥 one-off just for the Input background (although it would be used in multiple input components) +``` + +## Discarded Solutions + ### Compose components on application side Partners could create a new composed component on application side. and modify the props in their preferred way. If the partner would like to keep the original color, they could create the composed component and have the default component without the appearance prop renders as `filled-darker`. @@ -126,25 +150,3 @@ Discuss with designers to unify V0 and V9 design, setting the appearance to fill - Inherits design from old V0 package that does not meet our needs/goals - According to the design team, this is currently a no-go - -### Use React context - -Another option is to use React Context to override `appearance` defaults in `FluentProvider`. - -#### Example - -```tsx -function App() { - return ( - - {/* has "filled-darker" */} - {/* has "underline" */} - - ) -} - -馃憤 more universal solution than a custom token - can be used to override different concepts, not only tokens - props, icons, etc. - -馃憥 new concept -馃憥 one-off just for the Input background (although it would be used in multiple input components) -```