-
Notifications
You must be signed in to change notification settings - Fork 2.9k
RFC appearance migration #24181
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
Merged
Merged
RFC appearance migration #24181
Changes from 9 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
de90f8f
i
286ec19
appearance
19001b4
up
d136f9e
txtarea
3d9fa55
s
b1aa5f3
color
803cf5d
update
dff18df
update
f1c0e54
m
4bcf5f4
Update rfcs/convergence/apperance-migration.md
petdud 5b8838b
Update rfcs/convergence/apperance-migration.md
petdud 537026f
Update rfcs/convergence/apperance-migration.md
petdud 6ddb6f4
token alias
4c074b3
example
efd09db
iframes
e8ba998
Apply suggestions from code review
miroslavstastny e3484b4
Address PR comments
miroslavstastny d744cd7
Merge remote-tracking branch 'upstream/master' into rfc-input
miroslavstastny 5040d9c
Use react context
miroslavstastny 913adc9
Merge remote-tracking branch 'upstream/master' into rfc-input
miroslavstastny File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| # RFC: Appearance migration | ||
|
|
||
| @peta-duda | ||
|
|
||
| ## Summary | ||
|
|
||
| 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 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 | ||
|
|
||
| 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 | ||
|
|
||
|
petdud marked this conversation as resolved.
|
||
| ### Composition component | ||
|
petdud marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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`. | ||
|
|
||
|
miroslavstastny marked this conversation as resolved.
|
||
| 👍 Pros: | ||
|
|
||
| - 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’t work in iframes | ||
|
petdud marked this conversation as resolved.
Outdated
|
||
| - Does not work if repos have dependencies of another project with Fluent V9 input components (Nova) | ||
|
|
||
|
petdud marked this conversation as resolved.
|
||
| ### Global css selector | ||
|
|
||
| Targeting all input selectors from a partner app and change the color with global css. | ||
|
petdud marked this conversation as resolved.
|
||
|
|
||
| 👍 Pros: | ||
|
|
||
| - Relatively easy and fast to do | ||
| - Scalable | ||
|
|
||
| 👎 Cons: | ||
|
|
||
| - Difficult to validate | ||
| - Wouldn’t work in iframes | ||
|
petdud marked this conversation as resolved.
Outdated
|
||
|
|
||
| ### New token alias to theme | ||
|
petdud marked this conversation as resolved.
|
||
|
|
||
| Adding a new theme token that the partner could override the existing one. | ||
|
|
||
| 👍 Pros: | ||
|
|
||
| - Easy to do but creating for each input component (?) | ||
|
|
||
| 👎 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)) | ||
|
miroslavstastny marked this conversation as resolved.
|
||
| - Divergence themes from the original | ||
| - 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 | ||
| - 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 | ||
|
petdud marked this conversation as resolved.
|
||
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.
Uh oh!
There was an error while loading. Please reload this page.