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

fix: updates FluentCard to correctly resolve designSystem properties and set background-color / neutralPalette context #15509

Merged
merged 5 commits into from
Oct 14, 2020
Merged

fix: updates FluentCard to correctly resolve designSystem properties and set background-color / neutralPalette context #15509

merged 5 commits into from
Oct 14, 2020

Conversation

nicholasrice
Copy link
Contributor

@nicholasrice nicholasrice commented Oct 14, 2020 •
edited by chrisdholt
Loading

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ yarn change

Description of changes

This PR does several things:

  1. It bumps FAST dependencies to leverage some DesignSystemProvider bug fixes that went in.
  2. Changes FluentCard to extend the DesignSystemProvider base class instead of the FluentDesignSystemProvider. The card was previously extending FluentDesignSystemProvider as a work-around to a property resolution issue that has been fixed.
  3. It adds handling for when a custom card color is removed from a card, defaulting back to using the neutralLayerCard recipe with the parent provider's context.
  4. It removes logic that sets the card's neutral palette when the background-color property is set. This was getting invoked for every card because every card sets backgroundColor to either the product of neutralLayerCard at the parent DSP or to the cardBackgroundColor. This is the wrong behavior because in cases where the backgroundColor is the product of neutralLayerCard, the color is a product of the upstream palette and the still-valid palette exists on the FluentCards designSystem object. In cases where the backgroundColor is the cardBackgroundColor, there is already code to set the neutralPalette to be a product of the cardBackgroundColor in the cardBackgroundColorChanged() method.

Focus areas to test

FluentCard

@msft-github-bot msft-github-bot added the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Oct 14, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 14, 2020 •
edited
Loading

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 c8bc6d5:

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

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Oct 14, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
Avatar mount 933 899 5000
BaseButton mount 1056 1000 5000
Breadcrumb mount 44105 44784 5000
ButtonNext mount 773 783 5000
Checkbox mount 1749 1753 5000
CheckboxBase mount 1424 1456 5000
ChoiceGroup mount 5469 5570 5000
ComboBox mount 1020 1000 1000
CommandBar mount 8171 8114 1000
ContextualMenu mount 13806 13753 1000
DefaultButton mount 1267 1248 5000
DetailsRow mount 3982 3993 5000
DetailsRowFast mount 4092 3974 5000
DetailsRowNoStyles mount 3776 3833 5000
Dialog mount 1660 1663 1000
DocumentCardTitle mount 1930 1925 1000
Dropdown mount 2828 2871 5000
FocusTrapZone mount 1886 1871 5000
FocusZone mount 1924 1984 5000
IconButton mount 1967 1954 5000
Label mount 398 372 5000
Layer mount 2151 2156 5000
Link mount 485 509 5000
MenuButton mount 1605 1671 5000
MessageBar mount 2189 2172 5000
Nav mount 3567 3601 1000
OverflowSet mount 1556 1579 5000
Panel mount 1559 1572 1000
Persona mount 935 897 1000
Pivot mount 1558 1542 1000
PrimaryButton mount 1448 1422 5000
Rating mount 8454 8526 5000
SearchBox mount 1441 1398 5000
Shimmer mount 2884 2849 5000
Slider mount 1625 1681 5000
SpinButton mount 5474 5466 5000
Spinner mount 447 453 5000
SplitButton mount 3492 3502 5000
Stack mount 560 545 5000
StackWithIntrinsicChildren mount 1742 1706 5000
StackWithTextChildren mount 5275 5351 5000
SwatchColorPicker mount 11384 11316 5000
TagPicker mount 3010 3047 5000
TeachingBubble mount 53512 53911 5000
Text mount 490 486 5000
TextField mount 1583 1609 5000
ThemeProvider mount 1840 1808 5000
ThemeProvider virtual-rerender 670 675 5000
Toggle mount 897 940 5000
button mount 117 126 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.5 0.53 0.94:1 2000 1008
🦄 Button.Fluent 0.13 0.23 0.57:1 5000 671
🔧 Checkbox.Fluent 0.69 0.41 1.68:1 1000 689
🎯 Dialog.Fluent 0.18 0.25 0.72:1 5000 915
🔧 Dropdown.Fluent 3.21 0.52 6.17:1 1000 3205
🔧 Icon.Fluent 0.17 0.07 2.43:1 5000 856
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 454
🔧 Slider.Fluent 1.72 0.41 4.2:1 1000 1715
🔧 Text.Fluent 0.09 0.04 2.25:1 5000 468
🦄 Tooltip.Fluent 0.13 17.03 0.01:1 5000 630

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 202 0 Infinity:1
AlertMinimalPerf.default 374 0 Infinity:1
AnimationMinimalPerf.default 471 0 Infinity:1
AttachmentMinimalPerf.default 194 0 Infinity:1
AttachmentSlotsPerf.default 1332 0 Infinity:1
AvatarMinimalPerf.default 562 0 Infinity:1
BoxMinimalPerf.default 444 0 Infinity:1
ButtonMinimalPerf.default 247 0 Infinity:1
ButtonOverridesMissPerf.default 1890 0 Infinity:1
ButtonUseCssNestingPerf.default 1218 0 Infinity:1
CardMinimalPerf.default 672 0 Infinity:1
CarouselMinimalPerf.default 514 0 Infinity:1
ChatDuplicateMessagesPerf.default 488 0 Infinity:1
ChatMinimalPerf.default 710 0 Infinity:1
ChatWithPopoverPerf.default 510 0 Infinity:1
CheckboxMinimalPerf.default 3158 0 Infinity:1
DialogMinimalPerf.default 891 0 Infinity:1
DividerMinimalPerf.default 471 0 Infinity:1
DropdownManyItemsPerf.default 901 0 Infinity:1
DropdownMinimalPerf.default 3142 0 Infinity:1
EmbedMinimalPerf.default 2205 0 Infinity:1
FlexMinimalPerf.default 359 0 Infinity:1
HeaderMinimalPerf.default 476 0 Infinity:1
HeaderSlotsPerf.default 924 0 Infinity:1
ImageMinimalPerf.default 489 0 Infinity:1
InputMinimalPerf.default 1445 0 Infinity:1
ItemLayoutMinimalPerf.default 1502 0 Infinity:1
LabelMinimalPerf.default 504 0 Infinity:1
ListNestedPerf.default 730 0 Infinity:1
ListWith60ListItems.default 1041 0 Infinity:1
LoaderMinimalPerf.default 817 0 Infinity:1
MenuMinimalPerf.default 987 0 Infinity:1
PopupMinimalPerf.default 784 0 Infinity:1
PortalMinimalPerf.default 193 0 Infinity:1
ProviderMergeThemesPerf.default 2197 0 Infinity:1
ProviderMinimalPerf.default 1136 0 Infinity:1
RadioGroupMinimalPerf.default 537 0 Infinity:1
ReactionMinimalPerf.default 480 0 Infinity:1
RefMinimalPerf.default 267 0 Infinity:1
SegmentMinimalPerf.default 453 0 Infinity:1
SkeletonMinimalPerf.default 524 0 Infinity:1
SliderMinimalPerf.default 1738 0 Infinity:1
SplitButtonMinimalPerf.default 4240 0 Infinity:1
StatusMinimalPerf.default 870 0 Infinity:1
IconMinimalPerf.default 791 0 Infinity:1
TableManyItemsPerf.default 2662 0 Infinity:1
TableMinimalPerf.default 507 0 Infinity:1
TextMinimalPerf.default 414 0 Infinity:1
TextAreaMinimalPerf.default 587 0 Infinity:1
CustomToolbarPrototype.default 4257 0 Infinity:1
ToolbarMinimalPerf.default 1107 0 Infinity:1
TooltipMinimalPerf.default 926 0 Infinity:1
TreeMinimalPerf.default 1065 0 Infinity:1
TreeWith60ListItems.default 227 0 Infinity:1
VideoMinimalPerf.default 790 0 Infinity:1
Avatar.Fluent 1008 0 Infinity:1
Button.Fluent 671 0 Infinity:1
Checkbox.Fluent 689 0 Infinity:1
Dialog.Fluent 915 0 Infinity:1
Icon.Fluent 856 0 Infinity:1
Image.Fluent 454 0 Infinity:1
Slider.Fluent 1715 0 Infinity:1
Text.Fluent 468 0 Infinity:1
Tooltip.Fluent 630 0 Infinity:1
Dropdown.Fluent 3205 1 3205:1
MenuButtonMinimalPerf.default 1800 1 1800:1
ButtonUseCssPerf.default 959 1 959:1
ListCommonPerf.default 742 1 742:1
ButtonSlotsPerf.default 677 1 677:1
ListMinimalPerf.default 593 1 593:1
FormMinimalPerf.default 532 1 532:1
LayoutMinimalPerf.default 479 1 479:1
GridMinimalPerf.default 445 1 445:1

@size-auditor
Copy link

size-auditor bot commented Oct 14, 2020 •
edited
Loading

Asset size changes

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

Baseline commit: 5babd0e822f182f8a33e096e615524651ae77968 (build)

@nicholasrice nicholasrice force-pushed the users/nicholasrice/update-card-background-behavior branch from d03fe32 to c8bc6d5 Compare October 14, 2020 18:58
@chrisdholt chrisdholt merged commit 74b0bd5 into microsoft:7.0 Oct 14, 2020
@msft-github-bot
Copy link
Contributor

🎉@fluentui/web-components@v0.6.3 has been released which incorporates this pull request.:tada:

Handy links:

@khmakoto khmakoto mentioned this pull request Oct 15, 2020
Closed
40 tasks
@joschect joschect mentioned this pull request Oct 22, 2020
Merged
2 tasks
@xugao xugao removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisdholt chrisdholt chrisdholt approved these changes

Assignees

@tomi-msft tomi-msft

Labels
web-components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nicholasrice @msft-github-bot @chrisdholt @xugao @khmakoto @tomi-msft