chore: refactor styles for Button #25216

layershifter · 2022-10-13T13:57:58Z

What changes?

This PR refactors Button component to use makeResetStyles. The goal is to improve layouting performance by reducing an amount of applied CSS classes to an element.

The goal is to apply this change to all components (I am working on RFC to explain details), for simplicity:

all styles that should be in base (default state) should be defined with makeResetStyles
all styles for overrides (permutations for a component i.e. disabled, appearance) are defined with makeStyles

Notes

Only one classname generated by makeResetStyles can be applied to a slot
makeResetStyles allows to use CSS shorthands
makeResetStyles supports AOT and CSS extraction as makeStyles

Results

Case	Before (amount of classes on `button` element)	After
`<Button />`	97	2
`<Button appearance="primary" />`	97	28
`<Button disabled />`	104	47
`<Button appearance="primary" disabled />`	104	54

codesandbox-ci · 2022-10-13T14:10:03Z

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 acc7525:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

size-auditor · 2022-10-13T14:11:32Z

Asset size changes

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

Baseline commit: 388133213c27c5b72276621b64c1eff46504d971 (build)

fabricteam · 2022-10-13T14:16:36Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`react-alert` Alert	`86.891 kB` `21.708 kB`	`82.967 kB` `21.022 kB`	`-3.924 kB` `-686 B`
`react-button` Button	`36.86 kB` `9.913 kB`	`32.923 kB` `8.61 kB`	`-3.937 kB` `-1.303 kB`
`react-button` CompoundButton	`43.885 kB` `11.131 kB`	`39.953 kB` `9.933 kB`	`-3.932 kB` `-1.198 kB`
`react-button` MenuButton	`41.537 kB` `11.187 kB`	`37.611 kB` `9.942 kB`	`-3.926 kB` `-1.245 kB`
`react-button` SplitButton	`48.983 kB` `12.588 kB`	`45.057 kB` `11.323 kB`	`-3.926 kB` `-1.265 kB`
`react-button` ToggleButton	`52.685 kB` `11.415 kB`	`48.753 kB` `10.715 kB`	`-3.932 kB` `-700 B`
`react-components` react-components: Button, FluentProvider & webLightTheme	`62.975 kB` `17.687 kB`	`59.018 kB` `16.371 kB`	`-3.957 kB` `-1.316 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`190.411 kB` `53.077 kB`	`186.456 kB` `52.327 kB`	`-3.955 kB` `-750 B`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-accordion` Accordion (including children components)	`79.294 kB` `24.443 kB`
`react-avatar` Avatar	`51.076 kB` `14.285 kB`
`react-avatar` AvatarGroup	`15.006 kB` `6.009 kB`
`react-avatar` AvatarGroupItem	`67.306 kB` `18.581 kB`
`react-card` Card - All	`71.873 kB` `20.936 kB`
`react-card` Card	`67.651 kB` `19.892 kB`
`react-card` CardFooter	`8.617 kB` `3.62 kB`
`react-card` CardHeader	`9.792 kB` `3.995 kB`
`react-card` CardPreview	`8.718 kB` `3.674 kB`
`react-combobox` Combobox (including child components)	`78.244 kB` `25.015 kB`
`react-combobox` Dropdown (including child components)	`77.531 kB` `24.927 kB`
`react-components` react-components: FluentProvider & webLightTheme	`33.48 kB` `11.037 kB`
`react-dialog` Dialog (including children components)	`83.036 kB` `24.754 kB`
`react-infobutton` InfoButton	`117.922 kB` `35.391 kB`
`react-link` Link	`11.862 kB` `4.885 kB`
`react-menu` Menu (including children components)	`117.61 kB` `36.287 kB`
`react-menu` Menu (including selectable components)	`120.679 kB` `36.82 kB`
`react-persona` Persona	`57.131 kB` `15.951 kB`
`react-popover` Popover	`103.666 kB` `31.858 kB`
`react-portal` Portal	`10.495 kB` `3.851 kB`
`react-portal-compat` PortalCompatProvider	`5.857 kB` `1.978 kB`
`react-provider` FluentProvider	`15.817 kB` `5.905 kB`
`react-radio` Radio	`36.446 kB` `12.123 kB`
`react-radio` RadioGroup	`14.304 kB` `5.72 kB`
`react-slider` Slider	`32.118 kB` `10.192 kB`
`react-switch` Switch	`33.453 kB` `10.581 kB`
`react-tooltip` Tooltip	`42.032 kB` `14.739 kB`

_{🤖 This report was generated against 388133213c27c5b72276621b64c1eff46504d971}

fabricteam · 2022-10-13T14:17:48Z

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

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	1306	1347	5000
Button	mount	950	931	5000
FluentProvider	mount	1624	1597	5000
FluentProviderWithTheme	mount	636	632	10
FluentProviderWithTheme	virtual-rerender	603	594	10
FluentProviderWithTheme	virtual-rerender-with-unmount	628	633	10
MakeStyles	mount	1910	1933	50000
SpinButton	mount	2552	2617	5000

fabricteam · 2022-10-13T14:21:13Z

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

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
RefMinimalPerf.default	243	218	1.11:1
ButtonMinimalPerf.default	196	181	1.08:1
SkeletonMinimalPerf.default	398	367	1.08:1
TableMinimalPerf.default	459	426	1.08:1
TextAreaMinimalPerf.default	582	538	1.08:1
DividerMinimalPerf.default	411	383	1.07:1
FlexMinimalPerf.default	317	295	1.07:1
FormMinimalPerf.default	443	414	1.07:1
BoxMinimalPerf.default	385	363	1.06:1
ButtonSlotsPerf.default	632	600	1.05:1
CardMinimalPerf.default	615	587	1.05:1
LabelMinimalPerf.default	439	420	1.05:1
CarouselMinimalPerf.default	514	496	1.04:1
ChatDuplicateMessagesPerf.default	297	285	1.04:1
PortalMinimalPerf.default	171	164	1.04:1
GridMinimalPerf.default	387	376	1.03:1
PopupMinimalPerf.default	680	662	1.03:1
RadioGroupMinimalPerf.default	495	481	1.03:1
SegmentMinimalPerf.default	386	374	1.03:1
TooltipMinimalPerf.default	2595	2520	1.03:1
AttachmentMinimalPerf.default	172	169	1.02:1
AvatarMinimalPerf.default	210	205	1.02:1
DropdownMinimalPerf.default	2857	2803	1.02:1
InputMinimalPerf.default	1249	1225	1.02:1
ItemLayoutMinimalPerf.default	1314	1282	1.02:1
SplitButtonMinimalPerf.default	4833	4759	1.02:1
TreeMinimalPerf.default	889	874	1.02:1
VideoMinimalPerf.default	842	828	1.02:1
AttachmentSlotsPerf.default	1203	1191	1.01:1
ChatMinimalPerf.default	801	795	1.01:1
CheckboxMinimalPerf.default	2291	2268	1.01:1
EmbedMinimalPerf.default	4051	4030	1.01:1
ImageMinimalPerf.default	440	436	1.01:1
ListNestedPerf.default	619	611	1.01:1
MenuMinimalPerf.default	925	913	1.01:1
RosterPerf.default	2379	2351	1.01:1
ReactionMinimalPerf.default	430	427	1.01:1
ButtonOverridesMissPerf.default	1445	1442	1:1
DatepickerMinimalPerf.default	6206	6187	1:1
HeaderMinimalPerf.default	387	387	1:1
HeaderSlotsPerf.default	844	844	1:1
SliderMinimalPerf.default	1750	1749	1:1
TableManyItemsPerf.default	2132	2130	1:1
CustomToolbarPrototype.default	2793	2781	1:1
ChatWithPopoverPerf.default	396	401	0.99:1
DialogMinimalPerf.default	823	834	0.99:1
LayoutMinimalPerf.default	391	394	0.99:1
ListMinimalPerf.default	564	568	0.99:1
MenuButtonMinimalPerf.default	1837	1856	0.99:1
StatusMinimalPerf.default	746	750	0.99:1
TextMinimalPerf.default	367	371	0.99:1
ToolbarMinimalPerf.default	1008	1014	0.99:1
DropdownManyItemsPerf.default	747	759	0.98:1
ListWith60ListItems.default	673	684	0.98:1
LoaderMinimalPerf.default	702	715	0.98:1
ProviderMergeThemesPerf.default	1298	1329	0.98:1
ProviderMinimalPerf.default	410	420	0.98:1
TreeWith60ListItems.default	169	173	0.98:1
AnimationMinimalPerf.default	561	579	0.97:1
ListCommonPerf.default	678	701	0.97:1
AccordionMinimalPerf.default	157	163	0.96:1
IconMinimalPerf.default	718	754	0.95:1
AlertMinimalPerf.default	276	299	0.92:1

fabricteam · 2022-10-13T14:44:18Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
BaseButton	mount	1201	1210	5000
Breadcrumb	mount	2772	2784	1000
Checkbox	mount	2623	2660	5000
CheckboxBase	mount	2363	2363	5000
ChoiceGroup	mount	4240	4278	5000
ComboBox	mount	1166	1179	1000
CommandBar	mount	9282	9275	1000
ContextualMenu	mount	10129	10133	1000
DefaultButton	mount	1358	1369	5000
DetailsRow	mount	3392	3381	5000
DetailsRowFast	mount	3398	3387	5000
DetailsRowNoStyles	mount	3256	3220	5000
Dialog	mount	2910	2978	1000
DocumentCardTitle	mount	570	584	1000
Dropdown	mount	3167	3177	5000
FocusTrapZone	mount	1950	1957	5000
FocusZone	mount	1938	1926	5000
GroupedList	mount	48466	53144	2
GroupedList	virtual-rerender	22878	23119	2
GroupedList	virtual-rerender-with-unmount	81374	82522	2
GroupedListV2	mount	552	567	2
GroupedListV2	virtual-rerender	530	530	2
GroupedListV2	virtual-rerender-with-unmount	552	547	2
IconButton	mount	1790	1795	5000
Label	mount	746	761	5000
Layer	mount	4135	4147	5000
Link	mount	846	859	5000
MenuButton	mount	1629	1599	5000
MessageBar	mount	2413	2339	5000
Nav	mount	3071	3068	1000
OverflowSet	mount	1407	1427	5000
Panel	mount	2491	2487	1000
Persona	mount	1238	1252	1000
Pivot	mount	1526	1540	1000
PrimaryButton	mount	1488	1492	5000
Rating	mount	7006	7019	5000
SearchBox	mount	1493	1495	5000
Shimmer	mount	2932	2911	5000
Slider	mount	2100	2111	5000
SpinButton	mount	4312	4287	5000
Spinner	mount	810	825	5000
SplitButton	mount	2846	2851	5000
Stack	mount	858	857	5000
StackWithIntrinsicChildren	mount	2191	2214	5000
StackWithTextChildren	mount	4894	4896	5000
SwatchColorPicker	mount	9495	9582	5000
TagPicker	mount	2315	2311	5000
TeachingBubble	mount	74702	74543	5000
Text	mount	816	826	5000
TextField	mount	1538	1556	5000
ThemeProvider	mount	1458	1444	5000
ThemeProvider	virtual-rerender	1136	1144	5000
ThemeProvider	virtual-rerender-with-unmount	2005	1989	5000
Toggle	mount	1140	1125	5000
buttonNative	mount	541	526	5000

Hotell · 2022-11-02T11:22:39Z

the perf improvements thanks to this are amazing, I'm bit worried about DX though.

What's the plan in terms of enforcing/guiding devs by tooling when use makeStyles vs makeResetStyles ?

layershifter · 2022-11-02T11:28:07Z

the perf improvements thanks to this are amazing, I'm bit worried about DX though.
What's the plan in terms of enforcing/guiding devs by tooling when use makeStyles vs makeResetStyles ?

Yeah, I am also worried and it's a reason why I was pushing myself against it 😸 (well, there are no other options)

As I mentioned in PR's description: the plan is to have guidelines around our styling (when use what, how to write styles, etc.) and linting as an actionable step. For linting: rules are a bit unclear, but I will try to come with something reasonable.

packages/react-components/react-button/src/components/Button/useButtonStyles.ts

spmonahan · 2022-11-07T16:27:36Z

Ran some style recalc performance tests with stress-test:

MS Edge

DOM Size (num nodes in tree)	makeStyles Edge (ms)	makeResetStyles Edge (ms)	Change Edge (%)
250 (xs)	35.8	29.8	-16.75
500 (s)	44.7	34.3	-23.26
1000 (m)	82.7	62	-25.03
2000 (l)	127.5	90.2	-29.25
4000 (xl)	299.8	215.1	-28.25

Mozilla Firefox

DOM Size (num nodes in tree)	makeStyles Firefox(ms)	makeResetStyles Firefox (ms)	Change Firefox(%)
250 (xs)	17	15	-11.76
500 (s)	29	26	-10.34
1000 (m)	40	34	-15
2000 (l)	61	53	-13.11
4000 (xl)	116	96	-17.24

jspurlin

fabricteam · 2022-11-23T11:51:38Z

🕵 fluentuiv9 Open the Visual Regressions report to inspect the 2 screenshots

✅ There was 0 screenshots added, 0 screenshots removed, 1752 screenshots unchanged, 0 screenshots with different dimensions and 2 screenshots with visible difference.

unknown 2 screenshots

Image Name	Diff(in Pixels)	Image Type
Avatar Converged.size+active+badge.normal.chromium.png	13	Changed
Avatar Converged.size+active+ring-shadow.normal.chromium.png	2	Changed

packages/react-components/react-components/src/index.ts

packages/react-components/react-tabster/src/focus/createCustomFocusIndicatorStyle.ts

Co-authored-by: Makoto Morimoto <[email protected]>

* master: BREAKING: `useTable` renamed to `useTableFeatures` (microsoft#25797) chore: add retries for navigation in ssr-tests-v9 (microsoft#25844) fix: Cell actions should have correct background when row focused within (microsoft#25790) applying package updates Disable 3 Avatar Converged active stories (microsoft#25765) chore: introduce TS path aliases for improved DX in v8 (microsoft#25778) chore: prepare release react-northstar 0.65.0 (microsoft#25446) refactor(scripts): encapsulate v0 and v8 tooling within its domain boundaries (microsoft#25738) Support single point in area chart (microsoft#25842) chore: enable isolateModules in all v8 packages (microsoft#25774) chore: refactor styles for Button (microsoft#25216) feat: Improve docs for `DataGrid`, export as unstable (microsoft#25805) applying package updates fix: Allow data-selection-disabled to be respected by DetailsRow (microsoft#25836) docs(rfcs): Update recipes rfc with chosen option and add more details (microsoft#25823) chore(react-textarea): migrate to new package structure (microsoft#25820) chore(react-switch): migrate to new package structure (microsoft#25819) fix(react-avatar): AvatarGroup's pie layout places inline items correctly in rtl (microsoft#25822) chore: add few improvements to toolbar stories (microsoft#25635)

* chore: refactor styles for Button * refactor createCustomFocusIndicatorStyle to allow CSS shorthands * change file * Apply suggestions from code review Co-authored-by: Makoto Morimoto <[email protected]> Co-authored-by: Makoto Morimoto <[email protected]>

msft-fluent-ui-bot assigned miroslavstastny Oct 13, 2022

github-actions bot added this to the October Project Cycle Q4 2022 milestone Oct 13, 2022

layershifter force-pushed the chore/button-make-reset-styles branch from c20ed7f to f1edd10 Compare October 18, 2022 14:09

layershifter added the Ready for VR Used to trigger screener checks for PRs label Oct 18, 2022

layershifter closed this Oct 18, 2022

layershifter reopened this Oct 18, 2022

layershifter mentioned this pull request Oct 18, 2022

fix: relax check in transform plugin microsoft/griffel#255

Merged

layershifter force-pushed the chore/button-make-reset-styles branch 2 times, most recently from 664ceea to 1e0602f Compare October 19, 2022 09:15

layershifter closed this Oct 19, 2022

layershifter reopened this Oct 19, 2022

layershifter unassigned miroslavstastny Oct 24, 2022

msft-fluent-ui-bot added the Component: Button label Oct 24, 2022

layershifter added the Fluent UI react-components (v9) label Oct 24, 2022

spmonahan mentioned this pull request Oct 24, 2022

Refactor Button to use makeResetStyles #25353

Closed

layershifter force-pushed the chore/button-make-reset-styles branch 2 times, most recently from 7d525ea to 25a47af Compare October 27, 2022 11:49

layershifter mentioned this pull request Oct 27, 2022

chore: bump Griffel to latest #25412

Merged

layershifter force-pushed the chore/button-make-reset-styles branch 3 times, most recently from 0fe4903 to 11793de Compare November 1, 2022 21:01

layershifter marked this pull request as ready for review November 2, 2022 09:20

layershifter requested review from a team as code owners November 2, 2022 09:20

layershifter requested a review from khmakoto as a code owner November 2, 2022 09:20

layershifter marked this pull request as draft November 2, 2022 09:20

layershifter force-pushed the chore/button-make-reset-styles branch from 11793de to aa30023 Compare November 2, 2022 09:25

layershifter marked this pull request as ready for review November 2, 2022 10:03

Hotell approved these changes Nov 2, 2022

View reviewed changes

spmonahan reviewed Nov 2, 2022

View reviewed changes

spmonahan approved these changes Nov 7, 2022

View reviewed changes

jspurlin approved these changes Nov 10, 2022

View reviewed changes

layershifter added 2 commits November 23, 2022 12:01

chore: refactor styles for Button

6e89043

refactor createCustomFocusIndicatorStyle to allow CSS shorthands

444bef6

layershifter force-pushed the chore/button-make-reset-styles branch from f08c02c to 444bef6 Compare November 23, 2022 11:24

layershifter requested a review from a team as a code owner November 23, 2022 11:24

change file

91d604f

miroslavstastny approved these changes Nov 24, 2022

View reviewed changes

khmakoto approved these changes Nov 29, 2022

View reviewed changes

packages/react-components/react-components/src/index.ts Show resolved Hide resolved

packages/react-components/react-tabster/src/focus/createCustomFocusIndicatorStyle.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

acc7525

Co-authored-by: Makoto Morimoto <[email protected]>

layershifter enabled auto-merge (squash) November 30, 2022 09:05

layershifter merged commit 076ac58 into microsoft:master Nov 30, 2022

layershifter deleted the chore/button-make-reset-styles branch November 30, 2022 09:38

chore: refactor styles for Button #25216

chore: refactor styles for Button #25216

Uh oh!

Conversation

layershifter commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes?

Notes

Results

Uh oh!

codesandbox-ci bot commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

size-auditor bot commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

fabricteam commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react-components)

Uh oh!

fabricteam commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react-northstar)

Uh oh!

fabricteam commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react)

Uh oh!

Hotell commented Nov 2, 2022

Uh oh!

layershifter commented Nov 2, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spmonahan commented Nov 7, 2022

MS Edge

Mozilla Firefox

Uh oh!

jspurlin left a comment

Choose a reason for hiding this comment

Uh oh!

fabricteam commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🕵 fluentuiv9 Open the Visual Regressions report to inspect the 2 screenshots

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

layershifter commented Oct 13, 2022 •

edited

Loading

codesandbox-ci bot commented Oct 13, 2022 •

edited

Loading

size-auditor bot commented Oct 13, 2022 •

edited

Loading

fabricteam commented Oct 13, 2022 •

edited

Loading

fabricteam commented Oct 13, 2022 •

edited

Loading

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

fabricteam commented Oct 13, 2022 •

edited

Loading

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

fabricteam commented Oct 13, 2022 •

edited

Loading

Perf Analysis (`@fluentui/react`)

fabricteam commented Nov 23, 2022 •

edited

Loading