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

fix(Ref): react on prop changes when forwardRef is used #12817

Merged
merged 3 commits into from
Apr 22, 2020
Merged

fix(Ref): react on prop changes when forwardRef is used #12817

merged 3 commits into from
Apr 22, 2020

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Apr 22, 2020 •
edited by msft-github-bot

Pull request checklist

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

Description of changes

(give an overview)

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@msft-github-bot msft-github-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Apr 22, 2020
@mnajdova mnajdova changed the title fix(Ref): update on prop changes when ForwardRef is used fix(Ref): react on prop changes when forwardRef is used Apr 22, 2020
@size-auditor
Copy link

size-auditor bot commented Apr 22, 2020

Asset size changes

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

Baseline commit: 9661499e6265141126aa4608f9224c60a81d78da (build)

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 743 712 5000
Checkbox 1375 1397 5000
CheckboxBase 1150 1167 5000
ChoiceGroup 4407 4499 5000
ComboBox 835 817 1000
CommandBar 6710 6747 1000
ContextualMenu 12438 12548 1000
DefaultButton 920 928 5000
DetailsRow 3018 2991 5000
DetailsRow (fast icons) 3057 3011 5000
DetailsRow without styles 2779 2755 5000
Dialog 1306 1273 1000
DocumentCardTitle with truncation 1474 1483 1000
Dropdown 2092 2125 5000
FocusZone 1431 1355 5000
IconButton 1514 1436 5000
Label 267 273 5000
Link 391 403 5000
MenuButton 1218 1216 5000
Nav 2761 2750 1000
Panel 1303 1264 1000
Persona 738 728 1000
Pivot 1121 1149 1000
PrimaryButton 1022 1107 5000
SearchBox 1127 1104 5000
Slider 1314 1294 5000
Spinner 328 333 5000
SplitButton 2667 2716 5000
Stack 390 405 5000
Stack with Intrinsic children 953 959 5000
Stack with Text children 3580 3610 5000
TagPicker 2418 2353 5000
Text 326 322 5000
TextField 1183 1219 5000
Toggle 753 766 5000
button 61 59 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.44 0.42 1.05:1 2000 874
🦄 Button.Fluent 0.09 0.17 0.53:1 5000 462
🔧 Checkbox.Fluent 0.6 0.3 2:1 1000 602
🔧 Dialog.Fluent 0.32 0.18 1.78:1 5000 1624
🔧 Dropdown.Fluent 3.06 0.4 7.65:1 1000 3059
🔧 Icon.Fluent 0.13 0.04 3.25:1 5000 651
🎯 Image.Fluent 0.07 0.09 0.78:1 5000 344
🔧 Slider.Fluent 1.29 0.33 3.91:1 1000 1287
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 330
🦄 Tooltip.Fluent 0.09 16.36 0.01:1 5000 443

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
FormMinimalPerf.default 758 680 1.11:1
Tooltip.Fluent 443 406 1.09:1
HeaderSlotsPerf.default 1366 1281 1.07:1
AlertMinimalPerf.default 288 271 1.06:1
SegmentMinimalPerf.default 883 835 1.06:1
TableMinimalPerf.default 507 485 1.05:1
GridMinimalPerf.default 612 591 1.04:1
Dialog.Fluent 1624 1566 1.04:1
AttachmentMinimalPerf.default 135 131 1.03:1
CarouselMinimalPerf.default 573 558 1.03:1
ChatMinimalPerf.default 603 583 1.03:1
ChatWithPopoverPerf.default 568 554 1.03:1
DividerMinimalPerf.default 654 632 1.03:1
ImageMinimalPerf.default 343 333 1.03:1
ListCommonPerf.default 928 905 1.03:1
Icon.Fluent 651 634 1.03:1
CheckboxMinimalPerf.default 2759 2700 1.02:1
FlexMinimalPerf.default 272 266 1.02:1
InputMinimalPerf.default 912 895 1.02:1
RadioGroupMinimalPerf.default 516 506 1.02:1
SliderMinimalPerf.default 1325 1303 1.02:1
VideoMinimalPerf.default 576 565 1.02:1
Button.Fluent 462 453 1.02:1
Text.Fluent 330 322 1.02:1
ButtonMinimalPerf.default 147 146 1.01:1
ButtonSlotsPerf.default 564 556 1.01:1
DropdownMinimalPerf.default 3055 3014 1.01:1
ItemLayoutMinimalPerf.default 1505 1491 1.01:1
LayoutMinimalPerf.default 508 502 1.01:1
PopupMinimalPerf.default 244 241 1.01:1
PortalMinimalPerf.default 281 277 1.01:1
ProviderMinimalPerf.default 618 614 1.01:1
TextMinimalPerf.default 333 329 1.01:1
Dropdown.Fluent 3059 3019 1.01:1
ChatDuplicateMessagesPerf.default 390 389 1:1
DropdownManyItemsPerf.default 1232 1235 1:1
HeaderMinimalPerf.default 440 438 1:1
ListNestedPerf.default 836 840 1:1
MenuMinimalPerf.default 1726 1728 1:1
IconMinimalPerf.default 604 605 1:1
ToolbarMinimalPerf.default 926 924 1:1
TooltipMinimalPerf.default 656 654 1:1
TreeWith60ListItems.default 204 203 1:1
Slider.Fluent 1287 1285 1:1
AttachmentSlotsPerf.default 1026 1038 0.99:1
BoxMinimalPerf.default 299 303 0.99:1
DialogMinimalPerf.default 1544 1552 0.99:1
EmbedMinimalPerf.default 3971 3993 0.99:1
LabelMinimalPerf.default 368 370 0.99:1
LoaderMinimalPerf.default 754 760 0.99:1
StatusMinimalPerf.default 619 625 0.99:1
TextAreaMinimalPerf.default 2514 2549 0.99:1
CustomToolbarPrototype.default 3414 3434 0.99:1
TreeMinimalPerf.default 1063 1069 0.99:1
AnimationMinimalPerf.default 590 602 0.98:1
CardMinimalPerf.default 519 531 0.98:1
HierarchicalTreeMinimalPerf.default 881 898 0.98:1
ListMinimalPerf.default 429 437 0.98:1
ListWith60ListItems.default 1069 1088 0.98:1
MenuButtonMinimalPerf.default 1424 1453 0.98:1
ReactionMinimalPerf.default 1715 1744 0.98:1
RefMinimalPerf.default 185 188 0.98:1
SplitButtonMinimalPerf.default 3427 3485 0.98:1
Avatar.Fluent 874 891 0.98:1
Image.Fluent 344 351 0.98:1
ProviderMergeThemesPerf.default 1530 1576 0.97:1
AvatarMinimalPerf.default 474 498 0.95:1
Checkbox.Fluent 602 637 0.95:1
AccordionMinimalPerf.default 178 200 0.89:1

@mnajdova mnajdova merged commit bb5b0d7 into microsoft:master Apr 22, 2020
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
@mnajdova @DuanShaolong
fix(Ref): react on prop changes when forwardRef is used (microsoft#12817
b82d89c 
)

* -fix + test

* -added more tests

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

@pompomon pompomon pompomon approved these changes

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@layershifter layershifter Awaiting requested review from layershifter

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

Assignees

@joschect joschect

Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mnajdova @msft-github-bot @pompomon @joschect