-
Notifications
You must be signed in to change notification settings - Fork 2.9k
rfc: Global styles (normalize) #17561
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
Closed
miroslavstastny
wants to merge
2
commits into
microsoft:master
from
miroslavstastny:rfc/global-styles
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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,53 @@ | ||
| # RFC: Global styles | ||
|
|
||
| --- | ||
|
|
||
| @miroslavstastny | ||
|
|
||
| ## Summary | ||
|
|
||
| Fluent UI Converged components' styles expect global styles to be set. This RFC proposes how those global styles will be injected into DOM. | ||
|
|
||
| ## Background | ||
|
|
||
| There are two kinds of global (not owned by a component) styles which the converged components depend on - **CSS resets** and **Theme root styles**. | ||
|
|
||
| ## Detailed Design or Proposal | ||
|
|
||
| ### CSS resets | ||
|
|
||
| Converged components depend on [normalize.css](https://github.com/necolas/normalize.css/) for CSS resets. Fluent UI needs to provide a way how an application can inject the Normalize styles in each `document` instance. | ||
|
|
||
| Both v7/v8 and v0/Northstar components require `normalize.css` as well. | ||
|
|
||
| - v8 **A LINK FOR V8** | ||
| - v0 [normalizeCSS.ts](https://github.com/microsoft/fluentui/blob/31ff08b1d7ca1aded8f2f42d5d90fbb3146b5549/packages/fluentui/react-northstar/src/themes/teams/staticStyles/normalizeCSS.ts) | ||
|
|
||
| During a transition phase, converged components will be used side by side with existing components. In these cases `normalize.css` will be handled out of converged components, therefore converged must have a possibility to opt-out of injecting the Normalize. | ||
|
|
||
| **Proposal is to create a `CSSBaseline` component which will inject Normalize globally on the window.** | ||
|
|
||
| It is application's responsibility to instantiate the component wherever needed. | ||
|
|
||
| This is similar to [MaterialUI's CSSBaseline component](https://material-ui.com/components/css-baseline/). | ||
|
|
||
| ### Theme root styles | ||
|
|
||
| Theme global styles must be applied to the whole application (sub)tree the theme is applied to. | ||
| These are: | ||
|
|
||
| - foreground color | ||
| - background color | ||
| - font family, size and weight for default text | ||
|
|
||
| As Fluent UI supports multiple themes to coexist both side-by-side and nested, these Theme root styles cannot be applied globally. | ||
|
|
||
| Each `Provider` already creates a `div` and injects tokens as CSS variables on this div. | ||
|
|
||
| **Proposal is for the `Provider` to set theme root styles on this div as well.** | ||
|
|
||
| ## Open Issues | ||
|
|
||
| - Where will the `CSSBaseline` component take the `window` from? If from context, then it must be inside a `Provider` tree. | ||
|
|
||
| - If there will be a requirement to have element styles in `Theme root styles` (like font weight for `<b>`), how would we do that on a provider div? (as `.abcd b`?) | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that
CSSBaselineshould be rendered inside aProvideralways: it's required bymakeStaticStyles()to render into properdocument.