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

Renaming special-purpose Grid component to ButtonGrid within react-next #14854

Merged
merged 14 commits into from
Sep 8, 2020
Merged

Renaming special-purpose Grid component to ButtonGrid within react-next #14854

merged 14 commits into from
Sep 8, 2020

Conversation

czearing
Copy link
Collaborator

@czearing czearing commented Sep 1, 2020 •
edited by ecraig12345
Loading

Pull request checklist

Description of changes

Renaming Grid utility within react-next to ButtonGrid.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 1, 2020 •
edited
Loading

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

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

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Sep 2, 2020 •
edited
Loading

Perf Analysis

Scenario Render type Master Ticks PR Ticks Iterations Status
SwatchColorPickerNext mount 10022 10117 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 897 869 5000
ButtonNext mount 586 577 5000
Checkbox mount 1547 1624 5000
CheckboxBase mount 1335 1347 5000
CheckboxNext mount 1627 1611 5000
ChoiceGroup mount 4928 5029 5000
ChoiceGroupNext mount 5008 4898 5000
ComboBox mount 909 903 1000
CommandBar mount 7770 7676 1000
ContextualMenu mount 14964 14962 1000
DefaultButton mount 1101 1078 5000
DetailsRow mount 3604 3581 5000
DetailsRowFast mount 3531 3413 5000
DetailsRowNoStyles mount 3466 3281 5000
Dialog mount 1459 1519 1000
DocumentCardTitle mount 1803 1839 1000
Dropdown mount 2597 2581 5000
FocusZone mount 1823 1802 5000
IconButton mount 1691 1765 5000
Label mount 340 322 5000
Link mount 437 461 5000
LinkNext mount 479 476 5000
MenuButton mount 1383 1442 5000
MessageBar mount 2110 2064 5000
MessageBarNext mount 2109 2083 5000
Nav mount 3274 3269 1000
OverflowSet mount 1444 1362 5000
OverflowSetNext mount 1075 1012 5000
Panel mount 1479 1494 1000
Persona mount 829 884 1000
Pivot mount 1441 1385 1000
PivotNext mount 1357 1341 1000
PrimaryButton mount 1250 1233 5000
SearchBox mount 1240 1253 5000
SearchBoxNext mount 1345 1304 5000
Shimmer mount 2570 2596 5000
ShimmerNext mount 2528 2570 5000
Slider mount 1485 1532 5000
SliderNext mount 1964 1966 5000
SpinButton mount 4924 5027 5000
SpinButtonNext mount 5055 5120 5000
Spinner mount 426 412 5000
SplitButton mount 3109 3106 5000
Stack mount 506 497 5000
StackWithIntrinsicChildren mount 1896 1925 5000
StackWithTextChildren mount 5102 4998 5000
SwatchColorPicker mount 10199 10303 5000
SwatchColorPickerNext mount 10022 10117 5000 Possible regression
TagPicker mount 2759 2733 5000
TeachingBubble mount 51561 51584 5000
TeachingBubbleNext mount 51159 51464 5000
Text mount 419 426 5000
TextField mount 1350 1385 5000
ThemeProvider mount 4315 4314 5000
ThemeProvider virtual-rerender 455 477 5000
Toggle mount 843 873 5000
ToggleNext mount 810 789 5000
button mount 117 121 5000

Perf Analysis (Fluent)

⚠️ 6 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonOverridesMissPerf.default 1741 45 38.69:1 analysis
ButtonUseCssNestingPerf.default 1133 51 22.22:1 analysis
ButtonUseCssPerf.default 869 46 18.89:1 analysis
ChatWithPopoverPerf.default 481 467 1.03:1 analysis
ListCommonPerf.default 662 976 0.68:1 analysis
ListNestedPerf.default 589 866 0.68:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.45 0.49 0.92:1 2000 901
🦄 Button.Fluent 0.11 0.2 0.55:1 5000 558
🔧 Checkbox.Fluent 0.66 0.36 1.83:1 1000 656
🎯 Dialog.Fluent 0.16 0.23 0.7:1 5000 814
🔧 Dropdown.Fluent 2.98 0.51 5.84:1 1000 2984
🔧 Icon.Fluent 0.14 0.06 2.33:1 5000 722
🦄 Image.Fluent 0.08 0.12 0.67:1 5000 408
🔧 Slider.Fluent 1.61 0.38 4.24:1 1000 1613
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 377
🦄 Tooltip.Fluent 0.12 18.58 0.01:1 5000 601

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 192 140 1.37:1
PortalMinimalPerf.default 164 125 1.31:1
RefMinimalPerf.default 262 204 1.28:1
AnimationMinimalPerf.default 450 370 1.22:1
TableMinimalPerf.default 440 362 1.22:1
FormMinimalPerf.default 448 371 1.21:1
StatusMinimalPerf.default 782 647 1.21:1
HeaderMinimalPerf.default 407 340 1.2:1
SegmentMinimalPerf.default 382 319 1.2:1
GridMinimalPerf.default 388 326 1.19:1
Text.Fluent 377 318 1.19:1
BoxMinimalPerf.default 377 319 1.18:1
SkeletonMinimalPerf.default 456 385 1.18:1
RadioGroupMinimalPerf.default 457 390 1.17:1
FlexMinimalPerf.default 326 281 1.16:1
Image.Fluent 408 353 1.16:1
ButtonMinimalPerf.default 201 175 1.15:1
LabelMinimalPerf.default 443 388 1.14:1
AvatarMinimalPerf.default 506 449 1.13:1
ReactionMinimalPerf.default 414 365 1.13:1
Tooltip.Fluent 601 532 1.13:1
ChatMinimalPerf.default 676 602 1.12:1
DialogMinimalPerf.default 832 743 1.12:1
LayoutMinimalPerf.default 425 381 1.12:1
TextMinimalPerf.default 375 336 1.12:1
DividerMinimalPerf.default 382 344 1.11:1
IconMinimalPerf.default 693 626 1.11:1
TooltipMinimalPerf.default 840 758 1.11:1
CardMinimalPerf.default 581 530 1.1:1
Dialog.Fluent 814 750 1.09:1
HeaderSlotsPerf.default 809 746 1.08:1
ImageMinimalPerf.default 388 360 1.08:1
Button.Fluent 558 517 1.08:1
CarouselMinimalPerf.default 475 442 1.07:1
EmbedMinimalPerf.default 1963 1838 1.07:1
ProviderMinimalPerf.default 1000 931 1.07:1
ToolbarMinimalPerf.default 1013 943 1.07:1
TreeMinimalPerf.default 907 849 1.07:1
PopupMinimalPerf.default 730 686 1.06:1
TextAreaMinimalPerf.default 476 450 1.06:1
VideoMinimalPerf.default 647 610 1.06:1
ButtonSlotsPerf.default 627 600 1.05:1
ItemLayoutMinimalPerf.default 1304 1244 1.05:1
MenuButtonMinimalPerf.default 1593 1513 1.05:1
ProviderMergeThemesPerf.default 2009 1914 1.05:1
Avatar.Fluent 901 859 1.05:1
ListMinimalPerf.default 502 482 1.04:1
Checkbox.Fluent 656 630 1.04:1
Icon.Fluent 722 692 1.04:1
AlertMinimalPerf.default 299 290 1.03:1
AttachmentSlotsPerf.default 1183 1145 1.03:1
InputMinimalPerf.default 1400 1361 1.03:1
TableManyItemsPerf.default 2180 2112 1.03:1
CustomToolbarPrototype.default 3875 3758 1.03:1
CheckboxMinimalPerf.default 2900 2840 1.02:1
SplitButtonMinimalPerf.default 3817 3728 1.02:1
TreeWith60ListItems.default 207 203 1.02:1
DropdownMinimalPerf.default 2967 2944 1.01:1
SliderMinimalPerf.default 1625 1602 1.01:1
DropdownManyItemsPerf.default 769 772 1:1
MenuMinimalPerf.default 854 858 1:1
Dropdown.Fluent 2984 2997 1:1
LoaderMinimalPerf.default 754 759 0.99:1
AccordionMinimalPerf.default 166 170 0.98:1
ChatDuplicateMessagesPerf.default 416 425 0.98:1
Slider.Fluent 1613 1638 0.98:1
ListWith60ListItems.default 965 1080 0.89:1

@size-auditor
Copy link

size-auditor bot commented Sep 2, 2020 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-next-ButtonGrid New   172.114 kB ExceedsTolerance     172.114 kB
office-ui-fabric-react fluentui-react-next-Grid 172.616 kB  Deleted       BelowBaseline     -172.616 kB

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

Baseline commit: d1423ec6ee302d74966497ba4b8f6abb022aa884 (build)

ecraig12345
ecraig12345 requested changes Sep 2, 2020
View reviewed changes
Copy link
Member

@ecraig12345 ecraig12345 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think you're missing updates to react-next/src/Grid.ts -- renaming the file, making it export the react-next version, and updating the API file

ecraig12345
ecraig12345 approved these changes Sep 3, 2020
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.

Approved with a couple minor suggestions.

change/office-ui-fabric-react-2020-09-02-18-45-05-feat-renaming-gridNext.json Outdated Show resolved Hide resolved
packages/office-ui-fabric-react/src/components/Calendar/CalendarDay.tsx Show resolved Hide resolved
packages/office-ui-fabric-react/src/components/Pivot/Pivot.base.tsx Show resolved Hide resolved
@ecraig12345
Copy link
Member

Build is failing with a bunch of weird errors--I think you can fix it by adding the first two tsconfig file changes from here.

@czearing czearing requested a review from JasonGore as a code owner September 4, 2020 21:36
@czearing czearing requested a review from xugao as a code owner September 4, 2020 21:36
@ecraig12345 ecraig12345 merged commit 3111738 into microsoft:master Sep 8, 2020
hutchcodes pushed a commit to hutchcodes/fluentui that referenced this pull request Sep 10, 2020
@czearing @hutchcodes
Renaming special-purpose Grid component to ButtonGrid within react-ne…
d5e5ccf 
…xt (microsoft#14854)
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

@dzearing dzearing dzearing approved these changes

@behowell behowell Awaiting requested review from behowell

@lorejoh12 lorejoh12 Awaiting requested review from lorejoh12

@pompomon pompomon Awaiting requested review from pompomon

@JasonGore JasonGore Awaiting requested review from JasonGore

@xugao xugao Awaiting requested review from xugao

Assignees
No one assigned
Labels
Component: SwatchPicker PR: Possible Perf Regression 🐌
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rename special-purpose Grid component to ButtonGrid
4 participants
@czearing @msft-github-bot @ecraig12345 @dzearing