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

Divider: Adding initial implementation of converged component #16616

Merged
merged 87 commits into from
Mar 19, 2021
Merged

Divider: Adding initial implementation of converged component #16616

merged 87 commits into from
Mar 19, 2021

Conversation

joschemd
Copy link
Collaborator

@joschemd joschemd commented Jan 25, 2021 •
edited

Pull request checklist

Description of changes

Add react-divider package
Add Divider component
Add react-divider storybook framework
Add Divider storybook

Focus areas to test

react-divider / Divider

joschemd and others added 17 commits November 9, 2020 12:01
@joschemd
Merge pull request #1 from microsoft/master
b787a84 
Updating fork from master
@joschemd-MS
Merge remote-tracking branch 'upstream/master' into master
0cbd55d
@joschemd-MS
Merge remote-tracking branch 'upstream/master' into master
43e6873
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
af388b6
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
009a81e
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
a277f8d
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
1094236
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
1f74288
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
eaaaec1
@joschemd-MS
Merge branch 'master' of https://github.com/joschemd/fluentui
ee39953
@joschemd-MS
Merge branch 'master' of https://github.com/joschemd/fluentui
219f81f
@joschemd-MS
Merge branch 'master' of https://github.com/joschemd/fluentui
6ac8c8b
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
d4f19fe
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
188adf0
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
727d162
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
3ff0eb0
@joschemd-MS
Merge remote-tracking branch 'upstream/master'
0d7640c
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 25, 2021 •
edited

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 361d3f7:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Jan 25, 2021 •
edited

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1251 1225 5000
BaseButton mount 975 963 5000
Breadcrumb mount 44263 45942 5000
ButtonNext mount 1346 1298 5000
Checkbox mount 1734 1660 5000
CheckboxBase mount 1399 1412 5000
ChoiceGroup mount 5251 5080 5000
ComboBox mount 1005 1007 1000
CommandBar mount 10418 10396 1000
ContextualMenu mount 6372 6483 1000
DefaultButton mount 1234 1270 5000
DetailsRow mount 3834 3884 5000
DetailsRowFast mount 3962 3868 5000
DetailsRowNoStyles mount 3612 3594 5000
Dialog mount 1522 1550 1000
DocumentCardTitle mount 1882 1841 1000
Dropdown mount 3572 3529 5000
FocusTrapZone mount 1913 1894 5000
FocusZone mount 1851 1945 5000
IconButton mount 1855 1984 5000
Label mount 359 343 5000
Layer mount 1930 1935 5000
Link mount 508 478 5000
MakeStyles mount 2039 2052 50000
MenuButton mount 1598 1528 5000
MessageBar mount 2145 2099 5000
Nav mount 3510 3508 1000
OverflowSet mount 1092 1103 5000
Panel mount 1529 1516 1000
Persona mount 840 869 1000
Pivot mount 1506 1487 1000
PrimaryButton mount 1373 1353 5000
Rating mount 8478 8351 5000
SearchBox mount 1425 1427 5000
Shimmer mount 2734 2726 5000
Slider mount 2114 2063 5000
SpinButton mount 5305 5263 5000
Spinner mount 446 429 5000
SplitButton mount 3390 3368 5000
Stack mount 511 544 5000
StackWithIntrinsicChildren mount 1654 1721 5000
StackWithTextChildren mount 4956 5081 5000
SwatchColorPicker mount 10834 10820 5000
Tabs mount 1496 1483 1000
TagPicker mount 3046 3008 5000
TeachingBubble mount 12209 12026 5000
Text mount 470 454 5000
TextField mount 1463 1495 5000
ThemeProvider mount 1233 1216 5000
ThemeProvider virtual-rerender 624 630 5000
ThemeProviderNext mount 16082 16092 5000
Toggle mount 863 891 5000
buttonNative mount 112 114 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.2 0.52 0.38:1 2000 394
🦄 Button.Fluent 0.13 0.22 0.59:1 5000 669
🔧 Checkbox.Fluent 0.71 0.39 1.82:1 1000 705
🎯 Dialog.Fluent 0.17 0.23 0.74:1 5000 872
🔧 Dropdown.Fluent 3.33 0.45 7.4:1 1000 3334
🔧 Icon.Fluent 0.16 0.06 2.67:1 5000 786
🦄 Image.Fluent 0.09 0.15 0.6:1 5000 461
🔧 Slider.Fluent 1.74 0.5 3.48:1 1000 1739
🔧 Text.Fluent 0.09 0.03 3:1 5000 435
🦄 Tooltip.Fluent 0.16 0.94 0.17:1 5000 804

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
DividerMinimalPerf.default 473 435 1.09:1
ListCommonPerf.default 773 709 1.09:1
TreeWith60ListItems.default 215 197 1.09:1
FormMinimalPerf.default 539 502 1.07:1
AvatarMinimalPerf.default 251 236 1.06:1
FlexMinimalPerf.default 363 344 1.06:1
LayoutMinimalPerf.default 500 472 1.06:1
ImageMinimalPerf.default 465 441 1.05:1
LoaderMinimalPerf.default 827 787 1.05:1
ButtonMinimalPerf.default 223 215 1.04:1
ButtonUseCssPerf.default 987 950 1.04:1
ChatWithPopoverPerf.default 481 461 1.04:1
DropdownManyItemsPerf.default 847 812 1.04:1
GridMinimalPerf.default 443 426 1.04:1
RadioGroupMinimalPerf.default 554 534 1.04:1
TreeMinimalPerf.default 918 882 1.04:1
Tooltip.Fluent 804 771 1.04:1
AccordionMinimalPerf.default 199 194 1.03:1
CardMinimalPerf.default 668 646 1.03:1
MenuButtonMinimalPerf.default 1834 1786 1.03:1
SkeletonMinimalPerf.default 453 441 1.03:1
SplitButtonMinimalPerf.default 4227 4115 1.03:1
Avatar.Fluent 394 381 1.03:1
AnimationMinimalPerf.default 478 468 1.02:1
CarouselMinimalPerf.default 552 543 1.02:1
DatepickerMinimalPerf.default 51577 50647 1.02:1
DropdownMinimalPerf.default 3349 3297 1.02:1
HeaderMinimalPerf.default 456 446 1.02:1
InputMinimalPerf.default 1433 1406 1.02:1
ItemLayoutMinimalPerf.default 1385 1361 1.02:1
MenuMinimalPerf.default 1049 1029 1.02:1
ToolbarMinimalPerf.default 1126 1106 1.02:1
AttachmentSlotsPerf.default 1350 1338 1.01:1
BoxMinimalPerf.default 435 430 1.01:1
CheckboxMinimalPerf.default 3155 3131 1.01:1
PopupMinimalPerf.default 777 773 1.01:1
ProviderMergeThemesPerf.default 1695 1674 1.01:1
RefMinimalPerf.default 277 274 1.01:1
TextAreaMinimalPerf.default 579 575 1.01:1
TooltipMinimalPerf.default 1077 1065 1.01:1
Dropdown.Fluent 3334 3302 1.01:1
Icon.Fluent 786 778 1.01:1
Image.Fluent 461 457 1.01:1
Slider.Fluent 1739 1724 1.01:1
ChatMinimalPerf.default 745 744 1:1
DialogMinimalPerf.default 908 907 1:1
EmbedMinimalPerf.default 4626 4649 1:1
LabelMinimalPerf.default 508 509 1:1
SegmentMinimalPerf.default 423 421 1:1
StatusMinimalPerf.default 853 853 1:1
CustomToolbarPrototype.default 4026 4029 1:1
Button.Fluent 669 668 1:1
Checkbox.Fluent 705 705 1:1
Text.Fluent 435 436 1:1
ButtonSlotsPerf.default 667 676 0.99:1
ButtonUseCssNestingPerf.default 1246 1253 0.99:1
ChatDuplicateMessagesPerf.default 332 337 0.99:1
ListNestedPerf.default 660 665 0.99:1
RosterPerf.default 1269 1284 0.99:1
PortalMinimalPerf.default 180 182 0.99:1
SliderMinimalPerf.default 1701 1710 0.99:1
IconMinimalPerf.default 771 776 0.99:1
TableManyItemsPerf.default 2314 2338 0.99:1
TableMinimalPerf.default 491 495 0.99:1
ButtonOverridesMissPerf.default 1928 1968 0.98:1
ProviderMinimalPerf.default 1080 1097 0.98:1
ReactionMinimalPerf.default 480 492 0.98:1
VideoMinimalPerf.default 748 764 0.98:1
HeaderSlotsPerf.default 904 928 0.97:1
ListMinimalPerf.default 586 606 0.97:1
ListWith60ListItems.default 709 734 0.97:1
Dialog.Fluent 872 898 0.97:1
AlertMinimalPerf.default 341 357 0.96:1
TextMinimalPerf.default 433 456 0.95:1
AttachmentMinimalPerf.default 189 206 0.92:1

@size-auditor
Copy link

size-auditor bot commented Jan 25, 2021 •
edited

Asset size changes

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

Baseline commit: 24a23e2a84e5987096b844c73248526e3ac36a75 (build)

khmakoto
khmakoto reviewed Feb 4, 2021
View reviewed changes
Copy link
Member

@khmakoto khmakoto left a comment

Choose a reason for hiding this comment

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

A couple of comments here! Also, it would be awesome if you could split this PR into a couple of separate ones:

  • One to add the package itself.
  • One to add the spec.
  • One to add the Divider component.

packages/react-divider/README.md Outdated Show resolved Hide resolved
packages/react-divider/README.md Show resolved Hide resolved
packages/react-divider/Spec.md Outdated Show resolved Hide resolved
packages/react-divider/Spec.md Outdated Show resolved Hide resolved
packages/react-divider/Spec.md Outdated Show resolved Hide resolved
packages/react-divider/src/components/Divider/Divider.tsx Outdated Show resolved Hide resolved
packages/react-divider/src/components/Divider/Divider.types.ts Show resolved Hide resolved
packages/react-divider/src/components/Divider/renderDivider.tsx Outdated Show resolved Hide resolved
packages/react-divider/src/components/Divider/useDividerClasses.ts Outdated Show resolved Hide resolved
packages/react-examples/package.json Outdated Show resolved Hide resolved
Hotell
Hotell approved these changes Mar 16, 2021
View reviewed changes
Copy link
Contributor

@Hotell Hotell left a comment

Choose a reason for hiding this comment

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

LGTM for 3 files that I'm codeowner

ecraig12345
ecraig12345 reviewed Mar 16, 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.

Looks like some of the dep versions got messed up somehow, plus there have been some updates to the template after you used it.

packages/react-divider/package.json Outdated Show resolved Hide resolved
packages/react-divider/package.json Outdated Show resolved Hide resolved
packages/react-divider/package.json Outdated Show resolved Hide resolved
packages/react-divider/package.json Outdated Show resolved Hide resolved
packages/react-divider/package.json
Comment on lines +32 to +33
"@testing-library/react": "^10.4.9",
"@testing-library/react-hooks": "^5.0.3",
Copy link
Member

Choose a reason for hiding this comment

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

Once #17382 merges (which hopefully can happen today?) these should be listed only at the root

Suggested change
"@testing-library/react": "^10.4.9",
"@testing-library/react-hooks": "^5.0.3",

packages/react-divider/package.json Outdated Show resolved Hide resolved
@msft-fluent-ui-bot
Copy link
Collaborator

Hello @khmakoto!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msft-fluent-ui-bot) and give me an instruction to get started! Learn more here.

@ecraig12345 ecraig12345 merged commit 9d9ab2a into microsoft:master Mar 19, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-divider@v9.0.0-alpha.1 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@khmakoto khmakoto mentioned this pull request Mar 19, 2021
Merged
1 task
joshualamusga1 pushed a commit to joshualamusga1/fluentui that referenced this pull request Mar 25, 2021
@joschemd @joshualamusga1
Divider: Adding initial implementation of converged component (micros…
1005d19 
…oft#16616)
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
@joschemd @miroslavstastny
Divider: Adding initial implementation of converged component (micros…
ff061ca 
…oft#16616)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ecraig12345 ecraig12345 ecraig12345 left review comments

@layershifter layershifter layershifter left review comments

@ling1726 ling1726 ling1726 left review comments

@kolaps33 kolaps33 kolaps33 left review comments

@Hotell Hotell Hotell approved these changes

@smhigley smhigley smhigley approved these changes

@khmakoto khmakoto khmakoto approved these changes

@JustSlone JustSlone JustSlone approved these changes

@behowell behowell behowell approved these changes

@dzearing dzearing Awaiting requested review from dzearing

@xugao xugao Awaiting requested review from xugao

@assuncaocharles assuncaocharles Awaiting requested review from assuncaocharles

@joschect joschect Awaiting requested review from joschect

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@kubkon kubkon Awaiting requested review from kubkon

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@pompomon pompomon Awaiting requested review from pompomon

Assignees

@layershifter layershifter

Labels
AutoMerge Component: Separator Type: Spec Component spec PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet