Input spec #20035
Input spec #20035
Conversation
ecraig12345
requested review from
smhigley,
bsunderhus,
khmakoto,
GeoffCoxMSFT and
a team
📊 Bundle size reportUnchanged fixtures
|
|
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 d212bbd:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 0e0cf06c34f78d54db4468a602a17b0a5a30ac87 (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| buttonNative | mount | 133 | 150 | 5000 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1026 | 1001 | 5000 | |
| BaseButton | mount | 1011 | 969 | 5000 | |
| Breadcrumb | mount | 2706 | 2645 | 1000 | |
| ButtonNext | mount | 522 | 517 | 5000 | |
| Checkbox | mount | 1678 | 1711 | 5000 | |
| CheckboxBase | mount | 1412 | 1416 | 5000 | |
| ChoiceGroup | mount | 5112 | 5090 | 5000 | |
| ComboBox | mount | 995 | 1038 | 1000 | |
| CommandBar | mount | 10344 | 10308 | 1000 | |
| ContextualMenu | mount | 6734 | 6613 | 1000 | |
| DefaultButton | mount | 1217 | 1232 | 5000 | |
| DetailsRow | mount | 4033 | 4020 | 5000 | |
| DetailsRowFast | mount | 3992 | 3980 | 5000 | |
| DetailsRowNoStyles | mount | 3911 | 3845 | 5000 | |
| Dialog | mount | 2740 | 2648 | 1000 | |
| DocumentCardTitle | mount | 193 | 179 | 1000 | |
| Dropdown | mount | 3496 | 3400 | 5000 | |
| FluentProviderNext | mount | 3330 | 3317 | 5000 | |
| FluentProviderWithTheme | mount | 210 | 210 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 109 | 102 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 246 | 235 | 10 | |
| FocusTrapZone | mount | 1886 | 1897 | 5000 | |
| FocusZone | mount | 1907 | 1858 | 5000 | |
| IconButton | mount | 1892 | 1914 | 5000 | |
| Label | mount | 372 | 382 | 5000 | |
| Layer | mount | 3067 | 3109 | 5000 | |
| Link | mount | 508 | 527 | 5000 | |
| MakeStyles | mount | 1877 | 1831 | 50000 | |
| MenuButton | mount | 1586 | 1577 | 5000 | |
| MessageBar | mount | 2094 | 2057 | 5000 | |
| Nav | mount | 3649 | 3459 | 1000 | |
| OverflowSet | mount | 1130 | 1174 | 5000 | |
| Panel | mount | 2652 | 2531 | 1000 | |
| Persona | mount | 919 | 932 | 1000 | |
| Pivot | mount | 1553 | 1529 | 1000 | |
| PrimaryButton | mount | 1351 | 1396 | 5000 | |
| Rating | mount | 8565 | 8555 | 5000 | |
| SearchBox | mount | 1456 | 1492 | 5000 | |
| Shimmer | mount | 2794 | 2809 | 5000 | |
| Slider | mount | 2063 | 2098 | 5000 | |
| SpinButton | mount | 5357 | 5349 | 5000 | |
| Spinner | mount | 442 | 449 | 5000 | |
| SplitButton | mount | 3367 | 3382 | 5000 | |
| Stack | mount | 571 | 564 | 5000 | |
| StackWithIntrinsicChildren | mount | 1925 | 1881 | 5000 | |
| StackWithTextChildren | mount | 5194 | 5238 | 5000 | |
| SwatchColorPicker | mount | 11310 | 11248 | 5000 | |
| Tabs | mount | 1556 | 1541 | 1000 | |
| TagPicker | mount | 2757 | 2762 | 5000 | |
| TeachingBubble | mount | 13130 | 13390 | 5000 | |
| Text | mount | 492 | 473 | 5000 | |
| TextField | mount | 1497 | 1503 | 5000 | |
| ThemeProvider | mount | 1236 | 1222 | 5000 | |
| ThemeProvider | virtual-rerender | 642 | 623 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2045 | 2070 | 5000 | |
| Toggle | mount | 876 | 876 | 5000 | |
| buttonNative | mount | 133 | 150 | 5000 | Possible regression |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| SkeletonMinimalPerf.default | 428 | 366 | 1.17:1 |
| AvatarMinimalPerf.default | 228 | 206 | 1.11:1 |
| GridMinimalPerf.default | 386 | 353 | 1.09:1 |
| ImageMinimalPerf.default | 446 | 409 | 1.09:1 |
| SegmentMinimalPerf.default | 414 | 379 | 1.09:1 |
| BoxMinimalPerf.default | 395 | 365 | 1.08:1 |
| LoaderMinimalPerf.default | 777 | 724 | 1.07:1 |
| TreeWith60ListItems.default | 206 | 193 | 1.07:1 |
| LabelMinimalPerf.default | 417 | 393 | 1.06:1 |
| ListNestedPerf.default | 610 | 575 | 1.06:1 |
| DividerMinimalPerf.default | 420 | 401 | 1.05:1 |
| CardMinimalPerf.default | 620 | 594 | 1.04:1 |
| RadioGroupMinimalPerf.default | 499 | 479 | 1.04:1 |
| StatusMinimalPerf.default | 739 | 711 | 1.04:1 |
| TextMinimalPerf.default | 373 | 357 | 1.04:1 |
| AttachmentMinimalPerf.default | 171 | 166 | 1.03:1 |
| CarouselMinimalPerf.default | 521 | 508 | 1.03:1 |
| ListWith60ListItems.default | 701 | 682 | 1.03:1 |
| MenuMinimalPerf.default | 899 | 870 | 1.03:1 |
| PopupMinimalPerf.default | 606 | 588 | 1.03:1 |
| AlertMinimalPerf.default | 298 | 293 | 1.02:1 |
| DatepickerMinimalPerf.default | 5760 | 5672 | 1.02:1 |
| FlexMinimalPerf.default | 294 | 287 | 1.02:1 |
| HeaderSlotsPerf.default | 854 | 835 | 1.02:1 |
| ItemLayoutMinimalPerf.default | 1325 | 1305 | 1.02:1 |
| ListMinimalPerf.default | 552 | 543 | 1.02:1 |
| PortalMinimalPerf.default | 189 | 186 | 1.02:1 |
| TreeMinimalPerf.default | 859 | 846 | 1.02:1 |
| ButtonOverridesMissPerf.default | 1884 | 1858 | 1.01:1 |
| DropdownManyItemsPerf.default | 763 | 756 | 1.01:1 |
| HeaderMinimalPerf.default | 398 | 394 | 1.01:1 |
| InputMinimalPerf.default | 1381 | 1362 | 1.01:1 |
| SplitButtonMinimalPerf.default | 4576 | 4510 | 1.01:1 |
| TableMinimalPerf.default | 446 | 441 | 1.01:1 |
| AnimationMinimalPerf.default | 426 | 427 | 1:1 |
| ButtonSlotsPerf.default | 595 | 593 | 1:1 |
| EmbedMinimalPerf.default | 4509 | 4516 | 1:1 |
| MenuButtonMinimalPerf.default | 1725 | 1725 | 1:1 |
| ProviderMergeThemesPerf.default | 1711 | 1717 | 1:1 |
| ProviderMinimalPerf.default | 1204 | 1207 | 1:1 |
| ReactionMinimalPerf.default | 434 | 435 | 1:1 |
| TextAreaMinimalPerf.default | 551 | 550 | 1:1 |
| ChatWithPopoverPerf.default | 417 | 422 | 0.99:1 |
| CheckboxMinimalPerf.default | 2894 | 2917 | 0.99:1 |
| DialogMinimalPerf.default | 810 | 818 | 0.99:1 |
| SliderMinimalPerf.default | 1800 | 1813 | 0.99:1 |
| TableManyItemsPerf.default | 2072 | 2099 | 0.99:1 |
| CustomToolbarPrototype.default | 4289 | 4328 | 0.99:1 |
| TooltipMinimalPerf.default | 1123 | 1133 | 0.99:1 |
| AttachmentSlotsPerf.default | 1126 | 1148 | 0.98:1 |
| ButtonMinimalPerf.default | 190 | 194 | 0.98:1 |
| ChatMinimalPerf.default | 686 | 699 | 0.98:1 |
| DropdownMinimalPerf.default | 3215 | 3290 | 0.98:1 |
| FormMinimalPerf.default | 458 | 465 | 0.98:1 |
| LayoutMinimalPerf.default | 371 | 379 | 0.98:1 |
| ListCommonPerf.default | 675 | 689 | 0.98:1 |
| RefMinimalPerf.default | 261 | 265 | 0.98:1 |
| ToolbarMinimalPerf.default | 978 | 995 | 0.98:1 |
| ChatDuplicateMessagesPerf.default | 321 | 330 | 0.97:1 |
| RosterPerf.default | 1299 | 1353 | 0.96:1 |
| VideoMinimalPerf.default | 658 | 689 | 0.96:1 |
| IconMinimalPerf.default | 655 | 703 | 0.93:1 |
| AccordionMinimalPerf.default | 160 | 175 | 0.91:1 |
| @@ -0,0 +1,85 @@ | |||
| # Input Variants | |||
There was a problem hiding this comment.
Clarification: this is NOT the main spec and not intended to be 100% finished yet, it's just notes about tentative plans and some comparison with v8 since that's what I'm most familiar with. A detailed spec and full comparison for any of these variants will be filled out once we're closer to implementing them.
ecraig12345
force-pushed
the
input-spec
branch
from
d8cb238 to
30cea38
Compare
ecraig12345
force-pushed
the
input-spec
branch
from
30cea38 to
d5d840e
Compare
ecraig12345
force-pushed
the
input-spec
branch
from
0f0e40c to
89c3649
Compare
|
@khmakoto @micahgodbolt @layershifter @GeoffCoxMSFT I dismissed all the reviews due to making significant updates after the decision to remove bookends from the initial implementation. Please take another look. One thing I'm still undecided about is the name of the before/after slots (formerly (For those not in the Input syncs, note that we did discuss calling it |
packages/react-input/Spec-styling.md
Outdated
| @@ -11,45 +11,39 @@ General abbreviations used: | |||
| ## Sizes | |||
|
|
|||
| - padding and gap values are from (theoretical) `spacing.horizontal` unless otherwise specified | |||
| - bookend-related sizes are from separate bookends page (everything except L/R padding and spacing within inherits from default) | |||
There was a problem hiding this comment.
I updated this file to remove the parts about bookends and update naming, but it's mostly just notes for implementation purposes rather than actually being part of the formal spec, so less important to review in detail.
|
'before' and 'after' feel too much like css properties. contentBefore helps to denote what you're putting before, just as iconBefore would have a different context. I'd certainly like to be able to make a single decision on this type of naming and ensure that it is used across the entire framework. Have we done a rundown of what other frameworks call these slots? |
Agreed in general about consistency, but I'm not sure how many other components would have this specific situation. The only similar use case I can think of in other components involves specifically adding an icon (maybe to a Button or Label) which is less flexible than this and therefore could reasonably have different naming. But I could be missing something. This is what I found in other frameworks (nothing very helpful IMO):
|
|
one other bit of research to do: what other components use this pattern? where else might we need to include a pre/post something. Dig through all of the v8 components to see where else this might be used and ensure that whatever we choose would make sense in those contexts as well |
|
Merging to keep moving forward, but if there's any more feedback I can address it in the future. |
Pull request checklist
Description of changes
Add spec for Input.
Secondary things also included in this PR:
Spec-variants.mdunderreact-inputhas tentative notes about input variants we're not implementing right away. It's not intended to be 100% finished in this PR. A detailed spec and full comparison for any of these variants will be filled out once we're closer to implementing them.