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: correct palette generation behavior in Card #15429

Merged
merged 3 commits into from
Oct 9, 2020
Merged

fix: correct palette generation behavior in Card #15429

merged 3 commits into from
Oct 9, 2020

Conversation

nicholasrice
Copy link
Contributor

@nicholasrice nicholasrice commented Oct 8, 2020 •
edited
Loading

Pull request checklist

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

Description of changes

The FluentCard was always generating a neutralPalette when a card-background-color was unset (this is most cases) which is undesirable for several reasons:

  1. Generating palettes is reasonably expensive.
  2. The palette source was the incoming background color, which results in a different palette than the upstream palette, which is generally undesirable.

This PR changes the behavior to use the upstream design-system to generate the card's background, which is the correct behavior when no local overrides have been set. It also adds some null checking that prevents runtime errors.
(give an overview)

Focus areas to test

FluentCard component.

@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 8, 2020
@msft-github-bot
Copy link
Contributor

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

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
Avatar mount 883 882 5000
BaseButton mount 1003 996 5000
Breadcrumb mount 42738 43108 5000
ButtonNext mount 624 586 5000
Checkbox mount 1739 1681 5000
CheckboxBase mount 1429 1468 5000
ChoiceGroup mount 5430 5518 5000
ComboBox mount 994 977 1000
CommandBar mount 7987 7986 1000
ContextualMenu mount 13368 13426 1000
DefaultButton mount 1196 1177 5000
DetailsRow mount 3825 3799 5000
DetailsRowFast mount 3863 3826 5000
DetailsRowNoStyles mount 3619 3594 5000
Dialog mount 1563 1574 1000
DocumentCardTitle mount 1870 1854 1000
Dropdown mount 2726 2777 5000
FocusTrapZone mount 1734 1746 5000
FocusZone mount 1878 1827 5000
IconButton mount 1901 1948 5000
Label mount 355 347 5000
Layer mount 2114 2089 5000
Link mount 465 478 5000
MenuButton mount 1612 1605 5000
MessageBar mount 2168 2135 5000
Nav mount 3479 3562 1000
OverflowSet mount 1479 1512 5000
Panel mount 1508 1508 1000
Persona mount 911 874 1000
Pivot mount 1607 1539 1000
PrimaryButton mount 1380 1372 5000
Rating mount 8209 8191 5000
SearchBox mount 1342 1396 5000
Shimmer mount 2778 2763 5000
Slider mount 1645 1607 5000
SpinButton mount 5319 5357 5000
Spinner mount 449 423 5000
SplitButton mount 3412 3505 5000
Stack mount 568 571 5000
StackWithIntrinsicChildren mount 2197 2153 5000
StackWithTextChildren mount 5592 5563 5000
SwatchColorPicker mount 11117 11098 5000
TagPicker mount 2926 2976 5000
TeachingBubble mount 52239 51777 5000
Text mount 436 477 5000
TextField mount 1568 1555 5000
ThemeProvider mount 1743 1673 5000
ThemeProvider virtual-rerender 651 654 5000
Toggle mount 870 898 5000
button mount 116 124 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.5 0.52 0.96:1 2000 997
🦄 Button.Fluent 0.13 0.22 0.59:1 5000 666
🔧 Checkbox.Fluent 0.68 0.4 1.7:1 1000 678
🎯 Dialog.Fluent 0.17 0.24 0.71:1 5000 872
🔧 Dropdown.Fluent 3.14 0.5 6.28:1 1000 3140
🔧 Icon.Fluent 0.16 0.06 2.67:1 5000 784
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 453
🔧 Slider.Fluent 1.72 0.38 4.53:1 1000 1722
🔧 Text.Fluent 0.09 0.03 3:1 5000 432
🦄 Tooltip.Fluent 0.13 16.12 0.01:1 5000 628

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 180 0 Infinity:1
AlertMinimalPerf.default 350 0 Infinity:1
AttachmentMinimalPerf.default 195 0 Infinity:1
AttachmentSlotsPerf.default 1243 0 Infinity:1
AvatarMinimalPerf.default 552 0 Infinity:1
BoxMinimalPerf.default 422 0 Infinity:1
ButtonMinimalPerf.default 228 0 Infinity:1
ButtonOverridesMissPerf.default 1835 0 Infinity:1
ButtonSlotsPerf.default 656 0 Infinity:1
ButtonUseCssPerf.default 921 0 Infinity:1
CardMinimalPerf.default 657 0 Infinity:1
CarouselMinimalPerf.default 535 0 Infinity:1
ChatDuplicateMessagesPerf.default 500 0 Infinity:1
ChatMinimalPerf.default 731 0 Infinity:1
ChatWithPopoverPerf.default 556 0 Infinity:1
CheckboxMinimalPerf.default 3065 0 Infinity:1
DialogMinimalPerf.default 879 0 Infinity:1
DividerMinimalPerf.default 440 0 Infinity:1
DropdownManyItemsPerf.default 839 0 Infinity:1
DropdownMinimalPerf.default 3161 0 Infinity:1
EmbedMinimalPerf.default 2174 0 Infinity:1
FlexMinimalPerf.default 353 0 Infinity:1
FormMinimalPerf.default 508 0 Infinity:1
HeaderMinimalPerf.default 458 0 Infinity:1
HeaderSlotsPerf.default 915 0 Infinity:1
ImageMinimalPerf.default 465 0 Infinity:1
InputMinimalPerf.default 1428 0 Infinity:1
ItemLayoutMinimalPerf.default 1495 0 Infinity:1
LabelMinimalPerf.default 503 0 Infinity:1
LayoutMinimalPerf.default 533 0 Infinity:1
ListMinimalPerf.default 576 0 Infinity:1
ListNestedPerf.default 650 0 Infinity:1
ListWith60ListItems.default 1013 0 Infinity:1
MenuButtonMinimalPerf.default 1734 0 Infinity:1
PopupMinimalPerf.default 757 0 Infinity:1
PortalMinimalPerf.default 183 0 Infinity:1
ProviderMergeThemesPerf.default 2175 0 Infinity:1
ProviderMinimalPerf.default 1101 0 Infinity:1
RadioGroupMinimalPerf.default 488 0 Infinity:1
ReactionMinimalPerf.default 472 0 Infinity:1
RefMinimalPerf.default 260 0 Infinity:1
SegmentMinimalPerf.default 420 0 Infinity:1
SkeletonMinimalPerf.default 487 0 Infinity:1
SliderMinimalPerf.default 1698 0 Infinity:1
SplitButtonMinimalPerf.default 4123 0 Infinity:1
IconMinimalPerf.default 778 0 Infinity:1
TableManyItemsPerf.default 2490 0 Infinity:1
TableMinimalPerf.default 493 0 Infinity:1
TextMinimalPerf.default 411 0 Infinity:1
TextAreaMinimalPerf.default 568 0 Infinity:1
CustomToolbarPrototype.default 4195 0 Infinity:1
ToolbarMinimalPerf.default 1094 0 Infinity:1
TooltipMinimalPerf.default 901 0 Infinity:1
TreeMinimalPerf.default 987 0 Infinity:1
Button.Fluent 666 0 Infinity:1
Dialog.Fluent 872 0 Infinity:1
Dropdown.Fluent 3140 0 Infinity:1
Icon.Fluent 784 0 Infinity:1
Image.Fluent 453 0 Infinity:1
Slider.Fluent 1722 0 Infinity:1
Text.Fluent 432 0 Infinity:1
Tooltip.Fluent 628 0 Infinity:1
ButtonUseCssNestingPerf.default 1202 1 1202:1
Avatar.Fluent 997 1 997:1
MenuMinimalPerf.default 967 1 967:1
StatusMinimalPerf.default 865 1 865:1
LoaderMinimalPerf.default 833 1 833:1
ListCommonPerf.default 754 1 754:1
VideoMinimalPerf.default 716 1 716:1
Checkbox.Fluent 678 1 678:1
AnimationMinimalPerf.default 458 1 458:1
GridMinimalPerf.default 404 1 404:1
TreeWith60ListItems.default 226 1 226:1

@chrisdholt chrisdholt assigned chrisdholt and unassigned tomi-msft Oct 8, 2020
@size-auditor
Copy link

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

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: 2f6ebac

Possible causes

  • The baseline build 2f6ebac is broken
  • The Size Auditor run for the baseline build 2f6ebac was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 8, 2020

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

Sandbox Source
Fluent UI Button Configuration
microsoft/fluentui: codesandbox-react-template Configuration
microsoft/fluentui: codesandbox-react-next-template Configuration
microsoft/fluentui: codesandbox-react-northstar-template Configuration

@chrisdholt chrisdholt merged commit 6e09556 into microsoft:7.0 Oct 9, 2020
@msft-github-bot
Copy link
Contributor

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

Handy links:

@khmakoto khmakoto mentioned this pull request Oct 9, 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

@chrisdholt chrisdholt

Labels
web-components
Projects
None yet
Milestone
No milestone
Development

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