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

Remove EventGroup from ColorRectangle, ColorSlider, and Slider #12717

Merged
merged 7 commits into from
Apr 16, 2020
Merged

Remove EventGroup from ColorRectangle, ColorSlider, and Slider #12717

merged 7 commits into from
Apr 16, 2020

Conversation

behowell
Copy link
Contributor

@behowell behowell commented Apr 15, 2020 •
edited by msft-github-bot
Loading

Pull request checklist

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

Description of changes

Replace uses of EventGroup with the 'on' function and _disposables pattern in ColorRectangle, ColorSlider, and Slider. Also update the 'on' function to accept React events.

Focus areas to test

Test mouse and touch handling in ColorPicker and Slider. This affected how those events are registered.

Microsoft Reviewers: Open in CodeFlow

@size-auditor
Copy link

size-auditor bot commented Apr 16, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react ColorPicker 85.834 kB 82.979 kB BelowBaseline     -2.855 kB
office-ui-fabric-react Slider 59.098 kB 56.227 kB BelowBaseline     -2.871 kB

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

Baseline commit: 92cefe9175b6d67e1d7cdd84272a5933901b7e6f (build)

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 861 849 5000
Checkbox 1941 1910 5000
CheckboxBase 1536 1517 5000
ChoiceGroup 5543 5720 5000
ComboBox 993 992 1000
CommandBar 7339 7349 1000
DefaultButton 1100 1067 5000
DetailsRow 3429 3356 5000
DetailsRow (fast icons) 3446 3468 5000
DetailsRow without styles 3140 3166 5000
Dialog 1362 1452 1000
DocumentCardTitle with truncation 1495 1440 1000
Dropdown 3002 2928 5000
FocusZone 1631 1489 5000
IconButton 1673 1693 5000
Label 489 499 5000
Link 437 445 5000
MenuButton 1454 1414 5000
Nav 3070 3078 1000
Panel 1438 1390 1000
Persona 831 800 1000
Pivot 1328 1359 1000
PrimaryButton 1204 1198 5000
SearchBox 1543 1549 5000
Slider 1832 1848 5000
Spinner 373 369 5000
SplitButton 3068 2988 5000
Stack 474 488 5000
Stack with Intrinsic children 1077 1111 5000
Stack with Text children 4211 4147 5000
TagPicker 2760 2756 5000
Text 392 400 5000
TextField 1714 1743 5000
Toggle 869 882 5000
button 53 64 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.52 0.52 1:1 2000 1038
🦄 Button.Fluent 0.11 0.2 0.55:1 5000 573
🔧 Checkbox.Fluent 0.66 0.43 1.53:1 1000 660
🔧 Dialog.Fluent 0.36 0.2 1.8:1 5000 1804
🔧 Dropdown.Fluent 3.07 0.47 6.53:1 1000 3072
🔧 Icon.Fluent 0.15 0.05 3:1 5000 732
🎯 Image.Fluent 0.09 0.11 0.82:1 5000 434
🔧 Slider.Fluent 1.38 0.43 3.21:1 1000 1378
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 406
🦄 Tooltip.Fluent 0.09 15.39 0.01:1 5000 452

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonSlotsPerf.default 649 562 1.15:1
HeaderMinimalPerf.default 562 507 1.11:1
ButtonMinimalPerf.default 161 146 1.1:1
AttachmentSlotsPerf.default 1223 1118 1.09:1
ListCommonPerf.default 1074 991 1.08:1
ToolbarMinimalPerf.default 1074 1008 1.07:1
ChatDuplicateMessagesPerf.default 458 431 1.06:1
PopupMinimalPerf.default 255 241 1.06:1
PortalMinimalPerf.default 320 302 1.06:1
TooltipMinimalPerf.default 740 698 1.06:1
CardMinimalPerf.default 456 434 1.05:1
DividerMinimalPerf.default 825 789 1.05:1
Image.Fluent 434 413 1.05:1
AttachmentMinimalPerf.default 163 157 1.04:1
FormMinimalPerf.default 787 756 1.04:1
GridMinimalPerf.default 716 691 1.04:1
MenuMinimalPerf.default 1883 1816 1.04:1
Icon.Fluent 732 707 1.04:1
FlexMinimalPerf.default 331 321 1.03:1
InputMinimalPerf.default 993 964 1.03:1
ItemLayoutMinimalPerf.default 1800 1753 1.03:1
LayoutMinimalPerf.default 609 589 1.03:1
ProviderMergeThemesPerf.default 1565 1516 1.03:1
TextAreaMinimalPerf.default 2824 2737 1.03:1
AccordionMinimalPerf.default 215 211 1.02:1
EmbedMinimalPerf.default 4392 4327 1.02:1
VideoMinimalPerf.default 911 897 1.02:1
Avatar.Fluent 1038 1020 1.02:1
Dialog.Fluent 1804 1767 1.02:1
Text.Fluent 406 398 1.02:1
ChatMinimalPerf.default 683 673 1.01:1
ListMinimalPerf.default 512 508 1.01:1
ListWith60ListItems.default 1197 1187 1.01:1
MenuButtonMinimalPerf.default 1582 1574 1.01:1
RadioGroupMinimalPerf.default 619 610 1.01:1
SplitButtonMinimalPerf.default 3783 3760 1.01:1
Checkbox.Fluent 660 654 1.01:1
ChatWithPopoverPerf.default 602 601 1:1
CheckboxMinimalPerf.default 3016 3028 1:1
HierarchicalTreeMinimalPerf.default 1074 1074 1:1
ImageMinimalPerf.default 408 409 1:1
LabelMinimalPerf.default 445 445 1:1
ReactionMinimalPerf.default 1904 1907 1:1
IconMinimalPerf.default 693 693 1:1
TableMinimalPerf.default 602 601 1:1
TextMinimalPerf.default 387 388 1:1
TreeMinimalPerf.default 1256 1251 1:1
BoxMinimalPerf.default 356 361 0.99:1
CarouselMinimalPerf.default 608 616 0.99:1
DialogMinimalPerf.default 1807 1819 0.99:1
DropdownMinimalPerf.default 3183 3224 0.99:1
HeaderSlotsPerf.default 1567 1580 0.99:1
SegmentMinimalPerf.default 917 927 0.99:1
CustomToolbarPrototype.default 3567 3590 0.99:1
Button.Fluent 573 580 0.99:1
Tooltip.Fluent 452 456 0.99:1
AlertMinimalPerf.default 529 539 0.98:1
ProviderMinimalPerf.default 625 636 0.98:1
SliderMinimalPerf.default 1362 1396 0.98:1
ListNestedPerf.default 930 961 0.97:1
LoaderMinimalPerf.default 761 785 0.97:1
StatusMinimalPerf.default 720 743 0.97:1
Slider.Fluent 1378 1416 0.97:1
AnimationMinimalPerf.default 677 704 0.96:1
DropdownManyItemsPerf.default 1351 1411 0.96:1
RefMinimalPerf.default 195 204 0.96:1
AvatarMinimalPerf.default 510 538 0.95:1
Dropdown.Fluent 3072 3223 0.95:1
TreeWith60ListItems.default 217 249 0.87:1

dzearing
dzearing approved these changes Apr 16, 2020
View reviewed changes
Copy link
Member

@dzearing dzearing left a comment

Choose a reason for hiding this comment

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

Nice job! :)

@dzearing
Copy link
Member

Woohoo!

image

@dzearing dzearing merged commit 971317f into microsoft:master Apr 16, 2020
@msft-github-bot
Copy link
Contributor

🎉@uifabric/utilities@v7.15.8 has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉office-ui-fabric-react@v7.106.3 has been released which incorporates this pull request.:tada:

Handy links:

@behowell behowell deleted the remove-eventgroup branch April 17, 2020 00:24
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
@behowell @DuanShaolong
Remove EventGroup from ColorRectangle, ColorSlider, and Slider (micro…
d0a6bae 
…soft#12717)

* Replace uses of EventGroup with the 'on' function and _disposabels pattern in ColorRectangle, ColorSlider, and Slider

* Change files

* Add multiple listeners to _disposables in a single call to push

* Replace usage of any

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

@ecraig12345 ecraig12345 ecraig12345 left review comments

@dzearing dzearing dzearing approved these changes

@joschect joschect Awaiting requested review from joschect

@aneeshack4 aneeshack4 Awaiting requested review from aneeshack4

Assignees

@joschect joschect

Labels
PR: API Modified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@behowell @msft-github-bot @dzearing @ecraig12345 @joschect