Skip to content

RFC: Conventions for static class names on slots#21206

Merged
behowell merged 8 commits into
microsoft:masterfrom
behowell:rfc-slot-classNames
Feb 24, 2022
Merged

RFC: Conventions for static class names on slots#21206
behowell merged 8 commits into
microsoft:masterfrom
behowell:rfc-slot-classNames

Conversation

@behowell

@behowell behowell commented Jan 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

📄 RFC: Conventions for static class names on slots

This RFC proposes adding a static className to every slot, using the naming format "fui-{ComponentName}__{slotName}".

We currently have a static class name "fui-{ComponentName}" (e.g. "fui-Button") added to the root slot of every component, as described in issue #19937. However, we don't currently have any formal specification of the format of the classNames for other slots, nor any conformance tests ensuring that they are added to every slot.

@behowell behowell self-assigned this Jan 6, 2022
@github-actions github-actions Bot added the Type: RFC Request for Feedback label Jan 6, 2022
@codesandbox-ci

codesandbox-ci Bot commented Jan 6, 2022 •
edited
Loading

Copy link
Copy Markdown

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 3d90605:

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

@size-auditor

size-auditor Bot commented Jan 6, 2022 •
edited
Loading

Copy link
Copy Markdown

Asset size changes

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

Baseline commit: e5cafd8510c6d58d4ab213cf18a2b07c9baa5f29 (build)

@fabricteam

fabricteam commented Jan 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

📊 Bundle size report

🤖 This report was generated against e5cafd8510c6d58d4ab213cf18a2b07c9baa5f29

@fabricteam

fabricteam commented Jan 6, 2022 •
edited
Loading

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
Avatar mount 788 772 5000
BaseButton mount 809 808 5000
Breadcrumb mount 2262 2291 1000
ButtonNext mount 414 399 5000
Checkbox mount 1348 1329 5000
CheckboxBase mount 1141 1145 5000
ChoiceGroup mount 4115 4130 5000
ComboBox mount 866 851 1000
CommandBar mount 8794 8789 1000
ContextualMenu mount 7186 7210 1000
DefaultButton mount 995 993 5000
DetailsRow mount 3226 3242 5000
DetailsRowFast mount 3220 3246 5000
DetailsRowNoStyles mount 3087 3034 5000
Dialog mount 1936 1931 1000
DocumentCardTitle mount 174 156 1000
Dropdown mount 2777 2776 5000
FluentProviderNext mount 1740 1707 5000
FluentProviderWithTheme mount 132 124 10
FluentProviderWithTheme virtual-rerender 88 93 10
FluentProviderWithTheme virtual-rerender-with-unmount 221 154 10
FocusTrapZone mount 1608 1592 5000
FocusZone mount 1571 1588 5000
IconButton mount 1540 1656 5000
Label mount 335 328 5000
Layer mount 2582 2576 5000
Link mount 414 431 5000
MakeStyles mount 1502 1495 50000
MenuButton mount 1277 1288 5000
MessageBar mount 1759 1719 5000
Nav mount 2824 2861 1000
OverflowSet mount 977 972 5000
Panel mount 1831 1832 1000
Persona mount 749 763 1000
Pivot mount 1245 1260 1000
PrimaryButton mount 1118 1104 5000
Rating mount 6610 6603 5000
SearchBox mount 1171 1162 5000
Shimmer mount 2205 2225 5000
Slider mount 1688 1717 5000
SpinButton mount 4346 4304 5000
Spinner mount 388 407 5000
SplitButton mount 2739 2733 5000
Stack mount 469 477 5000
StackWithIntrinsicChildren mount 2009 2012 5000
StackWithTextChildren mount 4535 4517 5000
SwatchColorPicker mount 9932 9900 5000
TagPicker mount 2265 2256 5000
TeachingBubble mount 11321 11374 5000
Text mount 393 392 5000
TextField mount 1214 1220 5000
ThemeProvider mount 1039 1047 5000
ThemeProvider virtual-rerender 543 551 5000
ThemeProvider virtual-rerender-with-unmount 1608 1631 5000
Toggle mount 714 715 5000
buttonNative mount 137 131 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
RefMinimalPerf.default 215 200 1.08:1
VideoMinimalPerf.default 576 538 1.07:1
AlertMinimalPerf.default 235 222 1.06:1
ButtonSlotsPerf.default 479 451 1.06:1
SkeletonMinimalPerf.default 316 298 1.06:1
StatusMinimalPerf.default 603 567 1.06:1
ButtonMinimalPerf.default 148 141 1.05:1
AnimationMinimalPerf.default 475 457 1.04:1
CarouselMinimalPerf.default 415 399 1.04:1
ChatWithPopoverPerf.default 335 322 1.04:1
LayoutMinimalPerf.default 323 311 1.04:1
ReactionMinimalPerf.default 335 322 1.04:1
DividerMinimalPerf.default 320 310 1.03:1
PortalMinimalPerf.default 159 155 1.03:1
TextAreaMinimalPerf.default 433 422 1.03:1
TreeWith60ListItems.default 160 156 1.03:1
AccordionMinimalPerf.default 132 130 1.02:1
AvatarMinimalPerf.default 177 174 1.02:1
ChatMinimalPerf.default 639 627 1.02:1
DropdownMinimalPerf.default 2611 2565 1.02:1
HeaderMinimalPerf.default 315 308 1.02:1
ImageMinimalPerf.default 324 318 1.02:1
LabelMinimalPerf.default 334 329 1.02:1
ListNestedPerf.default 486 476 1.02:1
ListWith60ListItems.default 576 567 1.02:1
LoaderMinimalPerf.default 595 582 1.02:1
IconMinimalPerf.default 513 504 1.02:1
TooltipMinimalPerf.default 896 875 1.02:1
TreeMinimalPerf.default 693 679 1.02:1
AttachmentMinimalPerf.default 139 138 1.01:1
BoxMinimalPerf.default 299 295 1.01:1
DialogMinimalPerf.default 650 646 1.01:1
DropdownManyItemsPerf.default 586 581 1.01:1
GridMinimalPerf.default 296 294 1.01:1
InputMinimalPerf.default 1122 1110 1.01:1
ListMinimalPerf.default 441 436 1.01:1
TextMinimalPerf.default 303 300 1.01:1
ToolbarMinimalPerf.default 804 797 1.01:1
ButtonOverridesMissPerf.default 1440 1434 1:1
CardMinimalPerf.default 468 469 1:1
DatepickerMinimalPerf.default 4845 4855 1:1
EmbedMinimalPerf.default 3508 3509 1:1
HeaderSlotsPerf.default 654 655 1:1
ListCommonPerf.default 544 544 1:1
MenuMinimalPerf.default 724 727 1:1
MenuButtonMinimalPerf.default 1406 1407 1:1
PopupMinimalPerf.default 526 524 1:1
ProviderMergeThemesPerf.default 1484 1478 1:1
ProviderMinimalPerf.default 968 968 1:1
SegmentMinimalPerf.default 297 297 1:1
SplitButtonMinimalPerf.default 3680 3675 1:1
TableManyItemsPerf.default 1606 1614 1:1
CustomToolbarPrototype.default 3501 3509 1:1
CheckboxMinimalPerf.default 2282 2304 0.99:1
FlexMinimalPerf.default 245 247 0.99:1
FormMinimalPerf.default 353 356 0.99:1
ItemLayoutMinimalPerf.default 1001 1008 0.99:1
SliderMinimalPerf.default 1418 1432 0.99:1
AttachmentSlotsPerf.default 924 940 0.98:1
ChatDuplicateMessagesPerf.default 258 262 0.98:1
RosterPerf.default 1007 1025 0.98:1
RadioGroupMinimalPerf.default 377 389 0.97:1
TableMinimalPerf.default 338 348 0.97:1

@behowell behowell marked this pull request as ready for review January 7, 2022 03:00
@behowell behowell requested review from a team, khmakoto, layershifter, spmonahan and tomi-msft and removed request for a team, spmonahan and tomi-msft January 7, 2022 03:01
miroslavstastny
miroslavstastny reviewed Jan 7, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
miroslavstastny
miroslavstastny reviewed Jan 7, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md
spmonahan
spmonahan approved these changes Jan 7, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
GeoffCoxMSFT
GeoffCoxMSFT reviewed Jan 7, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md
GeoffCoxMSFT
GeoffCoxMSFT reviewed Jan 7, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
layershifter
layershifter reviewed Jan 10, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
layershifter
layershifter reviewed Jan 10, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
khmakoto
khmakoto approved these changes Jan 29, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
micahgodbolt
micahgodbolt reviewed Jan 31, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md Outdated
micahgodbolt
micahgodbolt reviewed Jan 31, 2022
View reviewed changes
Comment thread rfcs/react-components/convergence/static-classes-on-slots.md
@ling1726 ling1726 closed this Feb 17, 2022
@ling1726 ling1726 reopened this Feb 17, 2022
@behowell behowell mentioned this pull request Feb 23, 2022
Closed
31 tasks
@behowell behowell merged commit 43b133e into microsoft:master Feb 24, 2022
@behowell behowell deleted the rfc-slot-classNames branch February 24, 2022 00:12
@spmonahan spmonahan mentioned this pull request Feb 24, 2022
Closed
14 tasks
This was referenced Mar 2, 2022
Merged
Merged
spmonahan added a commit to spmonahan/fluentui that referenced this pull request Mar 4, 2022
@spmonahan
Add conformance tests for static classnames
7688850 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (microsoft#21206).

Only Radio and RadioGroup have been updated to pass these test.
spmonahan added a commit to spmonahan/fluentui that referenced this pull request Mar 4, 2022
@spmonahan
Add conformance tests for static classnames
4e9a3a4 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (microsoft#21206).

Only Radio and RadioGroup have been updated to pass these test.
spmonahan added a commit to spmonahan/fluentui that referenced this pull request Mar 4, 2022
@spmonahan
Add conformance tests for static classnames
74dd8df 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (microsoft#21206).

Only Radio and RadioGroup have been updated to pass these test.
@spmonahan spmonahan mentioned this pull request Mar 4, 2022
Merged
spmonahan added a commit to spmonahan/fluentui that referenced this pull request Mar 8, 2022
@spmonahan
Add conformance tests for static classnames
8431314 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (microsoft#21206).

Only Radio and RadioGroup have been updated to pass these test.
spmonahan added a commit to spmonahan/fluentui that referenced this pull request Mar 11, 2022
@spmonahan
Add conformance tests for static classnames
2acc259 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (microsoft#21206).

Only Radio and RadioGroup have been updated to pass these test.
spmonahan added a commit to spmonahan/fluentui that referenced this pull request Mar 14, 2022
@spmonahan
Add conformance tests for static classnames
1138c39 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (microsoft#21206).

Only Radio and RadioGroup have been updated to pass these test.
@spmonahan spmonahan mentioned this pull request Mar 15, 2022
Merged
spmonahan added a commit to spmonahan/fluentui that referenced this pull request Mar 15, 2022
@spmonahan
Add conformance tests for static classnames
a440c18 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (microsoft#21206).

Only Radio and RadioGroup have been updated to pass these test.
ecraig12345 pushed a commit that referenced this pull request Mar 16, 2022
@spmonahan
Add conformance tests for static classnames (#21960)
086a675 
This commit adds conformance tests for static class names. The tests:

1. Ensure that the static classnames object is exported
2. Ensure that the static classnames object is formatted correctly
3. Ensure that the static classnames are rendered in the component

The rules for static classnames are defined in RFC: Conventions for
static class names on slots (#21206).
@spmonahan spmonahan mentioned this pull request Mar 16, 2022
Closed
19 tasks
@sopranopillow sopranopillow mentioned this pull request May 17, 2022
Merged
@spmonahan spmonahan mentioned this pull request Jun 13, 2022
Closed
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@micahgodbolt micahgodbolt micahgodbolt left review comments
@layershifter layershifter layershifter approved these changes
@khmakoto khmakoto khmakoto approved these changes
@miroslavstastny miroslavstastny miroslavstastny approved these changes
@GeoffCoxMSFT GeoffCoxMSFT GeoffCoxMSFT approved these changes
@spmonahan spmonahan spmonahan approved these changes

Assignees

@behowell behowell

Labels

Type: RFC Request for Feedback

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@behowell @fabricteam @micahgodbolt @khmakoto @miroslavstastny @layershifter @GeoffCoxMSFT @spmonahan @ling1726