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

Accordion Convergence #16969

Merged
merged 72 commits into from
Mar 11, 2021
Merged

Accordion Convergence #16969

merged 72 commits into from
Mar 11, 2021

Conversation

bsunderhus
Copy link
Contributor

@bsunderhus bsunderhus commented Feb 12, 2021

Pull request checklist

  • Runs yarn change

Description of changes

  • Scaffolds react-accordion project
  • Creates Specification for discussion

Focus areas to test

  • Specification
  • Implementation (without A11y)

Completes some tasks from #16926

@ghost
Copy link

ghost commented Feb 12, 2021

CLA assistant check
All CLA requirements met.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 12, 2021

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 6c126d0:

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

@size-auditor
Copy link

size-auditor bot commented Feb 12, 2021

Asset size changes

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

Baseline commit: 1148bc99d2dd9a9d8452ab1d95d83e7d9ab6cf1d (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 12, 2021

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1278 1285 5000
BaseButton mount 1001 997 5000
Breadcrumb mount 45578 45544 5000
ButtonNext mount 1453 1406 5000
Checkbox mount 1621 1649 5000
CheckboxBase mount 1398 1411 5000
ChoiceGroup mount 5274 5199 5000
ComboBox mount 1034 1045 1000
CommandBar mount 10654 10681 1000
ContextualMenu mount 6507 6518 1000
DefaultButton mount 1246 1223 5000
DetailsRow mount 4028 3917 5000
DetailsRowFast mount 3894 3908 5000
DetailsRowNoStyles mount 3797 3732 5000
Dialog mount 1595 1593 1000
DocumentCardTitle mount 1943 1928 1000
Dropdown mount 3623 3625 5000
FocusTrapZone mount 1952 1954 5000
FocusZone mount 1846 1904 5000
IconButton mount 1934 1958 5000
Label mount 340 356 5000
Layer mount 1944 1982 5000
Link mount 503 501 5000
MakeStyles mount 2133 2115 50000
MenuButton mount 1664 1619 5000
MessageBar mount 2126 2118 5000
Nav mount 3578 3570 1000
OverflowSet mount 1128 1105 5000
Panel mount 1508 1557 1000
Persona mount 875 876 1000
Pivot mount 1532 1523 1000
PrimaryButton mount 1411 1399 5000
Rating mount 8317 8393 5000
SearchBox mount 1466 1487 5000
Shimmer mount 2834 2810 5000
Slider mount 2154 2159 5000
SpinButton mount 5375 5388 5000
Spinner mount 437 432 5000
SplitButton mount 3424 3483 5000
Stack mount 545 543 5000
StackWithIntrinsicChildren mount 1680 1701 5000
StackWithTextChildren mount 5048 5077 5000
SwatchColorPicker mount 11058 11041 5000
Tabs mount 1493 1520 1000
TagPicker mount 3056 3067 5000
TeachingBubble mount 12400 12404 5000
Text mount 463 456 5000
TextField mount 1511 1552 5000
ThemeProvider mount 1267 1249 5000
ThemeProvider virtual-rerender 640 644 5000
ThemeProviderNext mount 17097 16891 5000
Toggle mount 892 965 5000
buttonNative mount 121 126 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.2 0.51 0.39:1 2000 392
🦄 Button.Fluent 0.13 0.22 0.59:1 5000 674
🔧 Checkbox.Fluent 0.69 0.39 1.77:1 1000 685
🎯 Dialog.Fluent 0.18 0.24 0.75:1 5000 895
🔧 Dropdown.Fluent 3.33 0.44 7.57:1 1000 3325
🔧 Icon.Fluent 0.16 0.07 2.29:1 5000 787
🦄 Image.Fluent 0.09 0.14 0.64:1 5000 462
🔧 Slider.Fluent 1.67 0.49 3.41:1 1000 1674
🔧 Text.Fluent 0.09 0.03 3:1 5000 439
🦄 Tooltip.Fluent 0.15 0.96 0.16:1 5000 756

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
RefMinimalPerf.default 269 252 1.07:1
ImageMinimalPerf.default 476 447 1.06:1
ListNestedPerf.default 699 657 1.06:1
TreeWith60ListItems.default 211 200 1.06:1
AlertMinimalPerf.default 348 332 1.05:1
DropdownManyItemsPerf.default 863 824 1.05:1
TextAreaMinimalPerf.default 617 590 1.05:1
AvatarMinimalPerf.default 234 226 1.04:1
CardMinimalPerf.default 680 652 1.04:1
HeaderMinimalPerf.default 452 436 1.04:1
LoaderMinimalPerf.default 810 776 1.04:1
TextMinimalPerf.default 432 415 1.04:1
ToolbarMinimalPerf.default 1109 1071 1.04:1
Button.Fluent 674 649 1.04:1
AttachmentSlotsPerf.default 1362 1317 1.03:1
ButtonSlotsPerf.default 681 660 1.03:1
CarouselMinimalPerf.default 553 539 1.03:1
FormMinimalPerf.default 518 503 1.03:1
LayoutMinimalPerf.default 484 470 1.03:1
ListWith60ListItems.default 742 718 1.03:1
ProviderMinimalPerf.default 1011 986 1.03:1
ReactionMinimalPerf.default 487 475 1.03:1
TableMinimalPerf.default 491 479 1.03:1
TooltipMinimalPerf.default 1072 1039 1.03:1
Avatar.Fluent 392 382 1.03:1
AccordionMinimalPerf.default 183 179 1.02:1
CheckboxMinimalPerf.default 3125 3070 1.02:1
DividerMinimalPerf.default 443 434 1.02:1
FlexMinimalPerf.default 352 346 1.02:1
HeaderSlotsPerf.default 924 909 1.02:1
LabelMinimalPerf.default 494 484 1.02:1
ListCommonPerf.default 780 766 1.02:1
ListMinimalPerf.default 602 592 1.02:1
PopupMinimalPerf.default 768 750 1.02:1
PortalMinimalPerf.default 179 175 1.02:1
RadioGroupMinimalPerf.default 524 512 1.02:1
SegmentMinimalPerf.default 433 425 1.02:1
SkeletonMinimalPerf.default 445 435 1.02:1
Dialog.Fluent 895 878 1.02:1
Dropdown.Fluent 3325 3260 1.02:1
ButtonOverridesMissPerf.default 1842 1829 1.01:1
ButtonUseCssPerf.default 924 917 1.01:1
ChatDuplicateMessagesPerf.default 333 329 1.01:1
DialogMinimalPerf.default 897 886 1.01:1
DropdownMinimalPerf.default 3299 3268 1.01:1
EmbedMinimalPerf.default 4633 4608 1.01:1
InputMinimalPerf.default 1411 1401 1.01:1
MenuButtonMinimalPerf.default 1807 1790 1.01:1
SplitButtonMinimalPerf.default 4219 4180 1.01:1
TableManyItemsPerf.default 2399 2367 1.01:1
Icon.Fluent 787 780 1.01:1
BoxMinimalPerf.default 430 432 1:1
ButtonMinimalPerf.default 212 213 1:1
ButtonUseCssNestingPerf.default 1176 1172 1:1
ChatMinimalPerf.default 714 713 1:1
ChatWithPopoverPerf.default 433 431 1:1
DatepickerMinimalPerf.default 48076 48222 1:1
ItemLayoutMinimalPerf.default 1415 1408 1:1
StatusMinimalPerf.default 846 847 1:1
CustomToolbarPrototype.default 3960 3945 1:1
Image.Fluent 462 462 1:1
Slider.Fluent 1674 1681 1:1
Tooltip.Fluent 756 758 1:1
MenuMinimalPerf.default 1009 1018 0.99:1
IconMinimalPerf.default 772 782 0.99:1
VideoMinimalPerf.default 760 767 0.99:1
AnimationMinimalPerf.default 452 461 0.98:1
GridMinimalPerf.default 407 417 0.98:1
RosterPerf.default 1323 1349 0.98:1
ProviderMergeThemesPerf.default 1653 1680 0.98:1
SliderMinimalPerf.default 1652 1690 0.98:1
Checkbox.Fluent 685 702 0.98:1
Text.Fluent 439 446 0.98:1
AttachmentMinimalPerf.default 197 204 0.97:1
TreeMinimalPerf.default 896 920 0.97:1

@bsunderhus bsunderhus marked this pull request as draft February 15, 2021 16:25
@bsunderhus bsunderhus changed the title WIP: Accordion Convergence Accordion Convergence Feb 15, 2021
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.

Some comments on build setup--I think most of these have been fixed in the template but may have been after you created your package.

packages/react-accordion/package.json Outdated Show resolved Hide resolved
packages/react-accordion/package.json Outdated Show resolved Hide resolved
packages/react-accordion/tsconfig.json Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@bsunderhus
Copy link
Contributor Author

Seems like @reach/descendants requires Typescript 3.8, due to Type-only Export. #16101 may be required.

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 from a build perspective once the issues @layershifter mentioned about versions are addressed.

Also mentioned this in a thread comment but repeating here so it doesn't get buried: it might be worth filing a syncpack issue asking for an explicit option determining how to resolve mismatches (we usually prefer newest, but others might prefer most popular). The author has been pretty helpful in the past. https://github.com/JamieMason/syncpack

bsunderhus and others added 2 commits March 10, 2021 03:08
…sunderhus/accordion-convergence

� Conflicts:
�	packages/react-examples/package.json
@bsunderhus bsunderhus merged commit 29a6c8d into master Mar 11, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-accordion@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.4.4 has been released which incorporates this pull request.:tada:

Handy links:

@layershifter layershifter deleted the bsunderhus/accordion-convergence branch March 19, 2021 12:39
joshualamusga1 pushed a commit to joshualamusga1/fluentui that referenced this pull request Mar 25, 2021
* Scaffold react-accordion project

* Writes initial Spec

* Change files

* Updates API

* Bumps version of dependencies

* Update Test

* Updates API

* Updates spec

* Updates Spec

* Update deps

* Delete components

* Add components scaffolding

* Update AccordionHeader

* Updates button slot

* Updates accordion header and item

* Update Accordion context

* Update Accordion context

* Add examples

* Update packages/react-accordion/package.json

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Update packages/react-accordion/tsconfig.json

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Update dependencies and version

* Update accordion logic

* Change useRef for useConst

* Update change/@fluentui-react-accordion-0dc572ee-df54-4e96-923d-db68fe030532.json

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Update packages/react-accordion/jest.config.js

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Adds ability helpers to react-accordion examples

* Update styles and spec

* Update spec

* Update styles

* Add to AccordionHeader children as a slot

* Update packages/react-accordion/Spec.md

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/package.json

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/package.json

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/src/components/Accordion/useAccordionContext.ts

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update specs

* use useControllable hook to control Accordion open state

* Update versions

* Lint

* Updates accordion internals to normalize indexes

* updates API

* Change files

* Updates Accordion comments

* Fix sb problems with withCompatThemeProvider

* Adds AccordionHeader common props to Accordion context

* Adds rendering test

* Update AccordionContext to use context selector

* Adds useAccordion test

* Deletes unecessary change files

* Fiz prelint issues

* Change files

* Update Accordion public API

* Fix AMD issues

* Update packages/react-examples/.storybook/preview.js

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update scripts/format.js

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/package.json

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Bump "@fluentui/react-utilities" dependency version

* Update icon

* Update DefaultIcon and internal signatures

* updates wrongly bumped dependencies

* Deletes Cahnge file

* Change files

* Updates DefaultExpandIcon internal Chevron

* Updates public API

* Fix tests

* fix versions, remove change files, restore changes

* Update makeStyles to makeStylesCompat

* Removes unused style file

* Update snapshot

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>
Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
* Scaffold react-accordion project

* Writes initial Spec

* Change files

* Updates API

* Bumps version of dependencies

* Update Test

* Updates API

* Updates spec

* Updates Spec

* Update deps

* Delete components

* Add components scaffolding

* Update AccordionHeader

* Updates button slot

* Updates accordion header and item

* Update Accordion context

* Update Accordion context

* Add examples

* Update packages/react-accordion/package.json

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Update packages/react-accordion/tsconfig.json

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Update dependencies and version

* Update accordion logic

* Change useRef for useConst

* Update change/@fluentui-react-accordion-0dc572ee-df54-4e96-923d-db68fe030532.json

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Update packages/react-accordion/jest.config.js

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>

* Adds ability helpers to react-accordion examples

* Update styles and spec

* Update spec

* Update styles

* Add to AccordionHeader children as a slot

* Update packages/react-accordion/Spec.md

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/package.json

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/package.json

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/src/components/Accordion/useAccordionContext.ts

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update specs

* use useControllable hook to control Accordion open state

* Update versions

* Lint

* Updates accordion internals to normalize indexes

* updates API

* Change files

* Updates Accordion comments

* Fix sb problems with withCompatThemeProvider

* Adds AccordionHeader common props to Accordion context

* Adds rendering test

* Update AccordionContext to use context selector

* Adds useAccordion test

* Deletes unecessary change files

* Fiz prelint issues

* Change files

* Update Accordion public API

* Fix AMD issues

* Update packages/react-examples/.storybook/preview.js

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update scripts/format.js

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update packages/react-accordion/package.json

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Bump "@fluentui/react-utilities" dependency version

* Update icon

* Update DefaultIcon and internal signatures

* updates wrongly bumped dependencies

* Deletes Cahnge file

* Change files

* Updates DefaultExpandIcon internal Chevron

* Updates public API

* Fix tests

* fix versions, remove change files, restore changes

* Update makeStyles to makeStylesCompat

* Removes unused style file

* Update snapshot

Co-authored-by: Elizabeth Craig <elcraig@microsoft.com>
Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants