-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Scaffolding Nav component #28824
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
Scaffolding Nav component #28824
Conversation
|
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 ce61309:
|
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| InfoButton | mount | 12 | 15 | 5000 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 586 | 572 | 5000 | |
| Button | mount | 311 | 310 | 5000 | |
| Field | mount | 1091 | 1068 | 5000 | |
| FluentProvider | mount | 674 | 676 | 5000 | |
| FluentProviderWithTheme | mount | 87 | 75 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 69 | 74 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 78 | 75 | 10 | |
| InfoButton | mount | 12 | 15 | 5000 | Possible regression |
| MakeStyles | mount | 850 | 870 | 50000 | |
| Persona | mount | 1691 | 1654 | 5000 | |
| SpinButton | mount | 1346 | 1390 | 5000 |
🕵 fluentuiv8 No visual regressions between this PR and main |
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| ItemLayoutMinimalPerf.default | 821 | 680 | 1.21:1 |
| AttachmentMinimalPerf.default | 91 | 81 | 1.12:1 |
| PortalMinimalPerf.default | 90 | 82 | 1.1:1 |
| ChatDuplicateMessagesPerf.default | 155 | 144 | 1.08:1 |
| ButtonSlotsPerf.default | 326 | 305 | 1.07:1 |
| DividerMinimalPerf.default | 213 | 199 | 1.07:1 |
| DropdownManyItemsPerf.default | 393 | 372 | 1.06:1 |
| FlexMinimalPerf.default | 158 | 149 | 1.06:1 |
| MenuMinimalPerf.default | 498 | 472 | 1.06:1 |
| ChatWithPopoverPerf.default | 191 | 182 | 1.05:1 |
| LabelMinimalPerf.default | 226 | 216 | 1.05:1 |
| TextMinimalPerf.default | 187 | 178 | 1.05:1 |
| EmbedMinimalPerf.default | 1874 | 1809 | 1.04:1 |
| LoaderMinimalPerf.default | 193 | 185 | 1.04:1 |
| AccordionMinimalPerf.default | 80 | 78 | 1.03:1 |
| ImageMinimalPerf.default | 233 | 226 | 1.03:1 |
| LayoutMinimalPerf.default | 201 | 195 | 1.03:1 |
| ListMinimalPerf.default | 315 | 306 | 1.03:1 |
| ToolbarMinimalPerf.default | 536 | 519 | 1.03:1 |
| AlertMinimalPerf.default | 163 | 160 | 1.02:1 |
| AvatarMinimalPerf.default | 108 | 106 | 1.02:1 |
| GridMinimalPerf.default | 193 | 189 | 1.02:1 |
| ListWith60ListItems.default | 352 | 345 | 1.02:1 |
| MenuButtonMinimalPerf.default | 945 | 931 | 1.02:1 |
| ProviderMergeThemesPerf.default | 669 | 658 | 1.02:1 |
| ProviderMinimalPerf.default | 202 | 198 | 1.02:1 |
| RefMinimalPerf.default | 108 | 106 | 1.02:1 |
| SliderMinimalPerf.default | 722 | 708 | 1.02:1 |
| StatusMinimalPerf.default | 392 | 385 | 1.02:1 |
| CheckboxMinimalPerf.default | 1144 | 1131 | 1.01:1 |
| DatepickerMinimalPerf.default | 3457 | 3436 | 1.01:1 |
| DialogMinimalPerf.default | 450 | 444 | 1.01:1 |
| HeaderMinimalPerf.default | 200 | 199 | 1.01:1 |
| ListNestedPerf.default | 314 | 310 | 1.01:1 |
| PopupMinimalPerf.default | 344 | 342 | 1.01:1 |
| SegmentMinimalPerf.default | 193 | 191 | 1.01:1 |
| TableManyItemsPerf.default | 1105 | 1096 | 1.01:1 |
| AttachmentSlotsPerf.default | 642 | 644 | 1:1 |
| ChatMinimalPerf.default | 430 | 430 | 1:1 |
| RosterPerf.default | 1528 | 1533 | 1:1 |
| SplitButtonMinimalPerf.default | 2201 | 2203 | 1:1 |
| TextAreaMinimalPerf.default | 273 | 274 | 1:1 |
| CustomToolbarPrototype.default | 1435 | 1437 | 1:1 |
| TreeWith60ListItems.default | 82 | 82 | 1:1 |
| AnimationMinimalPerf.default | 294 | 297 | 0.99:1 |
| BoxMinimalPerf.default | 192 | 193 | 0.99:1 |
| CardMinimalPerf.default | 306 | 310 | 0.99:1 |
| InputMinimalPerf.default | 520 | 524 | 0.99:1 |
| TableMinimalPerf.default | 234 | 236 | 0.99:1 |
| TooltipMinimalPerf.default | 1240 | 1247 | 0.99:1 |
| TreeMinimalPerf.default | 459 | 462 | 0.99:1 |
| CarouselMinimalPerf.default | 248 | 253 | 0.98:1 |
| DropdownMinimalPerf.default | 1402 | 1426 | 0.98:1 |
| HeaderSlotsPerf.default | 464 | 472 | 0.98:1 |
| ListCommonPerf.default | 377 | 384 | 0.98:1 |
| RadioGroupMinimalPerf.default | 251 | 257 | 0.98:1 |
| SkeletonMinimalPerf.default | 193 | 196 | 0.98:1 |
| ButtonOverridesMissPerf.default | 651 | 671 | 0.97:1 |
| IconMinimalPerf.default | 348 | 368 | 0.95:1 |
| ReactionMinimalPerf.default | 214 | 230 | 0.93:1 |
| VideoMinimalPerf.default | 396 | 424 | 0.93:1 |
| ButtonMinimalPerf.default | 84 | 91 | 0.92:1 |
| FormMinimalPerf.default | 209 | 233 | 0.9:1 |
🕵 FluentUIV0 No visual regressions between this PR and main |
🕵 fluentuiv9 No visual regressions between this PR and main |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 639 | 633 | 5000 | |
| Breadcrumb | mount | 1672 | 1632 | 1000 | |
| Checkbox | mount | 1679 | 1676 | 5000 | |
| CheckboxBase | mount | 1473 | 1482 | 5000 | |
| ChoiceGroup | mount | 2879 | 2934 | 5000 | |
| ComboBox | mount | 654 | 639 | 1000 | |
| CommandBar | mount | 6153 | 6198 | 1000 | |
| ContextualMenu | mount | 12055 | 11998 | 1000 | |
| DefaultButton | mount | 735 | 739 | 5000 | |
| DetailsRow | mount | 2205 | 2165 | 5000 | |
| DetailsRowFast | mount | 2174 | 2182 | 5000 | |
| DetailsRowNoStyles | mount | 2044 | 1991 | 5000 | |
| Dialog | mount | 2575 | 2743 | 1000 | |
| DocumentCardTitle | mount | 227 | 221 | 1000 | |
| Dropdown | mount | 1969 | 1941 | 5000 | |
| FocusTrapZone | mount | 1126 | 1126 | 5000 | |
| FocusZone | mount | 1052 | 1037 | 5000 | |
| GroupedList | mount | 40819 | 41126 | 2 | |
| GroupedList | virtual-rerender | 17564 | 19733 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 50033 | 49946 | 2 | |
| GroupedListV2 | mount | 220 | 218 | 2 | |
| GroupedListV2 | virtual-rerender | 207 | 207 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 222 | 212 | 2 | |
| IconButton | mount | 1059 | 1094 | 5000 | |
| Label | mount | 346 | 334 | 5000 | |
| Layer | mount | 2721 | 2748 | 5000 | |
| Link | mount | 390 | 394 | 5000 | |
| MenuButton | mount | 934 | 954 | 5000 | |
| MessageBar | mount | 21683 | 21711 | 5000 | |
| Nav | mount | 1908 | 1947 | 1000 | |
| OverflowSet | mount | 775 | 773 | 5000 | |
| Panel | mount | 1712 | 1782 | 1000 | |
| Persona | mount | 761 | 751 | 1000 | |
| Pivot | mount | 868 | 856 | 1000 | |
| PrimaryButton | mount | 840 | 854 | 5000 | |
| Rating | mount | 4550 | 4509 | 5000 | |
| SearchBox | mount | 925 | 902 | 5000 | |
| Shimmer | mount | 1840 | 1825 | 5000 | |
| Slider | mount | 1280 | 1319 | 5000 | |
| SpinButton | mount | 2889 | 2841 | 5000 | |
| Spinner | mount | 374 | 391 | 5000 | |
| SplitButton | mount | 1816 | 1816 | 5000 | |
| Stack | mount | 409 | 408 | 5000 | |
| StackWithIntrinsicChildren | mount | 847 | 842 | 5000 | |
| StackWithTextChildren | mount | 2588 | 2593 | 5000 | |
| SwatchColorPicker | mount | 6067 | 5981 | 5000 | |
| TagPicker | mount | 1441 | 1456 | 5000 | |
| Text | mount | 372 | 368 | 5000 | |
| TextField | mount | 905 | 918 | 5000 | |
| ThemeProvider | mount | 822 | 824 | 5000 | |
| ThemeProvider | virtual-rerender | 571 | 588 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1265 | 1280 | 5000 | |
| Toggle | mount | 585 | 611 | 5000 | |
| buttonNative | mount | 195 | 190 | 5000 |
behowell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good -- just need to make sure the package has the -preview suffix and version 0.0.0.
| "name": "@fluentui/react-nav", | ||
| "version": "9.0.0-alpha.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be @fluentui/react-nav-preview and start at version 0.0.0, according to https://github.com/microsoft/fluentui/wiki/new-release-process---v9-packages.
The create-package script should have added -preview automatically, based on the changes in #28474. Did that not happen?
cc @Hotell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct - I just gave it the name 'react-nav'
| "devDependencies": { | ||
| "@fluentui/eslint-plugin": "*", | ||
| "@fluentui/react-conformance": "*", | ||
| "@fluentui/react-conformance-griffel": "9.0.0-beta.19", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependencies on fluentui packages should use version *. Out of curiosity, was this added by the create-package script? If so, the script may need to be fixed.
| "@fluentui/react-conformance-griffel": "9.0.0-beta.19", | |
| "@fluentui/react-conformance-griffel": "*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep - added by the script
yarn.lock
Outdated
| "@fluentui/react-conformance-griffel@9.0.0-beta.19": | ||
| version "9.0.0-beta.19" | ||
| resolved "https://registry.yarnpkg.com/@fluentui/react-conformance-griffel/-/react-conformance-griffel-9.0.0-beta.19.tgz#3998ff3764c685929091dcaac5b85ad1353feda8" | ||
| integrity sha512-wJSdW5Z08Jm8jQRqXbb1fj+Dn3ifqR2gTabx3ghKnpWvwSMj8s3EVPoeoaO8phCCyIP7c5XtQDYhc+KBW3K8Cw== | ||
| dependencies: | ||
| "@griffel/react" "^1.5.2" | ||
| tslib "^2.1.0" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be any changes to yarn.lock -- this should go away once you change to use * for the version in package.json. Adding this comment to make sure that happens :)
|
Closing in favor of #28853 |
|
Closing in favor of #28853 |
This scaffolds the react-nav package for future development and to make future reviews easier.
Result of running
yarn create-packageandyarn create-component