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.

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

Moving SelectedItemsList to @fluentui/react-internal package #15430

Merged

Conversation

czearing
Copy link
Collaborator

@czearing czearing commented Oct 8, 2020

Pull request checklist

Description of changes

  1. Moving SelectedItemsList from react-next to react-internal (including strict mode fixes from Floating Pickers: Make pickers strict mode compliant #15131)
  2. Updating imports for SelectedItemsList in react-internal.
  3. Updating the top level files in react-next to reference @fluentui/react/lib/SelectedItemsList.
  4. There where no perf or vr tests for SelectedItemsListNext to be removed.
  5. Deleting react-next examples for SelectedItemsList.

@msft-github-bot
Copy link
Contributor

Thanks for submitting this change, but due to work currently in progress to prepare master for our version 8 beta release, we're asking contributors to either wait a couple weeks to submit changes (if it's not urgent) or submit to the new 7.0 branch (if it's urgent). See #15222 for more details. Thank you for your contribution and understanding!

@czearing czearing changed the title Moving SelectedPeopleList to @fluentui/react-internal package Moving SelectedItemsList to @fluentui/react-internal package Oct 8, 2020
@@ -139,8 +139,8 @@ export class ExtendedPeoplePickerControlledExample extends React.Component<{}, I
copyMenuItemText: 'Copy name',
editMenuItemText: 'Edit',
getEditingItemText: this._getEditingItemText,
onRenderFloatingPicker: FloatingPeoplePicker,
floatingPickerProps: this._floatingPickerProps,
onRenderFloatingPicker: FloatingPeoplePicker as any,
Copy link
Member

Choose a reason for hiding this comment

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

Do you know why this was needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is because the import for SelectedItemsList comes from react-internal while FloatingPeoplePicker comes from react-next.

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.

LGTM but probably @joschect should review too.

@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 913 886 5000
BaseButton mount 966 994 5000
Breadcrumb mount 162052 163780 5000
ButtonNext mount 607 584 5000
Checkbox mount 1697 1672 5000
CheckboxBase mount 1372 1380 5000
CheckboxNext mount 1623 1625 5000
ChoiceGroup mount 5139 4978 5000
ComboBox mount 965 1023 1000
CommandBar mount 22194 22045 1000
ContextualMenu mount 13578 13860 1000
DefaultButton mount 1223 1201 5000
DetailsRow mount 3834 3960 5000
DetailsRowFast mount 3847 3881 5000
DetailsRowNoStyles mount 3662 3667 5000
Dialog mount 1572 1586 1000
DocumentCardTitle mount 1838 1842 1000
Dropdown mount 2759 2756 5000
FocusTrapZone mount 1745 1781 5000
FocusTrapZoneNext mount 1867 1900 5000
FocusZone mount 1900 1842 5000
IconButton mount 1922 1925 5000
Label mount 355 372 5000
Layer mount 2008 2003 5000
LayerNext mount 2016 2013 5000
Link mount 519 481 5000
LinkNext mount 480 501 5000
MenuButton mount 1598 1590 5000
MessageBar mount 2113 2082 5000
Nav mount 3536 3441 1000
OverflowSet mount 1504 1491 5000
OverflowSetNext mount 1099 1105 5000
Panel mount 1532 1522 1000
Persona mount 950 902 1000
Pivot mount 1449 1464 1000
PivotNext mount 1527 1515 1000
PrimaryButton mount 1389 1412 5000
Rating mount 8245 8269 5000
SearchBox mount 1467 1499 5000
Shimmer mount 2753 2720 5000
Slider mount 2049 2038 5000
SliderNext mount 2019 2107 5000
SpinButton mount 5443 5424 5000
Spinner mount 444 431 5000
SplitButton mount 3416 3404 5000
Stack mount 571 560 5000
StackWithIntrinsicChildren mount 2243 2151 5000
StackWithTextChildren mount 5619 5705 5000
SwatchColorPicker mount 10999 10998 5000
TagPicker mount 2831 2893 5000
TeachingBubble mount 51604 51855 5000
Text mount 478 462 5000
TextField mount 1481 1495 5000
ThemeProvider mount 2045 2014 5000
ThemeProvider virtual-rerender 682 672 5000
Toggle mount 901 882 5000
ToggleNext mount 903 889 5000
button mount 116 127 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.5 0.54 0.93:1 2000 1002
🦄 Button.Fluent 0.13 0.22 0.59:1 5000 651
🔧 Checkbox.Fluent 0.68 0.38 1.79:1 1000 675
🎯 Dialog.Fluent 0.18 0.24 0.75:1 5000 889
🔧 Dropdown.Fluent 3.1 0.52 5.96:1 1000 3101
🔧 Icon.Fluent 0.16 0.06 2.67:1 5000 805
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 438
🔧 Slider.Fluent 1.74 0.46 3.78:1 1000 1736
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 412
🦄 Tooltip.Fluent 0.12 16.07 0.01:1 5000 620

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
CardMinimalPerf.default 699 639 1.09:1
BoxMinimalPerf.default 453 419 1.08:1
PortalMinimalPerf.default 190 176 1.08:1
AvatarMinimalPerf.default 587 549 1.07:1
ChatDuplicateMessagesPerf.default 477 446 1.07:1
SkeletonMinimalPerf.default 505 476 1.06:1
TreeMinimalPerf.default 1027 968 1.06:1
SplitButtonMinimalPerf.default 4250 4064 1.05:1
IconMinimalPerf.default 794 759 1.05:1
TextMinimalPerf.default 428 406 1.05:1
DropdownManyItemsPerf.default 842 813 1.04:1
HeaderMinimalPerf.default 457 441 1.04:1
ListMinimalPerf.default 564 544 1.04:1
ReactionMinimalPerf.default 482 463 1.04:1
TextAreaMinimalPerf.default 571 550 1.04:1
Text.Fluent 412 396 1.04:1
Tooltip.Fluent 620 594 1.04:1
AnimationMinimalPerf.default 473 458 1.03:1
ButtonOverridesMissPerf.default 1887 1830 1.03:1
FlexMinimalPerf.default 359 347 1.03:1
GridMinimalPerf.default 412 400 1.03:1
MenuMinimalPerf.default 989 958 1.03:1
PopupMinimalPerf.default 803 777 1.03:1
RefMinimalPerf.default 263 256 1.03:1
ToolbarMinimalPerf.default 1078 1046 1.03:1
Slider.Fluent 1736 1685 1.03:1
ButtonUseCssPerf.default 939 917 1.02:1
CarouselMinimalPerf.default 512 502 1.02:1
DialogMinimalPerf.default 892 877 1.02:1
DividerMinimalPerf.default 429 421 1.02:1
ItemLayoutMinimalPerf.default 1468 1439 1.02:1
RadioGroupMinimalPerf.default 502 491 1.02:1
StatusMinimalPerf.default 837 818 1.02:1
TreeWith60ListItems.default 220 216 1.02:1
Checkbox.Fluent 675 664 1.02:1
AttachmentSlotsPerf.default 1250 1235 1.01:1
ButtonMinimalPerf.default 216 213 1.01:1
ButtonSlotsPerf.default 664 657 1.01:1
ButtonUseCssNestingPerf.default 1200 1193 1.01:1
ChatMinimalPerf.default 723 714 1.01:1
LabelMinimalPerf.default 490 487 1.01:1
ListWith60ListItems.default 1019 1010 1.01:1
LoaderMinimalPerf.default 793 785 1.01:1
ProviderMinimalPerf.default 1106 1099 1.01:1
SegmentMinimalPerf.default 413 409 1.01:1
TableMinimalPerf.default 474 468 1.01:1
TooltipMinimalPerf.default 882 873 1.01:1
Avatar.Fluent 1002 994 1.01:1
Dialog.Fluent 889 877 1.01:1
AlertMinimalPerf.default 354 355 1:1
ChatWithPopoverPerf.default 534 532 1:1
CheckboxMinimalPerf.default 3067 3076 1:1
HeaderSlotsPerf.default 906 909 1:1
LayoutMinimalPerf.default 467 467 1:1
ListCommonPerf.default 746 744 1:1
MenuButtonMinimalPerf.default 1729 1733 1:1
TableManyItemsPerf.default 2498 2508 1:1
Dropdown.Fluent 3101 3091 1:1
DropdownMinimalPerf.default 3075 3113 0.99:1
EmbedMinimalPerf.default 2095 2108 0.99:1
ImageMinimalPerf.default 457 461 0.99:1
ListNestedPerf.default 640 648 0.99:1
CustomToolbarPrototype.default 4103 4150 0.99:1
FormMinimalPerf.default 503 514 0.98:1
ProviderMergeThemesPerf.default 2141 2193 0.98:1
AccordionMinimalPerf.default 179 185 0.97:1
SliderMinimalPerf.default 1665 1713 0.97:1
Icon.Fluent 805 827 0.97:1
AttachmentMinimalPerf.default 172 179 0.96:1
InputMinimalPerf.default 1395 1446 0.96:1
Image.Fluent 438 454 0.96:1
Button.Fluent 651 689 0.94:1
VideoMinimalPerf.default 712 762 0.93:1

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 8, 2020

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

Sandbox Source
Fluent UI Button Configuration
microsoft/fluentui: codesandbox-react-template Configuration
microsoft/fluentui: codesandbox-react-next-template Configuration
microsoft/fluentui: codesandbox-react-northstar-template Configuration

@size-auditor
Copy link

size-auditor bot commented Oct 8, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-SelectedItemsList 219.951 kB 220.144 kB ExceedsBaseline     193 bytes
office-ui-fabric-react fluentui-react-next-SelectedItemsList 267.174 kB  Deleted       BelowBaseline     -267.174 kB

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

Baseline commit: bbef6869c91cf388b5a1e7f33a869ef23476973c (build)

@ecraig12345 ecraig12345 merged commit 5f106c1 into microsoft:master Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants