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

[Ref-Conformance] Adding a forwardRef to react-internal Separator #15678

Merged
merged 2 commits into from
Oct 24, 2020
Merged

[Ref-Conformance] Adding a forwardRef to react-internal Separator #15678

merged 2 commits into from
Oct 24, 2020

Conversation

czearing
Copy link
Collaborator

Pull request checklist

  • Include a change request file using $ yarn change

Description of changes

  • Adding a forwardRef to Separator
  • Appling the ref to the root element.
  • Extending ISeparatorProps to React.RefAttributes<HTMLDivElement>
  • Removing component-handles-ref and component-has-root-ref disabledTests from Separator.test.tsx

Focus areas to test

Separator

@czearing czearing requested a review from xugao October 23, 2020 18:27
@codesandbox-ci
Copy link

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 f790a25:

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

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 829 819 5000
BaseButtonCompat mount 897 886 5000
Breadcrumb mount 158352 157471 5000
Checkbox mount 1459 1511 5000
CheckboxBase mount 1207 1196 5000
ChoiceGroup mount 4648 4659 5000
ComboBox mount 957 943 1000
CommandBar mount 21638 21501 1000
ContextualMenu mount 6026 6114 1000
DefaultButtonCompat mount 1068 1104 5000
DetailsRow mount 3546 3496 5000
DetailsRowFast mount 3535 3572 5000
DetailsRowNoStyles mount 3318 3368 5000
Dialog mount 1462 1471 1000
DocumentCardTitle mount 1840 1793 1000
Dropdown mount 3297 3362 5000
FocusTrapZone mount 1783 1797 5000
FocusZone mount 1762 1790 5000
IconButtonCompat mount 1732 1719 5000
Label mount 326 328 5000
Layer mount 1748 1775 5000
Link mount 446 441 5000
MenuButtonCompat mount 1440 1437 5000
MessageBar mount 1999 2059 5000
Nav mount 3198 3251 1000
OverflowSet mount 1014 1037 5000
Panel mount 1404 1447 1000
Persona mount 873 861 1000
Pivot mount 1356 1424 1000
PrimaryButtonCompat mount 1265 1247 5000
Rating mount 7227 7341 5000
SearchBox mount 1273 1275 5000
Shimmer mount 2478 2510 5000
Slider mount 1876 1873 5000
SpinButton mount 4863 4882 5000
Spinner mount 419 397 5000
SplitButtonCompat mount 3354 3103 5000
Stack mount 512 503 5000
StackWithIntrinsicChildren mount 1495 1535 5000
StackWithTextChildren mount 4521 4485 5000
SwatchColorPicker mount 10020 10003 5000
TagPicker mount 2670 2758 5000
TeachingBubble mount 11421 11514 5000
Text mount 438 432 5000
TextField mount 1344 1382 5000
ThemeProvider mount 1972 1975 5000
ThemeProvider virtual-rerender 628 644 5000
Toggle mount 806 781 5000
button mount 538 562 5000
buttonNative mount 115 109 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.45 0.5 0.9:1 2000 890
🦄 Button.Fluent 0.12 0.23 0.52:1 5000 576
🔧 Checkbox.Fluent 0.66 0.34 1.94:1 1000 660
🎯 Dialog.Fluent 0.17 0.22 0.77:1 5000 825
🔧 Dropdown.Fluent 2.98 0.4 7.45:1 1000 2983
🔧 Icon.Fluent 0.15 0.06 2.5:1 5000 745
🦄 Image.Fluent 0.08 0.12 0.67:1 5000 383
🔧 Slider.Fluent 1.58 0.43 3.67:1 1000 1579
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 370
🦄 Tooltip.Fluent 0.12 0.88 0.14:1 5000 579

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 168 149 1.13:1
TreeWith60ListItems.default 213 198 1.08:1
Icon.Fluent 745 690 1.08:1
ChatMinimalPerf.default 650 605 1.07:1
ImageMinimalPerf.default 411 385 1.07:1
SkeletonMinimalPerf.default 430 402 1.07:1
ChatDuplicateMessagesPerf.default 442 418 1.06:1
IconMinimalPerf.default 703 665 1.06:1
CardMinimalPerf.default 590 561 1.05:1
DividerMinimalPerf.default 390 373 1.05:1
HeaderMinimalPerf.default 395 375 1.05:1
LayoutMinimalPerf.default 430 410 1.05:1
TextAreaMinimalPerf.default 505 483 1.05:1
Text.Fluent 370 354 1.05:1
Tooltip.Fluent 579 549 1.05:1
Checkbox.Fluent 660 634 1.04:1
ButtonMinimalPerf.default 179 174 1.03:1
ButtonSlotsPerf.default 599 580 1.03:1
ChatWithPopoverPerf.default 492 477 1.03:1
HeaderSlotsPerf.default 799 779 1.03:1
TextMinimalPerf.default 374 364 1.03:1
TreeMinimalPerf.default 899 873 1.03:1
Avatar.Fluent 890 864 1.03:1
Dialog.Fluent 825 802 1.03:1
AnimationMinimalPerf.default 423 413 1.02:1
PopupMinimalPerf.default 720 706 1.02:1
ProviderMergeThemesPerf.default 2083 2048 1.02:1
RadioGroupMinimalPerf.default 450 442 1.02:1
TableManyItemsPerf.default 2149 2115 1.02:1
AvatarMinimalPerf.default 465 462 1.01:1
CarouselMinimalPerf.default 445 440 1.01:1
DialogMinimalPerf.default 795 789 1.01:1
FormMinimalPerf.default 420 415 1.01:1
ListMinimalPerf.default 490 483 1.01:1
ListWith60ListItems.default 931 924 1.01:1
RefMinimalPerf.default 245 243 1.01:1
SegmentMinimalPerf.default 369 364 1.01:1
SliderMinimalPerf.default 1562 1550 1.01:1
SplitButtonMinimalPerf.default 3776 3730 1.01:1
CustomToolbarPrototype.default 3944 3886 1.01:1
VideoMinimalPerf.default 640 634 1.01:1
Button.Fluent 576 573 1.01:1
Dropdown.Fluent 2983 2939 1.01:1
Slider.Fluent 1579 1568 1.01:1
AttachmentMinimalPerf.default 156 156 1:1
BoxMinimalPerf.default 373 372 1:1
ButtonOverridesMissPerf.default 1684 1690 1:1
DropdownMinimalPerf.default 2931 2939 1:1
InputMinimalPerf.default 1297 1296 1:1
ListCommonPerf.default 651 653 1:1
ListNestedPerf.default 571 570 1:1
LoaderMinimalPerf.default 750 748 1:1
ProviderMinimalPerf.default 1019 1022 1:1
TableMinimalPerf.default 410 412 1:1
ToolbarMinimalPerf.default 944 940 1:1
TooltipMinimalPerf.default 806 804 1:1
ButtonUseCssPerf.default 817 827 0.99:1
ButtonUseCssNestingPerf.default 1070 1083 0.99:1
CheckboxMinimalPerf.default 2848 2884 0.99:1
DropdownManyItemsPerf.default 731 736 0.99:1
EmbedMinimalPerf.default 1933 1953 0.99:1
LabelMinimalPerf.default 424 430 0.99:1
PortalMinimalPerf.default 164 165 0.99:1
ReactionMinimalPerf.default 402 406 0.99:1
ItemLayoutMinimalPerf.default 1253 1275 0.98:1
MenuMinimalPerf.default 883 903 0.98:1
AttachmentSlotsPerf.default 1125 1164 0.97:1
FlexMinimalPerf.default 303 312 0.97:1
StatusMinimalPerf.default 728 747 0.97:1
GridMinimalPerf.default 350 363 0.96:1
MenuButtonMinimalPerf.default 1543 1602 0.96:1
Image.Fluent 383 397 0.96:1
AlertMinimalPerf.default 290 307 0.94:1

@size-auditor
Copy link

size-auditor bot commented Oct 23, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Separator 18.047 kB 18.075 kB ExceedsBaseline     28 bytes

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

Baseline commit: 4da219bab9f1ea95e24ec1b7742e46f457a271c9 (build)

@ecraig12345 ecraig12345 merged commit 8a3c81a into microsoft:master Oct 24, 2020
@msft-github-bot
Copy link
Contributor

🎉@fluentui/react-internal@v8.0.0-beta.2 has been released which incorporates this pull request.:tada:

Handy links:

SethDonohue pushed a commit to SethDonohue/fluentui that referenced this pull request Nov 2, 2020
@czearing
[Ref-Conformance] Adding a forwardRef to react-internal Separator (mi…
1bdcd3d 
…crosoft#15678)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xugao xugao xugao approved these changes

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@natalieethell natalieethell Awaiting requested review from natalieethell

Assignees

@tomi-msft tomi-msft

Labels
Component: Separator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@czearing @msft-github-bot @xugao @ecraig12345 @tomi-msft