Text - Reusable make-styles rules by theerebuss · Pull Request #18778 · microsoft/fluentui

theerebuss · 2021-06-30T15:03:32Z

Pull request checklist

~~- [ ] Include a change request file using $ yarn change~~ Package not published

Description of changes

Add the reusable make-styles rules for the Typography variants:

Caption
Body
Subheadline
Headline
Title 3
Title 2
Title 1
Large Title
Display

The Text spec was also amended to reflect the real implementation of the rules.

Blocked by #18920

size-auditor · 2021-06-30T15:13:55Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: bafb9f17e40291fef604725051aae526ba7345a6 (build)

codesandbox-ci · 2021-06-30T15:14:27Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9e4e616:

Sandbox	Source
Fluent UI React Starter	Configuration

fabricteam · 2021-06-30T15:27:48Z

Perf Analysis (`@fluentui/react`)

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
DocumentCardTitle	mount	137	131	1000	Possible regression

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
Avatar	mount	804	774	5000
BaseButton	mount	887	906	5000
Breadcrumb	mount	2614	2609	1000
ButtonNext	mount	534	526	5000
Checkbox	mount	1508	1487	5000
CheckboxBase	mount	1278	1266	5000
ChoiceGroup	mount	4667	4674	5000
ComboBox	mount	975	994	1000
CommandBar	mount	10076	10109	1000
ContextualMenu	mount	6230	6243	1000
DefaultButton	mount	1122	1105	5000
DetailsRow	mount	3778	3689	5000
DetailsRowFast	mount	3685	3683	5000
DetailsRowNoStyles	mount	3470	3507	5000
Dialog	mount	2169	2118	1000
DocumentCardTitle	mount	137	131	1000	Possible regression
Dropdown	mount	3200	3174	5000
FluentProviderNext	mount	7202	7240	5000
FocusTrapZone	mount	1775	1764	5000
FocusZone	mount	1813	1787	5000
IconButton	mount	1737	1745	5000
Label	mount	344	334	5000
Layer	mount	1770	1747	5000
Link	mount	466	458	5000
MakeStyles	mount	1827	1829	50000
MenuButton	mount	1442	1444	5000
MessageBar	mount	2020	2016	5000
Nav	mount	3194	3222	1000
OverflowSet	mount	1047	1051	5000
Panel	mount	2063	2069	1000
Persona	mount	807	825	1000
Pivot	mount	1400	1403	1000
PrimaryButton	mount	1265	1256	5000
Rating	mount	7527	7520	5000
SearchBox	mount	1303	1298	5000
Shimmer	mount	2471	2454	5000
Slider	mount	1940	1939	5000
SpinButton	mount	4906	4928	5000
Spinner	mount	422	415	5000
SplitButton	mount	3130	3084	5000
Stack	mount	501	493	5000
StackWithIntrinsicChildren	mount	1479	1492	5000
StackWithTextChildren	mount	4450	4412	5000
SwatchColorPicker	mount	10012	10015	5000
Tabs	mount	1373	1400	1000
TagPicker	mount	2389	2356	5000
TeachingBubble	mount	11770	11816	5000
Text	mount	420	407	5000
TextField	mount	1339	1354	5000
ThemeProvider	mount	1181	1199	5000
ThemeProvider	virtual-rerender	601	600	5000
Toggle	mount	803	777	5000
buttonNative	mount	114	110	5000

Perf Analysis (`@fluentui/react-northstar`)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AttachmentMinimalPerf.default	160	145	1.1:1
RefMinimalPerf.default	242	230	1.05:1
ButtonMinimalPerf.default	163	157	1.04:1
CardMinimalPerf.default	556	536	1.04:1
SegmentMinimalPerf.default	348	336	1.04:1
TextMinimalPerf.default	340	328	1.04:1
TreeWith60ListItems.default	170	163	1.04:1
BoxMinimalPerf.default	347	336	1.03:1
CarouselMinimalPerf.default	462	447	1.03:1
GridMinimalPerf.default	330	320	1.03:1
ListCommonPerf.default	614	595	1.03:1
PopupMinimalPerf.default	600	582	1.03:1
PortalMinimalPerf.default	180	175	1.03:1
RadioGroupMinimalPerf.default	445	432	1.03:1
TreeMinimalPerf.default	794	773	1.03:1
AnimationMinimalPerf.default	412	403	1.02:1
ChatMinimalPerf.default	643	631	1.02:1
DividerMinimalPerf.default	362	354	1.02:1
DropdownManyItemsPerf.default	676	666	1.02:1
DropdownMinimalPerf.default	3087	3041	1.02:1
ListWith60ListItems.default	660	645	1.02:1
ButtonOverridesMissPerf.default	1670	1661	1.01:1
FlexMinimalPerf.default	278	274	1.01:1
HeaderMinimalPerf.default	354	350	1.01:1
InputMinimalPerf.default	1249	1233	1.01:1
ItemLayoutMinimalPerf.default	1211	1194	1.01:1
ListMinimalPerf.default	505	498	1.01:1
MenuMinimalPerf.default	831	820	1.01:1
ReactionMinimalPerf.default	368	366	1.01:1
SplitButtonMinimalPerf.default	3725	3683	1.01:1
StatusMinimalPerf.default	674	670	1.01:1
IconMinimalPerf.default	591	587	1.01:1
TableManyItemsPerf.default	1877	1856	1.01:1
TooltipMinimalPerf.default	993	986	1.01:1
VideoMinimalPerf.default	608	604	1.01:1
AlertMinimalPerf.default	260	261	1:1
AvatarMinimalPerf.default	189	189	1:1
ChatDuplicateMessagesPerf.default	277	278	1:1
DialogMinimalPerf.default	753	750	1:1
FormMinimalPerf.default	388	387	1:1
HeaderSlotsPerf.default	741	742	1:1
LabelMinimalPerf.default	374	373	1:1
ListNestedPerf.default	542	543	1:1
LoaderMinimalPerf.default	683	685	1:1
MenuButtonMinimalPerf.default	1607	1609	1:1
ProviderMergeThemesPerf.default	1652	1652	1:1
SliderMinimalPerf.default	1513	1514	1:1
ToolbarMinimalPerf.default	921	917	1:1
ButtonSlotsPerf.default	531	539	0.99:1
CheckboxMinimalPerf.default	2708	2738	0.99:1
EmbedMinimalPerf.default	4018	4053	0.99:1
LayoutMinimalPerf.default	347	352	0.99:1
ProviderMinimalPerf.default	950	956	0.99:1
TextAreaMinimalPerf.default	479	484	0.99:1
CustomToolbarPrototype.default	3749	3776	0.99:1
DatepickerMinimalPerf.default	5227	5346	0.98:1
ImageMinimalPerf.default	361	367	0.98:1
SkeletonMinimalPerf.default	336	344	0.98:1
AttachmentSlotsPerf.default	1014	1042	0.97:1
TableMinimalPerf.default	396	408	0.97:1
RosterPerf.default	1106	1151	0.96:1
AccordionMinimalPerf.default	141	149	0.95:1
ChatWithPopoverPerf.default	336	360	0.93:1

Co-authored-by: Tringa Krasniqi <tkrasniqi@microsoft.com>

Change lib to ES2015 Co-authored-by: Tringa Krasniqi <tkrasniqi@microsoft.com>

layershifter · 2021-07-09T10:20:29Z

@andrefcdias is there a vision how usage of these mixins will be enforced in our components? Currently nothing stops me from defining styles directly in a component:

  root: theme => ({
    // next styles
    fontFamily: theme.global.type.fontFamilies.base,
    fontSize: theme.global.type.fontSizes.base[200],
    lineHeight: theme.global.type.lineHeights.base[200],
    fontWeight: theme.global.type.fontWeights.regular,

    // component styles
    color: 'red',
  }),

theerebuss · 2021-07-09T15:45:47Z

@andrefcdias is there a vision how usage of these mixins will be enforced in our components? Currently nothing stops me from defining styles directly in a component:

  root: theme => ({
    // next styles
    fontFamily: theme.global.type.fontFamilies.base,
    fontSize: theme.global.type.fontSizes.base[200],
    lineHeight: theme.global.type.lineHeights.base[200],
    fontWeight: theme.global.type.fontWeights.regular,

    // component styles
    color: 'red',
  }),

@layershifter The purpose of these are not for enforcing styles but a quick way for you to apply them and be compliant with design, without having to check the spec.

ling1726

LGTM, can you please add a bundle-size fixture for the styles to make sure we don't regress ?

Take a look here for an example: https://github.com/microsoft/fluentui/tree/master/packages/react-menu/bundle-size

fabricteam · 2021-07-15T10:50:46Z

📊 Bundle size report

_{🤖 This report was generated against bafb9f17e40291fef604725051aae526ba7345a6}

Hotell · 2021-07-16T08:43:56Z

@andrefcdias is there a vision how usage of these mixins will be enforced in our components? Currently nothing stops me from defining styles directly in a component:

  root: theme => ({
    // next styles
    fontFamily: theme.global.type.fontFamilies.base,
    fontSize: theme.global.type.fontSizes.base[200],
    lineHeight: theme.global.type.lineHeights.base[200],
    fontWeight: theme.global.type.fontWeights.regular,

    // component styles
    color: 'red',
  }),

AFAIR the conclusion on tech sync was to use Text component directly within our components.
we could provide some lint rule but I'd say that it might not work properly as the conditions might be way too broad

miroslavstastny · 2021-07-16T09:06:55Z

AFAIR the conclusion on tech sync was to use Text component directly within our components.

Sorry for (my) confusion, there must have been a miscommunication somewhere :-( I think reusing basic components in bigger FUI components is not a good pattern going forward.

I don't like reopening discussions but would you mind discussing it once more?

@layershifter what do you think? Can you share your opinion and pros and cons of the approaches?

theerebuss · 2021-07-16T09:22:46Z

I would suggest that an RFC is spun to decide if we should/should not reuse our own components in other components as it's not a Text specific "issue".
My own opinion is that the more we reuse functionality, the better to maintain it.
The only problems I see might be with dependencies. However, specifically to Text, using the make-styles rules directly, Text or wrappers, will always require a dependency on react-text.

Hotell · 2021-07-16T09:28:07Z

also from what I remember (that might be obsolete info - no RFC/style-guide yet) Text will handle RTL in future ? meaning, if you use only these "variant" styles, you won't get rtl support (thats something we wanna avoid)

theerebuss · 2021-07-16T09:32:10Z

There's an open RFC (#18921) regarding dir prop for Text but RTL should be handled by the browser by default and dir is a globally available prop.

Co-authored-by: Tringa Krasniqi <tkrasniqi@microsoft.com>

Raw double implementation for testing

9a3985d

msft-fluent-ui-bot assigned miroslavstastny Jun 30, 2021

theerebuss assigned tringakrasniqi and theerebuss and unassigned miroslavstastny Jun 30, 2021

theerebuss added the Component: Text label Jun 30, 2021

theerebuss mentioned this pull request Jun 30, 2021

Text Convergence #16847

Closed

44 tasks

André Dias and others added 3 commits July 2, 2021 12:12

Remove alternative implementation

e37e97c

Ammend spec

8bd9d24

Move typographyStyles to root folder

e2bcf19

Co-authored-by: Tringa Krasniqi <tkrasniqi@microsoft.com>

theerebuss marked this pull request as ready for review July 2, 2021 10:52

theerebuss requested review from a team July 2, 2021 10:53

André Dias and others added 3 commits July 2, 2021 12:56

Fix imports

d210fff

Co-authored-by: Tringa Krasniqi <tkrasniqi@microsoft.com>

Add docs for typographyStyles

c9cbeb1

Change lib to ES2015 Co-authored-by: Tringa Krasniqi <tkrasniqi@microsoft.com>

Merge branch 'master' into typography-hook-implementation

bc2106c

microsoft deleted a comment from azure-pipelines Bot Jul 2, 2021

ling1726 reviewed Jul 2, 2021

View reviewed changes

Comment thread packages/react-text/src/typographyStyles/typographyStyles.ts Outdated

theerebuss mentioned this pull request Jul 2, 2021

RFC: Simplified component structure for react-text wrappers #18817

Merged

layershifter reviewed Jul 9, 2021

View reviewed changes

Comment thread packages/react-text/src/typographyStyles/typographyStyles.ts Outdated

Separate typographyStyles into multiple exports

1ee3883

theerebuss requested review from jurokapsiar, miroslavstastny and pompomon as code owners July 9, 2021 13:46

theerebuss removed request for jurokapsiar, miroslavstastny and pompomon July 9, 2021 15:47

theerebuss added this to the July Project Cycle Q2 2021 milestone Jul 9, 2021

Fix test import

76ca9bd

theerebuss mentioned this pull request Jul 9, 2021

Update API Extractor version #18885

Closed

1 task

ling1726 mentioned this pull request Jul 12, 2021

feat(keyboard-keys): Export keys and legacy keyCodes #18896

Merged

2 tasks

theerebuss mentioned this pull request Jul 12, 2021

Support for export * as namespace from localpackage from API Extractor #18906

Closed

Implement temporary alternative until #18906 is fixed

75755f2

theerebuss added the Status: Blocked Resolution blocked by another issue label Jul 12, 2021

ecraig12345 mentioned this pull request Jul 13, 2021

Update API Extractor (take 2) #18920

Merged

1 task

tringakrasniqi requested review from layershifter and ling1726 July 15, 2021 10:01

ling1726 approved these changes Jul 15, 2021

View reviewed changes

Merge branch 'master' into typography-hook-implementation

1cb1549

Update API

e0220ee

theerebuss requested a review from a team as a code owner July 15, 2021 12:31

Hotell reviewed Jul 15, 2021

View reviewed changes

Comment thread packages/react-text/tsconfig.json

Hotell reviewed Jul 15, 2021

View reviewed changes

Comment thread packages/react-text/src/typographyStyles/typographyStyles.test.ts Outdated

Cleanup test

9e4e616

theerebuss removed the Status: Blocked Resolution blocked by another issue label Jul 15, 2021

theerebuss merged commit dbb070e into microsoft:master Jul 15, 2021

Hotell added the Fluent UI react-components (v9) label Jul 16, 2021

PeterDraex pushed a commit to PeterDraex/fluentui that referenced this pull request Aug 6, 2021

Text - Reusable make-styles rules (microsoft#18778)

00b6555

Co-authored-by: Tringa Krasniqi <tkrasniqi@microsoft.com>

Conversation

theerebuss commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Uh oh!

size-auditor Bot commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

codesandbox-ci Bot commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabricteam commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react)

Perf Analysis (@fluentui/react-northstar)

Uh oh!

Uh oh!

Uh oh!

layershifter commented Jul 9, 2021

Uh oh!

theerebuss commented Jul 9, 2021

Uh oh!

ling1726 left a comment

Choose a reason for hiding this comment

Uh oh!

fabricteam commented Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

Uh oh!

Uh oh!

Hotell commented Jul 16, 2021

Uh oh!

miroslavstastny commented Jul 16, 2021

Uh oh!

theerebuss commented Jul 16, 2021

Uh oh!

Hotell commented Jul 16, 2021

Uh oh!

theerebuss commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

theerebuss commented Jun 30, 2021 •

edited

Loading

size-auditor Bot commented Jun 30, 2021 •

edited

Loading

codesandbox-ci Bot commented Jun 30, 2021 •

edited

Loading

fabricteam commented Jun 30, 2021 •

edited

Loading

Perf Analysis (`@fluentui/react`)

Perf Analysis (`@fluentui/react-northstar`)

fabricteam commented Jul 15, 2021 •

edited

Loading

theerebuss commented Jul 16, 2021 •

edited

Loading