Skip to content
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

fix: Replace child selectors with css vars in Checkbox styles #29796

Merged

Conversation

behowell
Copy link
Contributor

@behowell behowell commented Nov 8, 2023

Previous Behavior

Checkbox styles use the child selector ['> .${checkboxClassNames.indicator}'] to set the indicator color based on the hover/selected state of the root slot. This makes it more difficult for users to override the styles.

New Behavior

Set the indicator colors using CSS variables on the Checkbox root, and the indicator slot's reset styles reference the CSS variables. Existing style overrides should continue to work, as the new styles have lower specificity.

Related Issue(s)

@behowell behowell requested a review from ling1726 November 8, 2023 22:33
@behowell behowell self-assigned this Nov 8, 2023
@behowell behowell requested review from khmakoto and a team as code owners November 8, 2023 22:33
@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
FluentProviderWithTheme virtual-rerender 73 70 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 672 663 5000
Button mount 316 318 5000
Field mount 1142 1152 5000
FluentProvider mount 722 730 5000
FluentProviderWithTheme mount 78 82 10
FluentProviderWithTheme virtual-rerender 73 70 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 82 79 10
MakeStyles mount 867 897 50000
Persona mount 1778 1734 5000
SpinButton mount 1359 1361 5000

Copy link

codesandbox-ci bot commented Nov 8, 2023

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

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

@fabricteam
Copy link
Collaborator

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-checkbox
Checkbox
37.505 kB
12.327 kB
34.253 kB
11.785 kB
-3.252 kB
-542 B
react-table
DataGrid
157.084 kB
43.906 kB
153.832 kB
43.307 kB
-3.252 kB
-599 B
react-table
Table as DataGrid
130.165 kB
35.174 kB
126.913 kB
34.6 kB
-3.252 kB
-574 B
react-table
Table (Selection only)
75.777 kB
20.477 kB
72.525 kB
19.909 kB
-3.252 kB
-568 B
react-table
Table (Sort only)
74.384 kB
20.075 kB
71.132 kB
19.512 kB
-3.252 kB
-563 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
70.013 kB
20.17 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
207.755 kB
59.417 kB
react-components
react-components: FluentProvider & webLightTheme
42.297 kB
14.008 kB
react-portal-compat
PortalCompatProvider
6.651 kB
2.252 kB
react-table
Table (Primitives only)
43.941 kB
13.76 kB
🤖 This report was generated against c139fc66cca05255b5c96f6c92d9230b37975276

Copy link

size-auditor bot commented Nov 8, 2023

Asset size changes

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

Baseline commit: c139fc66cca05255b5c96f6c92d9230b37975276 (build)

@fabricteam
Copy link
Collaborator

🕵 fluentuiv9 No visual regressions between this PR and main

@behowell behowell merged commit 31f1f61 into microsoft:master Nov 15, 2023
@behowell behowell deleted the checkbox/refactor-indicator-styles branch November 15, 2023 00:55
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.

Checkbox indicator rest styles do not need to use combinators
4 participants