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

Still trigger deprecated onChanged property of Toggle #17296

Merged
merged 3 commits into from Mar 5, 2021
Merged

Still trigger deprecated onChanged property of Toggle #17296

merged 3 commits into from Mar 5, 2021

Conversation

MLoughry
Copy link
Contributor

@MLoughry MLoughry commented Mar 5, 2021

The onChanged property of Toggle is deprecated, but still exists in the typings. However, nothing in code was actually triggering the callback, which can cause regressions that are not caught at build time.

Pull request checklist

  • Include a change request file using $ yarn change

@MLoughry MLoughry requested a review from khmakoto as a code owner March 5, 2021 17:21
@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 5, 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 30e7141:

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

ecraig12345
ecraig12345 approved these changes Mar 5, 2021
View reviewed changes
Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

Thanks for noticing and fixing this!

@fabricteam
Copy link
Collaborator

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1131 1133 5000
BaseButton mount 865 861 5000
Breadcrumb mount 42274 42596 5000
ButtonNext mount 1197 1168 5000
Checkbox mount 1444 1457 5000
CheckboxBase mount 1212 1219 5000
ChoiceGroup mount 4576 4587 5000
ComboBox mount 933 946 1000
CommandBar mount 9827 9895 1000
ContextualMenu mount 6068 6042 1000
DefaultButton mount 1085 1089 5000
DetailsRow mount 3457 3491 5000
DetailsRowFast mount 3517 3493 5000
DetailsRowNoStyles mount 3268 3335 5000
Dialog mount 1417 1404 1000
DocumentCardTitle mount 1788 1773 1000
Dropdown mount 3164 3215 5000
FocusTrapZone mount 1752 1748 5000
FocusZone mount 1775 1782 5000
IconButton mount 1673 1680 5000
Label mount 331 321 5000
Layer mount 1712 1740 5000
Link mount 460 450 5000
MakeStyles mount 1969 1944 50000
MenuButton mount 1428 1405 5000
MessageBar mount 1958 1944 5000
Nav mount 3161 3178 1000
OverflowSet mount 991 1014 5000
Panel mount 1386 1392 1000
Persona mount 796 796 1000
Pivot mount 1379 1364 1000
PrimaryButton mount 1260 1217 5000
Rating mount 7204 7221 5000
SearchBox mount 1265 1260 5000
Shimmer mount 2448 2455 5000
Slider mount 1930 1903 5000
SpinButton mount 4796 4758 5000
Spinner mount 408 406 5000
SplitButton mount 3060 3041 5000
Stack mount 481 478 5000
StackWithIntrinsicChildren mount 1543 1509 5000
StackWithTextChildren mount 4411 4346 5000
SwatchColorPicker mount 9928 9915 5000
Tabs mount 1381 1380 1000
TagPicker mount 2762 2677 5000
TeachingBubble mount 11276 11352 5000
Text mount 413 412 5000
TextField mount 1325 1337 5000
ThemeProvider mount 1173 1134 5000
ThemeProvider virtual-rerender 581 604 5000
ThemeProviderNext mount 15567 15631 5000
Toggle mount 778 786 5000
buttonNative mount 120 116 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ListWith60ListItems.default 619 613 1.01:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.17 0.46 0.37:1 2000 331
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 561
🔧 Checkbox.Fluent 0.63 0.33 1.91:1 1000 631
🎯 Dialog.Fluent 0.16 0.21 0.76:1 5000 782
🔧 Dropdown.Fluent 3.06 0.41 7.46:1 1000 3061
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 667
🦄 Image.Fluent 0.08 0.12 0.67:1 5000 383
🔧 Slider.Fluent 1.59 0.45 3.53:1 1000 1591
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 377
🦄 Tooltip.Fluent 0.12 0.87 0.14:1 5000 577

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 185 165 1.12:1
TreeWith60ListItems.default 193 174 1.11:1
ToolbarMinimalPerf.default 1003 939 1.07:1
AccordionMinimalPerf.default 168 160 1.05:1
AnimationMinimalPerf.default 423 405 1.04:1
DividerMinimalPerf.default 387 373 1.04:1
AttachmentSlotsPerf.default 1212 1174 1.03:1
ChatWithPopoverPerf.default 453 439 1.03:1
FlexMinimalPerf.default 307 297 1.03:1
FormMinimalPerf.default 436 423 1.03:1
HeaderMinimalPerf.default 380 370 1.03:1
HeaderSlotsPerf.default 795 774 1.03:1
MenuButtonMinimalPerf.default 1578 1537 1.03:1
SkeletonMinimalPerf.default 388 377 1.03:1
Text.Fluent 377 365 1.03:1
ImageMinimalPerf.default 377 371 1.02:1
ListNestedPerf.default 579 566 1.02:1
MenuMinimalPerf.default 894 878 1.02:1
ProviderMinimalPerf.default 984 962 1.02:1
SegmentMinimalPerf.default 371 364 1.02:1
IconMinimalPerf.default 681 668 1.02:1
TableMinimalPerf.default 424 416 1.02:1
TextAreaMinimalPerf.default 506 495 1.02:1
ButtonUseCssNestingPerf.default 1076 1065 1.01:1
CardMinimalPerf.default 567 559 1.01:1
ChatDuplicateMessagesPerf.default 377 374 1.01:1
DatepickerMinimalPerf.default 46189 45845 1.01:1
DropdownManyItemsPerf.default 718 708 1.01:1
GridMinimalPerf.default 368 366 1.01:1
ItemLayoutMinimalPerf.default 1210 1201 1.01:1
PopupMinimalPerf.default 699 694 1.01:1
PortalMinimalPerf.default 175 173 1.01:1
RefMinimalPerf.default 242 239 1.01:1
CustomToolbarPrototype.default 3779 3731 1.01:1
TreeMinimalPerf.default 776 772 1.01:1
Button.Fluent 561 558 1.01:1
Dialog.Fluent 782 774 1.01:1
Slider.Fluent 1591 1580 1.01:1
Tooltip.Fluent 577 574 1.01:1
AvatarMinimalPerf.default 202 203 1:1
ButtonOverridesMissPerf.default 1671 1674 1:1
CarouselMinimalPerf.default 471 472 1:1
CheckboxMinimalPerf.default 2773 2764 1:1
DialogMinimalPerf.default 781 784 1:1
DropdownMinimalPerf.default 3048 3051 1:1
EmbedMinimalPerf.default 4226 4229 1:1
LabelMinimalPerf.default 424 425 1:1
ListMinimalPerf.default 513 515 1:1
ProviderMergeThemesPerf.default 1606 1604 1:1
RadioGroupMinimalPerf.default 446 447 1:1
SliderMinimalPerf.default 1589 1596 1:1
SplitButtonMinimalPerf.default 3723 3709 1:1
TableManyItemsPerf.default 2019 2014 1:1
TooltipMinimalPerf.default 826 823 1:1
Checkbox.Fluent 631 628 1:1
Dropdown.Fluent 3061 3051 1:1
Icon.Fluent 667 665 1:1
InputMinimalPerf.default 1265 1284 0.99:1
LayoutMinimalPerf.default 414 417 0.99:1
ListCommonPerf.default 628 634 0.99:1
ReactionMinimalPerf.default 416 419 0.99:1
Avatar.Fluent 331 334 0.99:1
BoxMinimalPerf.default 364 373 0.98:1
ButtonUseCssPerf.default 807 826 0.98:1
ChatMinimalPerf.default 620 632 0.98:1
LoaderMinimalPerf.default 707 723 0.98:1
StatusMinimalPerf.default 725 737 0.98:1
AlertMinimalPerf.default 291 300 0.97:1
ButtonSlotsPerf.default 565 581 0.97:1
Image.Fluent 383 394 0.97:1
ButtonMinimalPerf.default 172 181 0.95:1
VideoMinimalPerf.default 624 655 0.95:1
TextMinimalPerf.default 369 398 0.93:1
RosterPerf.default 1109 1205 0.92:1

@size-auditor
Copy link

size-auditor bot commented Mar 5, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Toggle 40.049 kB 40.171 kB ExceedsBaseline     122 bytes

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

Baseline commit: d5a8c91597cd66c0e1e03ac7cb4d8ada7e0d1b30 (build)

@MLoughry MLoughry merged commit 5e06dfb into microsoft:master Mar 5, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/theme-samples@v8.0.6 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react@v8.1.6 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-tabs@v1.0.0-beta.60 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-toggle@v1.0.0-beta.58 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-monaco-editor@v1.0.6 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-docsite-components@v8.0.6 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-slider@v1.0.0-beta.58 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-charting@v5.0.7 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/public-docsite-resources@v8.0.7 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-date-time@v8.0.6 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/public-docsite@v8.0.7 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-examples@v8.4.0 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/api-docs@v8.0.6 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/azure-themes@v8.0.6 has been released which incorporates this pull request.:tada:

Handy links:

joshualamusga1 pushed a commit to joshualamusga1/fluentui that referenced this pull request Mar 25, 2021
@MLoughry @joshualamusga1
Still trigger deprecated onChanged property of Toggle (microsoft#17296
d0bff23 
)

* Still trigger deprecated `onChanged` property of Toggle until it is removed

* Add types

* Change files
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
@MLoughry @miroslavstastny
Still trigger deprecated onChanged property of Toggle (microsoft#17296
eff3280 
)

* Still trigger deprecated `onChanged` property of Toggle until it is removed

* Add types

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

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@khmakoto khmakoto khmakoto approved these changes

Assignees

@khmakoto khmakoto

Labels
Component: Toggle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@MLoughry @fabricteam @msft-fluent-ui-bot @ecraig12345 @khmakoto