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.

Sign up for GitHub

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

Jump to bottom

chore: use constants instead of magic numbers in makeStyles() #17729

Merged
merged 2 commits into from
Apr 7, 2021
Merged

chore: use constants instead of magic numbers in makeStyles() #17729

merged 2 commits into from
Apr 7, 2021

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Apr 7, 2021 •
edited

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ yarn change

Description of changes

This PR is based on a suggestion from #17669 (comment) @miroslavstastny to use constants as magic numbers are hard to read & understand.

@size-auditor
Copy link

size-auditor bot commented Apr 7, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-components-Avatar 45.555 kB 45.563 kB ExceedsBaseline     8 bytes
office-ui-fabric-react fluentui-react-components-Button 38.197 kB 38.205 kB ExceedsBaseline     8 bytes
office-ui-fabric-react fluentui-react-components-Divider 33.145 kB 33.153 kB ExceedsBaseline     8 bytes
office-ui-fabric-react fluentui-react-components-Image 32.893 kB 32.901 kB ExceedsBaseline     8 bytes
office-ui-fabric-react fluentui-react-components-Link 33.127 kB 33.135 kB ExceedsBaseline     8 bytes
office-ui-fabric-react fluentui-react-components-Menu 66.632 kB 66.64 kB ExceedsBaseline     8 bytes
office-ui-fabric-react fluentui-react-components-makeStyles 22.251 kB 22.259 kB ExceedsBaseline     8 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 45f6f31cefc412032a792179308ad837af63155a (build)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 7, 2021

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 805a405:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@layershifter layershifter marked this pull request as ready for review April 7, 2021 14:20
@layershifter layershifter changed the title chore: use constans instead of magic numbers in makeStyles() chore: use constants instead of magic numbers in makeStyles() Apr 7, 2021
@fabricteam
Copy link
Collaborator

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1079 1053 5000
BaseButton mount 1058 1067 5000
Breadcrumb mount 47708 48362 5000
ButtonNext mount 632 592 5000
Checkbox mount 1834 1764 5000
CheckboxBase mount 1494 1502 5000
ChoiceGroup mount 5374 5503 5000
ComboBox mount 1098 1089 1000
CommandBar mount 11230 11405 1000
ContextualMenu mount 7102 6852 1000
DefaultButton mount 1279 1295 5000
DetailsRow mount 4085 4179 5000
DetailsRowFast mount 4071 4197 5000
DetailsRowNoStyles mount 3907 3942 5000
Dialog mount 1646 1686 1000
DocumentCardTitle mount 1975 2000 1000
Dropdown mount 3710 3679 5000
FocusTrapZone mount 1976 1973 5000
FocusZone mount 2032 2041 5000
IconButton mount 2030 2042 5000
Label mount 370 373 5000
Layer mount 2037 2077 5000
Link mount 534 545 5000
MakeStyles mount 1948 1937 50000
MenuButton mount 1682 1694 5000
MessageBar mount 2263 2219 5000
Nav mount 3657 3757 1000
OverflowSet mount 1187 1146 5000
Panel mount 1572 1604 1000
Persona mount 898 926 1000
Pivot mount 1573 1598 1000
PrimaryButton mount 1459 1445 5000
Rating mount 8779 8861 5000
SearchBox mount 1507 1545 5000
Shimmer mount 2936 2907 5000
Slider mount 2210 2209 5000
SpinButton mount 5674 5550 5000
Spinner mount 462 467 5000
SplitButton mount 3622 3585 5000
Stack mount 578 570 5000
StackWithIntrinsicChildren mount 1718 1778 5000
StackWithTextChildren mount 5251 5334 5000
SwatchColorPicker mount 11351 11418 5000
Tabs mount 1547 1634 1000
TagPicker mount 3216 3188 5000
TeachingBubble mount 12640 12802 5000
Text mount 463 473 5000
TextField mount 1539 1550 5000
ThemeProvider mount 1300 1326 5000
ThemeProvider virtual-rerender 638 647 5000
ThemeProviderNext mount 16805 16665 5000
Toggle mount 935 889 5000
buttonNative mount 131 122 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.2 0.53 0.38:1 2000 402
🦄 Button.Fluent 0.13 0.23 0.57:1 5000 659
🔧 Checkbox.Fluent 0.68 0.44 1.55:1 1000 682
🦄 Dialog.Fluent 0.17 0.25 0.68:1 5000 849
🔧 Dropdown.Fluent 3.43 0.46 7.46:1 1000 3432
🔧 Icon.Fluent 0.15 0.07 2.14:1 5000 760
🦄 Image.Fluent 0.09 0.15 0.6:1 5000 471
🔧 Slider.Fluent 1.72 0.52 3.31:1 1000 1724
🔧 Text.Fluent 0.09 0.04 2.25:1 5000 453
🦄 Tooltip.Fluent 0.17 0.99 0.17:1 5000 842

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 232 199 1.17:1
ListWith60ListItems.default 799 736 1.09:1
PopupMinimalPerf.default 823 762 1.08:1
FormMinimalPerf.default 521 487 1.07:1
ImageMinimalPerf.default 496 465 1.07:1
TextMinimalPerf.default 438 409 1.07:1
SegmentMinimalPerf.default 438 413 1.06:1
IconMinimalPerf.default 732 688 1.06:1
AnimationMinimalPerf.default 508 483 1.05:1
ButtonOverridesMissPerf.default 1966 1881 1.05:1
CardMinimalPerf.default 699 667 1.05:1
RefMinimalPerf.default 282 269 1.05:1
SplitButtonMinimalPerf.default 4452 4259 1.05:1
AlertMinimalPerf.default 340 326 1.04:1
ButtonSlotsPerf.default 652 626 1.04:1
GridMinimalPerf.default 421 405 1.04:1
ProviderMinimalPerf.default 1144 1101 1.04:1
TooltipMinimalPerf.default 1119 1078 1.04:1
TreeMinimalPerf.default 944 905 1.04:1
Icon.Fluent 760 731 1.04:1
Image.Fluent 471 454 1.04:1
Text.Fluent 453 436 1.04:1
ButtonUseCssPerf.default 954 927 1.03:1
ChatWithPopoverPerf.default 454 441 1.03:1
DividerMinimalPerf.default 455 440 1.03:1
InputMinimalPerf.default 1438 1402 1.03:1
LayoutMinimalPerf.default 443 432 1.03:1
SkeletonMinimalPerf.default 443 432 1.03:1
TreeWith60ListItems.default 217 210 1.03:1
HeaderSlotsPerf.default 896 881 1.02:1
AttachmentSlotsPerf.default 1358 1341 1.01:1
BoxMinimalPerf.default 428 423 1.01:1
ButtonUseCssNestingPerf.default 1220 1211 1.01:1
CheckboxMinimalPerf.default 3098 3070 1.01:1
DatepickerMinimalPerf.default 54222 53490 1.01:1
DialogMinimalPerf.default 863 854 1.01:1
MenuMinimalPerf.default 1047 1036 1.01:1
StatusMinimalPerf.default 786 776 1.01:1
CustomToolbarPrototype.default 4234 4208 1.01:1
ToolbarMinimalPerf.default 1122 1112 1.01:1
DropdownMinimalPerf.default 3373 3385 1:1
FlexMinimalPerf.default 348 348 1:1
ItemLayoutMinimalPerf.default 1461 1464 1:1
MenuButtonMinimalPerf.default 1816 1822 1:1
ProviderMergeThemesPerf.default 1800 1796 1:1
RadioGroupMinimalPerf.default 520 520 1:1
TableMinimalPerf.default 490 488 1:1
TextAreaMinimalPerf.default 626 629 1:1
VideoMinimalPerf.default 736 739 1:1
Avatar.Fluent 402 404 1:1
Dropdown.Fluent 3432 3430 1:1
Slider.Fluent 1724 1723 1:1
Tooltip.Fluent 842 842 1:1
DropdownManyItemsPerf.default 805 812 0.99:1
EmbedMinimalPerf.default 4627 4662 0.99:1
ListMinimalPerf.default 602 606 0.99:1
LoaderMinimalPerf.default 786 805 0.98:1
Button.Fluent 659 673 0.98:1
RosterPerf.default 1309 1350 0.97:1
PortalMinimalPerf.default 194 201 0.97:1
TableManyItemsPerf.default 2227 2300 0.97:1
Dialog.Fluent 849 875 0.97:1
LabelMinimalPerf.default 439 458 0.96:1
ListCommonPerf.default 736 763 0.96:1
ListNestedPerf.default 658 682 0.96:1
AccordionMinimalPerf.default 196 207 0.95:1
ButtonMinimalPerf.default 214 225 0.95:1
ChatMinimalPerf.default 716 756 0.95:1
HeaderMinimalPerf.default 422 445 0.95:1
SliderMinimalPerf.default 1710 1798 0.95:1
CarouselMinimalPerf.default 535 567 0.94:1
ChatDuplicateMessagesPerf.default 340 361 0.94:1
ReactionMinimalPerf.default 461 493 0.94:1
AvatarMinimalPerf.default 230 247 0.93:1
Checkbox.Fluent 682 757 0.9:1

@layershifter layershifter merged commit 3b3a908 into master Apr 7, 2021
@layershifter layershifter deleted the chore/mk-consts branch April 7, 2021 14:36
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/make-styles@v9.0.0-alpha.9 has been released which incorporates this pull request.:tada:

Handy links:

miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
@layershifter @miroslavstastny
chore: use constants instead of magic numbers in makeStyles() (micros…
686788c 
…oft#17729)

* chore: use constans instead of magic numbers in makeStyles()

* Change files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miroslavstastny miroslavstastny miroslavstastny approved these changes

Assignees

@tomi-msft tomi-msft

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@layershifter @fabricteam @msft-fluent-ui-bot @miroslavstastny @tomi-msft