Skip to content

fix(merge-styles): Correctly processing selectors when value is a class name and they are not wrapped in selectors wrapper#31798

Merged
khmakoto merged 2 commits into
microsoft:masterfrom
khmakoto:fix/merge-styles/classNamesAreCorrectlyPassedToSelectors
Jun 24, 2024
Merged

fix(merge-styles): Correctly processing selectors when value is a class name and they are not wrapped in selectors wrapper#31798
khmakoto merged 2 commits into
microsoft:masterfrom
khmakoto:fix/merge-styles/classNamesAreCorrectlyPassedToSelectors

Conversation

@khmakoto

@khmakoto khmakoto commented Jun 24, 2024

Copy link
Copy Markdown
Member

Previous Behavior

After #31253 was merged and the selectors wrapper was removed from every button style, it was discovered that some style customizations were no longer working, such as the one in this Stackblitz project where the rootFocused style customizations are not being applied.

After some investigation, I discovered that the selectors wrapper forced a path within extractRules to actually go through the extractSelector process, which ensured that the pseudo selectors were being properly handled. In contrast, not applying the wrapper meant that the logic checked for the value of the pseudo selector being an object, a case that is handled correctly, but the same logic doesn't apply when the value is just a class name as in the example above.

This is something that has never worked but that hadn't been exposed before because most of our components still use the selectors wrapper internally.

New Behavior

I have added an isSelector local function that checks for the same set of minimum conditions that the currently used expandSelector checks, and use it to determine if something needs to go through the extractSelector code path. This ensures that even when the value of a pseudo selector is not an object, it is treated properly.

Note: I tried to add a test to shield this in the future, but since the correct behavior when the value is a string is to not add anything to the stylesheet, since the class name should presumably already be there and is just being applied to that pseudo selector, I could not really check for something else being added to the stylesheet to verify this.

@khmakoto khmakoto added Package: merge-styles Fluent UI react (v8) Issues about @fluentui/react (v8) labels Jun 24, 2024
@khmakoto khmakoto self-assigned this Jun 24, 2024
@khmakoto
khmakoto requested review from a team and dzearing as code owners June 24, 2024 18:58
@github-actions github-actions Bot added this to the April Project Cycle Q1 2024 milestone Jun 24, 2024
@fabricteam

Copy link
Copy Markdown
Collaborator

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react
ActivityItem
71.094 kB
23.309 kB
71.147 kB
23.33 kB
53 B
21 B
react
Announced
38.366 kB
13.259 kB
38.419 kB
13.288 kB
53 B
29 B
react
Breadcrumb
201.952 kB
60.34 kB
202.005 kB
60.362 kB
53 B
22 B
react
Button
195.42 kB
56.534 kB
195.473 kB
56.558 kB
53 B
24 B
react
ButtonGrid
180.363 kB
54.589 kB
180.416 kB
54.61 kB
53 B
21 B
react
Calendar
121.617 kB
36.963 kB
121.67 kB
36.976 kB
53 B
13 B
react
Callout
84.183 kB
27.574 kB
84.236 kB
27.607 kB
53 B
33 B
react
Check
53.038 kB
17.798 kB
53.091 kB
17.837 kB
53 B
39 B
react
Checkbox
59.826 kB
19.83 kB
59.879 kB
19.865 kB
53 B
35 B
react
ChoiceGroup
65.351 kB
21.457 kB
65.404 kB
21.469 kB
53 B
12 B
react
ChoiceGroupOption
58.606 kB
19.332 kB
58.659 kB
19.356 kB
53 B
24 B
react
Coachmark
93.012 kB
29.377 kB
93.065 kB
29.412 kB
53 B
35 B
react
ColorPicker
133.06 kB
41.521 kB
133.113 kB
41.536 kB
53 B
15 B
react
ComboBox
251.721 kB
72.161 kB
251.774 kB
72.184 kB
53 B
23 B
react
CommandBar
202.989 kB
60.048 kB
203.042 kB
60.075 kB
53 B
27 B
react
ContextualMenu
154.945 kB
48.139 kB
154.998 kB
48.158 kB
53 B
19 B
react
DatePicker
184.014 kB
56.1 kB
184.067 kB
56.121 kB
53 B
21 B
react
DetailsList
229.392 kB
65.655 kB
229.445 kB
65.681 kB
53 B
26 B
react
Dialog
211.276 kB
63.066 kB
211.329 kB
63.082 kB
53 B
16 B
react
Divider
19.478 kB
6.825 kB
19.531 kB
6.862 kB
53 B
37 B
react
DocumentCard
216.961 kB
64.34 kB
217.014 kB
64.363 kB
53 B
23 B
react
DraggableZone
34.188 kB
11.472 kB
34.241 kB
11.491 kB
53 B
19 B
react
Dropdown
233.882 kB
68.535 kB
233.935 kB
68.547 kB
53 B
12 B
react
ExtendedPicker
96.64 kB
27.827 kB
96.693 kB
27.85 kB
53 B
23 B
react
Fabric
41.612 kB
14.306 kB
41.665 kB
14.358 kB
53 B
52 B
react
Facepile
210.537 kB
63.11 kB
210.59 kB
63.132 kB
53 B
22 B
react
FloatingPicker
242.007 kB
68.903 kB
242.06 kB
68.931 kB
53 B
28 B
react
FocusZone
54.919 kB
17.421 kB
54.972 kB
17.442 kB
53 B
21 B
react
Grid
180.363 kB
54.589 kB
180.416 kB
54.61 kB
53 B
21 B
react
GroupedList
134.709 kB
40.617 kB
134.762 kB
40.629 kB
53 B
12 B
react
GroupedListV2
122.326 kB
37.724 kB
122.379 kB
37.737 kB
53 B
13 B
react
HoverCard
97.044 kB
30.76 kB
97.097 kB
30.782 kB
53 B
22 B
react
Icon
51.719 kB
17.219 kB
51.772 kB
17.252 kB
53 B
33 B
react
Image
46.776 kB
15.668 kB
46.829 kB
15.701 kB
53 B
33 B
react
Keytip
81.557 kB
26.731 kB
81.61 kB
26.75 kB
53 B
19 B
react
KeytipLayer
103.353 kB
31.983 kB
103.406 kB
32.004 kB
53 B
21 B
react
Keytips
106.121 kB
32.986 kB
106.174 kB
33.006 kB
53 B
20 B
react
Label
38.213 kB
13.226 kB
38.266 kB
13.244 kB
53 B
18 B
react
Layer
47.962 kB
16.316 kB
48.015 kB
16.337 kB
53 B
21 B
react
Link
39.563 kB
13.637 kB
39.616 kB
13.649 kB
53 B
12 B
react
List
39.251 kB
12.405 kB
39.304 kB
12.441 kB
53 B
36 B
react
MarqueeSelection
74.396 kB
22.41 kB
74.449 kB
22.441 kB
53 B
31 B
react
MessageBar
190.467 kB
57.013 kB
190.52 kB
57.042 kB
53 B
29 B
react
Modal
93.515 kB
30.201 kB
93.568 kB
30.235 kB
53 B
34 B
react
Nav
187.945 kB
56.45 kB
187.998 kB
56.464 kB
53 B
14 B
react
OverflowSet
33.266 kB
11.28 kB
33.319 kB
11.317 kB
53 B
37 B
react
Overlay
40.769 kB
14.046 kB
40.822 kB
14.083 kB
53 B
37 B
react
Panel
200.987 kB
59.938 kB
201.04 kB
59.952 kB
53 B
14 B
react
Persona
114.692 kB
36.466 kB
114.745 kB
36.478 kB
53 B
12 B
react
PersonaCoin
114.692 kB
36.466 kB
114.745 kB
36.478 kB
53 B
12 B
react
PersonaPresence
57.908 kB
19.327 kB
57.961 kB
19.359 kB
53 B
32 B
react
Pickers
293.697 kB
82.241 kB
293.75 kB
82.262 kB
53 B
21 B
react
Pivot
188.859 kB
57.186 kB
188.912 kB
57.213 kB
53 B
27 B
react
Positioning
22.683 kB
7.651 kB
22.736 kB
7.685 kB
53 B
34 B
react
PositioningContainer
73.722 kB
23.735 kB
73.775 kB
23.762 kB
53 B
27 B
react
ProgressIndicator
39.361 kB
13.505 kB
39.414 kB
13.548 kB
53 B
43 B
react
Rating
81.837 kB
26.078 kB
81.89 kB
26.097 kB
53 B
19 B
react
Fluent UI React (entire library)
1.013 MB
281.366 kB
1.013 MB
281.385 kB
53 B
19 B
react
ScrollablePane
55.404 kB
17.693 kB
55.457 kB
17.735 kB
53 B
42 B
react
SearchBox
188.742 kB
56.617 kB
188.795 kB
56.649 kB
53 B
32 B
react
SelectedItemsList
232.506 kB
67.852 kB
232.559 kB
67.863 kB
53 B
11 B
react
Selection
42.327 kB
12.227 kB
42.38 kB
12.255 kB
53 B
28 B
react
Separator
35.262 kB
12.117 kB
35.315 kB
12.157 kB
53 B
40 B
react
Shimmer
49.124 kB
16.228 kB
49.177 kB
16.256 kB
53 B
28 B
react
ShimmeredDetailsList
240.167 kB
68.421 kB
240.22 kB
68.444 kB
53 B
23 B
react
Slider
57.524 kB
19.164 kB
57.577 kB
19.194 kB
53 B
30 B
react
SpinButton
192.47 kB
57.735 kB
192.523 kB
57.748 kB
53 B
13 B
react
Spinner
41.556 kB
14.431 kB
41.609 kB
14.458 kB
53 B
27 B
react
Stack
41.622 kB
14.253 kB
41.675 kB
14.283 kB
53 B
30 B
react
Styling
45.928 kB
15.103 kB
45.981 kB
15.121 kB
53 B
18 B
react
SwatchColorPicker
190.689 kB
58.074 kB
190.742 kB
58.094 kB
53 B
20 B
react
TeachingBubble
205.742 kB
60.967 kB
205.795 kB
60.984 kB
53 B
17 B
react
Text
36.798 kB
12.785 kB
36.851 kB
12.828 kB
53 B
43 B
react
TextField
80.63 kB
25.28 kB
80.683 kB
25.303 kB
53 B
23 B
react
Theme
43.396 kB
14.154 kB
43.449 kB
14.169 kB
53 B
15 B
react
TimePicker
241.525 kB
69.93 kB
241.578 kB
69.953 kB
53 B
23 B
react
Toggle
46.095 kB
15.926 kB
46.148 kB
15.96 kB
53 B
34 B
react
Tooltip
87.211 kB
28.189 kB
87.264 kB
28.207 kB
53 B
18 B
react
Utilities
82.57 kB
25.068 kB
82.623 kB
25.106 kB
53 B
38 B
react
Viewport
23.778 kB
7.614 kB
23.831 kB
7.638 kB
53 B
24 B
react
WeeklyDayPicker
101.761 kB
31.775 kB
101.814 kB
31.796 kB
53 B
21 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react
Autofill
15.365 kB
4.743 kB
react
Color
7.737 kB
3.106 kB
react
DateTimeUtilities
5.244 kB
1.849 kB
react
DragDrop
8.343 kB
2.724 kB
react
FocusTrapZone
16.979 kB
5.921 kB
react
Icons
66.309 kB
24.381 kB
react
KeytipData
13.973 kB
4.573 kB
react
Popup
12.242 kB
4.181 kB
react
ResizeGroup
13.286 kB
4.365 kB
react
ResponsiveMode
8.078 kB
2.95 kB
react
SelectableOption
724 B
413 B
react
Sticky
32.541 kB
10.49 kB
react
ThemeGenerator
12.34 kB
4.106 kB
react
WindowProvider
1.059 kB
541 B
🤖 This report was generated against 9f13dee5c8a1e6f2c9ce99494a26ea195e8a4ce6

@fabricteam

Copy link
Copy Markdown
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 617 622 5000
Breadcrumb mount 1687 1675 1000
Checkbox mount 1683 1740 5000
CheckboxBase mount 1493 1459 5000
ChoiceGroup mount 2974 3025 5000
ComboBox mount 665 655 1000
CommandBar mount 6467 6540 1000
ContextualMenu mount 12808 12674 1000
DefaultButton mount 785 791 5000
DetailsRow mount 2203 2218 5000
DetailsRowFast mount 2226 2221 5000
DetailsRowNoStyles mount 2037 2025 5000
Dialog mount 2759 2767 1000
DocumentCardTitle mount 217 222 1000
Dropdown mount 2001 1978 5000
FocusTrapZone mount 1145 1168 5000
FocusZone mount 1103 1084 5000
GroupedList mount 41827 42340 2
GroupedList virtual-rerender 18010 20365 2
GroupedList virtual-rerender-with-unmount 51045 50861 2
GroupedListV2 mount 231 232 2
GroupedListV2 virtual-rerender 202 215 2
GroupedListV2 virtual-rerender-with-unmount 221 234 2
IconButton mount 1126 1115 5000
Label mount 341 345 5000
Layer mount 2703 2725 5000
Link mount 394 397 5000
MenuButton mount 968 980 5000
MessageBar mount 21353 21327 5000
Nav mount 2025 2051 1000
OverflowSet mount 781 788 5000
Panel mount 1841 1788 1000
Persona mount 754 755 1000
Pivot mount 916 908 1000
PrimaryButton mount 914 925 5000
Rating mount 4772 4584 5000
SearchBox mount 921 936 5000
Shimmer mount 1850 1873 5000
Slider mount 1330 1322 5000
SpinButton mount 2984 2957 5000
Spinner mount 391 392 5000
SplitButton mount 1878 1852 5000
Stack mount 418 430 5000
StackWithIntrinsicChildren mount 833 885 5000
StackWithTextChildren mount 2669 2743 5000
SwatchColorPicker mount 6474 6499 5000
TagPicker mount 1444 1416 5000
Text mount 383 396 5000
TextField mount 914 917 5000
ThemeProvider mount 834 848 5000
ThemeProvider virtual-rerender 586 582 5000
ThemeProvider virtual-rerender-with-unmount 1275 1316 5000
Toggle mount 603 609 5000
buttonNative mount 182 185 5000

@khmakoto
khmakoto merged commit 1c0ba2f into microsoft:master Jun 24, 2024
@khmakoto
khmakoto deleted the fix/merge-styles/classNamesAreCorrectlyPassedToSelectors branch June 24, 2024 23:09
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jun 26, 2024
* master: (90 commits)
  fix(react-drawer): do not create global header/footer roles (microsoft#31799)
  release: applying package updates - react v8
  release: applying package updates - web-components
  Add 'allowParentArrowNavigation' prop to `ComboBox`. (microsoft#31805)
  Add drawer web component (microsoft#31521)
  fix: MenuButton is styled correctly when aria-expanded is a string (microsoft#31809)
  chore: Updating version of `@fluentui/react-icons` to `^2.0.245` (microsoft#31802)
  chore:(docs) Migrate e2e Cypress testing doc (microsoft#31796)
  chore:(docs) Update API Extractor file (microsoft#31651)
  chore: migrate to nx 18 (microsoft#31746)
  release: applying package updates - react v8
  release: applying package updates - web-components
  feat: update text to use element internals for custom states (microsoft#31770)
  chore(web-components): Remove define imports from story modules (microsoft#31622)
  fix(`merge-styles`): Correctly processing selectors when value is a class name and they are not wrapped in `selectors` wrapper (microsoft#31798)
  style(react-combobox): turn off react-compiler lints for non compliant code (microsoft#31795)
  docs: Add AriaLiveAnnouncer to all examples (microsoft#31794)
  fix(public-docsite-v9): disable storybook keyboard shortcuts (microsoft#31762)
  feat: V0 Attachment compact component (microsoft#31634)
  release: applying package updates - react v8
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fluent UI react (v8) Issues about @fluentui/react (v8) Package: merge-styles

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants