Skip to content

chore: Update Checkbox and Radio to use griffel reset styles#25984

Merged
behowell merged 4 commits intomicrosoft:masterfrom
behowell:checkbox/reset-styles
Dec 14, 2022
Merged

chore: Update Checkbox and Radio to use griffel reset styles#25984
behowell merged 4 commits intomicrosoft:masterfrom
behowell:checkbox/reset-styles

Conversation

@behowell
Copy link
Contributor

@behowell behowell commented Dec 14, 2022

Previous Behavior

Checkbox and Radio's styles include all of the styles for the checked, hover, etc. states in the base styles, and use CSS selectors to conditionally apply the styles.

  • This results in a large number of classes being added to the default checkbox, because Griffel styles use a separate class for each combination of selector & property. As shown below, there are a total of 154 css classes across all of Checkbox's slots.
  • This affects performance because chromium's css performance degrades with the number of classes.

Checkbox class counts

New Behavior

Use Griffel's makeResetStyles to combine all of the base styles for each of Checkbox and Radio's slots into a single CSS class. Also, merge the styles for the default state into the base styles (size: medium, labelPosition: after).

  • For Checkbox, this results in an 85% reduction in the number of CSS classes across all slots: down to 23 from 154 before.
  • The stress test for Checkbox shows a roughly 12% improvement in performance for applying styling.

Checkbox class counts

Checkbox perf test results

Related Issue(s)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 14, 2022

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

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

@size-auditor
Copy link

size-auditor bot commented Dec 14, 2022

Asset size changes

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

Baseline commit: 134bbf565b13a0375a8db4ac022737d45e46c425 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Dec 14, 2022

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-radio
Radio
36.42 kB
12.126 kB
31.827 kB
10.316 kB
-4.593 kB
-1.81 kB
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
59.288 kB
16.435 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
186.264 kB
52.303 kB
react-components
react-components: FluentProvider & webLightTheme
33.75 kB
11.101 kB
react-portal-compat
PortalCompatProvider
5.857 kB
1.978 kB
react-radio
RadioGroup
14.304 kB
5.72 kB
🤖 This report was generated against 134bbf565b13a0375a8db4ac022737d45e46c425

@fabricteam
Copy link
Collaborator

fabricteam commented Dec 14, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1271 1293 5000
Button mount 918 922 5000
FluentProvider mount 1494 1500 5000
FluentProviderWithTheme mount 567 588 10
FluentProviderWithTheme virtual-rerender 553 548 10
FluentProviderWithTheme virtual-rerender-with-unmount 584 598 10
MakeStyles mount 1971 1973 50000
Persona mount 2841 2825 5000
SpinButton mount 2360 2330 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Dec 14, 2022

🕵 fluentuiv9 No visual regressions between this PR and main

@behowell behowell marked this pull request as ready for review December 14, 2022 08:43
@behowell behowell requested review from a team and khmakoto as code owners December 14, 2022 08:43
@behowell behowell changed the title chore: Update Checkbox to use griffel reset styles chore: Update Checkbox and Radio to use griffel reset styles Dec 14, 2022
@behowell behowell requested a review from spmonahan as a code owner December 14, 2022 09:03
justifyContent: 'center',
overflow: 'hidden',

borderWidth: tokens.strokeWidthThin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this line and the next can be collapsed into

border: `${tokens.strokeWidthThin} solid`

justifyContent: 'center',
overflow: 'hidden',

borderWidth: tokens.strokeWidthThin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: same as in Checkbox, I believe these two border styles can be collapsed into one.

@behowell behowell merged commit 9c6efea into microsoft:master Dec 14, 2022
@behowell behowell deleted the checkbox/reset-styles branch December 14, 2022 21:31
Hotell pushed a commit to Hotell/fluentui that referenced this pull request Feb 9, 2023
…ft#25984)

Use Griffel's makeResetStyles to combine all of the base styles for each of Checkbox and Radio's slots into a single CSS class. Also, merge the styles for the default state into the base styles (size: medium, labelPosition: after).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants