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

react-card - Migrate Card to new prop merging / root as a slot #20111

Merged
merged 6 commits into from
Oct 6, 2021
Merged

react-card - Migrate Card to new prop merging / root as a slot #20111

merged 6 commits into from
Oct 6, 2021

Conversation

andrefcdias
Copy link
Contributor

Description of changes

Implementation of #18642 and #19068.

@andrefcdias andrefcdias requested a review from a team October 5, 2021 14:46
@andrefcdias andrefcdias requested a review from a team October 5, 2021 14:47
@andrefcdias andrefcdias marked this pull request as draft October 5, 2021 14:48
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 5, 2021

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-card
Card - All
13.471 kB
5.068 kB
13.377 kB
4.967 kB
-94 B
-101 B
react-card
Card
9.317 kB
3.925 kB
8.896 kB
3.753 kB
-421 B
-172 B
react-card
CardFooter
8.468 kB
3.557 kB
8.128 kB
3.424 kB
-340 B
-133 B
react-card
CardHeader
9.695 kB
3.99 kB
9.448 kB
3.878 kB
-247 B
-112 B
react-card
CardPreview
8.783 kB
3.741 kB
8.421 kB
3.599 kB
-362 B
-142 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
169.173 kB
48.002 kB
react-components
react-components: FluentProvider & webLightTheme
32.188 kB
10.658 kB
🤖 This report was generated against 1b595efc686bb5ba705c70f441e517e2036613bf

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 5, 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 d517ffc:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Oct 5, 2021

Asset size changes

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

Baseline commit: 1b595efc686bb5ba705c70f441e517e2036613bf (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 5, 2021

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 955 971 5000
BaseButton mount 945 968 5000
Breadcrumb mount 2539 2578 1000
ButtonNext mount 503 510 5000
Checkbox mount 1629 1669 5000
CheckboxBase mount 1365 1375 5000
ChoiceGroup mount 4925 4921 5000
ComboBox mount 1096 980 1000
CommandBar mount 9885 9905 1000
ContextualMenu mount 6277 6240 1000
DefaultButton mount 1158 1180 5000
DetailsRow mount 3768 3746 5000
DetailsRowFast mount 3869 3806 5000
DetailsRowNoStyles mount 3638 3642 5000
Dialog mount 2423 2453 1000
DocumentCardTitle mount 171 180 1000
Dropdown mount 3239 3286 5000
FluentProviderNext mount 3120 3104 5000
FluentProviderWithTheme mount 189 209 10
FluentProviderWithTheme virtual-rerender 112 105 10
FluentProviderWithTheme virtual-rerender-with-unmount 227 223 10
FocusTrapZone mount 1740 1776 5000
FocusZone mount 1698 1709 5000
IconButton mount 1822 1839 5000
Label mount 341 357 5000
Layer mount 2948 2993 5000
Link mount 484 517 5000
MakeStyles mount 1729 1764 50000
MenuButton mount 1531 1555 5000
MessageBar mount 1972 1950 5000
Nav mount 3394 3335 1000
OverflowSet mount 1107 1137 5000
Panel mount 2343 2344 1000
Persona mount 826 850 1000
Pivot mount 1421 1455 1000
PrimaryButton mount 1344 1299 5000
Rating mount 7998 7982 5000
SearchBox mount 1384 1422 5000
Shimmer mount 2699 2605 5000
Slider mount 1955 1986 5000
SpinButton mount 5090 5091 5000
Spinner mount 421 444 5000
SplitButton mount 3232 3230 5000
Stack mount 512 523 5000
StackWithIntrinsicChildren mount 1809 1800 5000
StackWithTextChildren mount 4950 4972 5000
SwatchColorPicker mount 10683 10626 5000
Tabs mount 1430 1471 1000
TagPicker mount 2902 2737 5000
TeachingBubble mount 12834 13028 5000
Text mount 443 448 5000
TextField mount 1444 1443 5000
ThemeProvider mount 1191 1204 5000
ThemeProvider virtual-rerender 600 606 5000
ThemeProvider virtual-rerender-with-unmount 1924 1964 5000
Toggle mount 816 821 5000
buttonNative mount 133 127 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TreeWith60ListItems.default 205 183 1.12:1
GridMinimalPerf.default 373 337 1.11:1
PortalMinimalPerf.default 180 164 1.1:1
RefMinimalPerf.default 244 224 1.09:1
AttachmentMinimalPerf.default 175 163 1.07:1
DividerMinimalPerf.default 388 363 1.07:1
DropdownManyItemsPerf.default 735 689 1.07:1
LabelMinimalPerf.default 408 383 1.07:1
ListCommonPerf.default 691 644 1.07:1
ChatMinimalPerf.default 706 664 1.06:1
ListMinimalPerf.default 529 501 1.06:1
SegmentMinimalPerf.default 376 356 1.06:1
VideoMinimalPerf.default 674 634 1.06:1
ButtonMinimalPerf.default 196 187 1.05:1
ButtonSlotsPerf.default 588 559 1.05:1
StatusMinimalPerf.default 727 691 1.05:1
AnimationMinimalPerf.default 425 408 1.04:1
ChatWithPopoverPerf.default 406 392 1.04:1
FlexMinimalPerf.default 290 278 1.04:1
FormMinimalPerf.default 446 429 1.04:1
HeaderSlotsPerf.default 806 773 1.04:1
ItemLayoutMinimalPerf.default 1259 1212 1.04:1
LayoutMinimalPerf.default 370 357 1.04:1
RadioGroupMinimalPerf.default 479 462 1.04:1
SliderMinimalPerf.default 1784 1721 1.04:1
TableManyItemsPerf.default 2087 2007 1.04:1
ImageMinimalPerf.default 404 392 1.03:1
TextAreaMinimalPerf.default 553 537 1.03:1
ChatDuplicateMessagesPerf.default 309 302 1.02:1
InputMinimalPerf.default 1323 1297 1.02:1
MenuButtonMinimalPerf.default 1683 1652 1.02:1
PopupMinimalPerf.default 591 581 1.02:1
SplitButtonMinimalPerf.default 4470 4374 1.02:1
TableMinimalPerf.default 429 420 1.02:1
CustomToolbarPrototype.default 4079 4001 1.02:1
TreeMinimalPerf.default 825 809 1.02:1
AttachmentSlotsPerf.default 1118 1109 1.01:1
AvatarMinimalPerf.default 203 201 1.01:1
ButtonOverridesMissPerf.default 1800 1783 1.01:1
DialogMinimalPerf.default 759 754 1.01:1
DropdownMinimalPerf.default 3135 3098 1.01:1
ProviderMinimalPerf.default 1130 1119 1.01:1
ReactionMinimalPerf.default 406 401 1.01:1
TextMinimalPerf.default 362 360 1.01:1
CarouselMinimalPerf.default 487 489 1:1
CheckboxMinimalPerf.default 2707 2702 1:1
DatepickerMinimalPerf.default 5497 5499 1:1
EmbedMinimalPerf.default 4353 4362 1:1
LoaderMinimalPerf.default 701 699 1:1
ProviderMergeThemesPerf.default 1607 1604 1:1
TooltipMinimalPerf.default 1051 1049 1:1
CardMinimalPerf.default 570 573 0.99:1
ListNestedPerf.default 582 589 0.99:1
ListWith60ListItems.default 653 659 0.99:1
SkeletonMinimalPerf.default 371 373 0.99:1
HeaderMinimalPerf.default 363 369 0.98:1
MenuMinimalPerf.default 845 859 0.98:1
ToolbarMinimalPerf.default 944 968 0.98:1
BoxMinimalPerf.default 353 363 0.97:1
RosterPerf.default 1267 1302 0.97:1
IconMinimalPerf.default 640 662 0.97:1
AccordionMinimalPerf.default 149 156 0.96:1
AlertMinimalPerf.default 264 275 0.96:1

@@ -6,11 +6,11 @@ exports[`CardFooter renders a default state 1`] = `
class=""
>
Default CardFooter
<span
<div
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Intended change, action slot can contain any kind of elements so it should not be a span

@varholak-peter varholak-peter marked this pull request as ready for review October 6, 2021 12:26
@varholak-peter varholak-peter self-assigned this Oct 6, 2021
Comment on lines +16 to +17
Title slot
Description slot
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an intended change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also intended, we had an extraneous div being generated.

Copy link
Contributor

@varholak-peter varholak-peter left a comment

Choose a reason for hiding this comment

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

LGTM 🇿🇲

@varholak-peter varholak-peter removed their assignment Oct 6, 2021
@andrefcdias andrefcdias merged commit a200215 into microsoft:master Oct 6, 2021
@andrefcdias andrefcdias deleted the card-prop-merge-simplification branch October 6, 2021 18:24
varholak-peter added a commit that referenced this pull request Oct 12, 2021
* Add useGroupper

* Change files

* Update API

* Rename hook

* Update API

* Rename enum

* Change options prop

* react-card - Migrate Card to new prop merging / root as a slot (#20111)

* Add focus keyboard interactions

* Update stories to have context menus

* Change files

* Remove context examples because Typescript hates me

* Update snapshots

* Use new hook name

* Formatting

* revert(react-card): remove tabster behavior from card footer

* fix: update snapshots

Co-authored-by: varholak-peter <[email protected]>
mlp73 pushed a commit to mlp73/fluentui that referenced this pull request Jan 17, 2022
mlp73 pushed a commit to mlp73/fluentui that referenced this pull request Jan 17, 2022
* Add useGroupper

* Change files

* Update API

* Rename hook

* Update API

* Rename enum

* Change options prop

* react-card - Migrate Card to new prop merging / root as a slot (microsoft#20111)

* Add focus keyboard interactions

* Update stories to have context menus

* Change files

* Remove context examples because Typescript hates me

* Update snapshots

* Use new hook name

* Formatting

* revert(react-card): remove tabster behavior from card footer

* fix: update snapshots

Co-authored-by: varholak-peter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants