BREAKING(Text): change font size scheme and Text size API#214
BREAKING(Text): change font size scheme and Text size API#214levithomason merged 11 commits intomasterfrom
Conversation
miroslavstastny
left a comment
There was a problem hiding this comment.
By this change, you are defining a contract for all themes that they must define their sizes in fontSizes.
How do you plan to enforce it? It should be at least added to ISiteVariables
| render={({ siteVariables }) => { | ||
| return _.map(siteVariables.fontSizes, (value, key) => ( | ||
| <div key={key}> | ||
| <Text size={key} content="Dicta voluptatum dolorem." /> |
There was a problem hiding this comment.
I would also add the size to the content so I can immediately see the size names in docsite.
Codecov Report
@@ Coverage Diff @@
## master #214 +/- ##
==========================================
- Coverage 91.45% 91.37% -0.08%
==========================================
Files 63 63
Lines 1158 1160 +2
Branches 153 176 +23
==========================================
+ Hits 1059 1060 +1
- Misses 95 96 +1
Partials 4 4
Continue to review full report at Codecov.
|
levithomason
left a comment
There was a problem hiding this comment.
We will consider how to enforce this with typings later. This topic also warrants an RFC on /specifications for how we deal with font sizes and component sizes at the framework API level. The changes here are the result of many conversations and ideas with Fabric, teams around MS, and the open source. We chose a middle ground path for sizes for now. We are open to future changes.
Before
After
Was
Now