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

Vertical stacked bar chart combined callout and other tweaks #14912

Merged
merged 5 commits into from
Sep 10, 2020
Merged

Vertical stacked bar chart combined callout and other tweaks #14912

merged 5 commits into from
Sep 10, 2020

Conversation

mbest
Copy link
Member

@mbest mbest commented Sep 4, 2020

Pull request checklist

  • Include a change request file using $ yarn change

Description of changes

VerticalStackedBarChart

  • props for custom rendering the callout and choosing between combined and per-point callouts
  • props for customizing the style or position of the callout
  • skip outputting bars for zero-value points
  • remove ref array where indexes were not deterministic

Screenshots

Customize position and display (formatting value)

image

Custom callout for whole stack

image

Design

image

VerticalStackedBarChart

- props for custom rendering the callout and choosing between combined and per-point callouts
- props for customizing the style or position of the callout
- skip outputting bars for zero-value points
- remove ref array where indexes were not deterministic
@mbest
Copy link
Member Author

mbest commented Sep 4, 2020

The screenshot above showing a custom combined callout uses this code:

          onRenderCalloutPerStack={props => (
            <pre>{JSON.stringify(props, null, 2)}</pre>
          )}

Of course, in real use, it would output a pretty version of the data.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 4, 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 da41823:

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

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Sep 4, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 1024 1004 5000
ButtonNext mount 769 739 5000
Checkbox mount 1794 1776 5000
CheckboxBase mount 1521 1556 5000
CheckboxNext mount 1720 1749 5000
ChoiceGroup mount 5632 5714 5000
ChoiceGroupNext mount 5638 5597 5000
ComboBox mount 1002 1046 1000
CommandBar mount 8271 8725 1000
ContextualMenu mount 14537 14612 1000
DefaultButton mount 1279 1291 5000
DetailsRow mount 4040 3984 5000
DetailsRowFast mount 3997 4009 5000
DetailsRowNoStyles mount 3785 3775 5000
Dialog mount 1595 1659 1000
DocumentCardTitle mount 1974 1980 1000
Dropdown mount 2890 2918 5000
FocusZone mount 1947 2014 5000
IconButton mount 1998 1987 5000
Label mount 375 379 5000
Link mount 503 495 5000
LinkNext mount 527 539 5000
MenuButton mount 1619 1670 5000
MessageBar mount 2257 2264 5000
MessageBarNext mount 2240 2273 5000
Nav mount 3635 3639 1000
OverflowSet mount 1555 1561 5000
OverflowSetNext mount 1149 1113 5000
Panel mount 1659 1621 1000
Persona mount 968 954 1000
Pivot mount 1612 1699 1000
PivotNext mount 1556 1585 1000
PrimaryButton mount 1446 1422 5000
Rating mount 8691 8723 5000
RatingNext mount 8719 8766 5000
SearchBox mount 1464 1501 5000
SearchBoxNext mount 1554 1561 5000
Shimmer mount 2981 2923 5000
ShimmerNext mount 2910 2887 5000
Slider mount 1705 1730 5000
SliderNext mount 2125 2149 5000
SpinButton mount 5602 5544 5000
SpinButtonNext mount 5714 5604 5000
Spinner mount 468 449 5000
SplitButton mount 3595 3799 5000
Stack mount 580 634 5000
StackWithIntrinsicChildren mount 2194 2138 5000
StackWithTextChildren mount 5775 5655 5000
SwatchColorPicker mount 11577 11593 5000
SwatchColorPickerNext mount 11504 11423 5000
TagPicker mount 3013 3133 5000
TeachingBubble mount 54757 54559 5000
TeachingBubbleNext mount 54982 54166 5000
Text mount 479 492 5000
TextField mount 1563 1562 5000
ThemeProvider mount 4879 4844 5000
ThemeProvider virtual-rerender 500 495 5000
Toggle mount 953 961 5000
ToggleNext mount 920 906 5000
button mount 132 136 5000

Perf Analysis (Fluent)

⚠️ 5 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonOverridesMissPerf.default 1928 50 38.56:1 analysis
ButtonUseCssNestingPerf.default 1237 51 24.25:1 analysis
ButtonUseCssPerf.default 944 50 18.88:1 analysis
ChatWithPopoverPerf.default 546 509 1.07:1 analysis
ListNestedPerf.default 690 1009 0.68:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.51 0.54 0.94:1 2000 1029
🦄 Button.Fluent 0.14 0.23 0.61:1 5000 689
🔧 Checkbox.Fluent 0.71 0.4 1.78:1 1000 713
🎯 Dialog.Fluent 0.18 0.25 0.72:1 5000 897
🔧 Dropdown.Fluent 3.2 0.55 5.82:1 1000 3195
🔧 Icon.Fluent 0.18 0.07 2.57:1 5000 877
🎯 Image.Fluent 0.1 0.13 0.77:1 5000 515
🔧 Slider.Fluent 1.74 0.4 4.35:1 1000 1744
🔧 Text.Fluent 0.09 0.04 2.25:1 5000 448
🦄 Tooltip.Fluent 0.13 17.76 0.01:1 5000 641

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 179 123 1.46:1
RadioGroupMinimalPerf.default 567 449 1.26:1
Image.Fluent 515 413 1.25:1
GridMinimalPerf.default 454 373 1.22:1
RefMinimalPerf.default 259 216 1.2:1
SegmentMinimalPerf.default 458 382 1.2:1
BoxMinimalPerf.default 463 388 1.19:1
ReactionMinimalPerf.default 494 415 1.19:1
DividerMinimalPerf.default 473 400 1.18:1
Text.Fluent 448 379 1.18:1
FormMinimalPerf.default 527 449 1.17:1
LayoutMinimalPerf.default 514 440 1.17:1
Tooltip.Fluent 641 550 1.17:1
StatusMinimalPerf.default 880 757 1.16:1
CardMinimalPerf.default 703 613 1.15:1
HeaderMinimalPerf.default 463 402 1.15:1
SkeletonMinimalPerf.default 520 458 1.14:1
AccordionMinimalPerf.default 198 175 1.13:1
ImageMinimalPerf.default 468 413 1.13:1
TableMinimalPerf.default 501 443 1.13:1
TreeMinimalPerf.default 1077 957 1.13:1
Button.Fluent 689 610 1.13:1
AnimationMinimalPerf.default 478 426 1.12:1
AttachmentMinimalPerf.default 200 178 1.12:1
FlexMinimalPerf.default 353 315 1.12:1
HeaderSlotsPerf.default 992 882 1.12:1
TextMinimalPerf.default 443 399 1.11:1
Icon.Fluent 877 792 1.11:1
ListMinimalPerf.default 573 521 1.1:1
IconMinimalPerf.default 797 726 1.1:1
TextAreaMinimalPerf.default 602 546 1.1:1
VideoMinimalPerf.default 765 695 1.1:1
DialogMinimalPerf.default 912 835 1.09:1
LabelMinimalPerf.default 521 476 1.09:1
PopupMinimalPerf.default 780 718 1.09:1
AvatarMinimalPerf.default 569 529 1.08:1
ChatMinimalPerf.default 756 701 1.08:1
ItemLayoutMinimalPerf.default 1553 1438 1.08:1
MenuMinimalPerf.default 1016 940 1.08:1
ToolbarMinimalPerf.default 1171 1080 1.08:1
TooltipMinimalPerf.default 943 872 1.08:1
Dialog.Fluent 897 834 1.08:1
CarouselMinimalPerf.default 542 507 1.07:1
ChatDuplicateMessagesPerf.default 496 464 1.07:1
LoaderMinimalPerf.default 843 785 1.07:1
ProviderMinimalPerf.default 1065 1007 1.06:1
Avatar.Fluent 1029 974 1.06:1
AttachmentSlotsPerf.default 1312 1267 1.04:1
ButtonSlotsPerf.default 707 682 1.04:1
EmbedMinimalPerf.default 2218 2132 1.04:1
MenuButtonMinimalPerf.default 1797 1736 1.04:1
TreeWith60ListItems.default 251 242 1.04:1
CheckboxMinimalPerf.default 3224 3117 1.03:1
InputMinimalPerf.default 1464 1427 1.03:1
Checkbox.Fluent 713 690 1.03:1
ButtonMinimalPerf.default 217 212 1.02:1
ProviderMergeThemesPerf.default 2111 2066 1.02:1
SliderMinimalPerf.default 1750 1716 1.02:1
SplitButtonMinimalPerf.default 4256 4175 1.02:1
TableManyItemsPerf.default 2589 2547 1.02:1
DropdownMinimalPerf.default 3246 3200 1.01:1
CustomToolbarPrototype.default 4130 4073 1.01:1
AlertMinimalPerf.default 361 360 1:1
Slider.Fluent 1744 1745 1:1
Dropdown.Fluent 3195 3230 0.99:1
DropdownManyItemsPerf.default 845 864 0.98:1
ListWith60ListItems.default 1070 1238 0.86:1
ListCommonPerf.default 795 1068 0.74:1

@size-auditor
Copy link

size-auditor bot commented Sep 4, 2020

Asset size changes

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

Baseline commit: d0bdb5b1f6dae8c39be8ef012f70c3a5688a10f0 (build)

@Raghurk
Copy link
Contributor

Raghurk commented Sep 9, 2020

Can you please resolve the conflicts

@Raghurk Raghurk merged commit df56f46 into microsoft:master Sep 10, 2020
@msft-github-bot
Copy link
Contributor

🎉@uifabric/charting@v4.0.0 has been released which incorporates this pull request.:tada:

Handy links:

hutchcodes pushed a commit to hutchcodes/fluentui that referenced this pull request Sep 10, 2020
…ft#14912)

* Vertical stacked bar chart combined callout and other tweaks

VerticalStackedBarChart

- props for custom rendering the callout and choosing between combined and per-point callouts
- props for customizing the style or position of the callout
- skip outputting bars for zero-value points
- remove ref array where indexes were not deterministic

* Change files

* changing to "major" due to removed exports (internal state interface shouldn't be exported)

* maintain earlier update
@mbest mbest deleted the VerticalStackedBarChart-callout-improvements branch September 10, 2020 17:49
*/
chartTitle?: string;
xAxisCalloutData?: string;
Copy link
Member

Choose a reason for hiding this comment

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

Was this intentional? It seems like xAxisCalloutData should have been a new prop, not a replacement for chartTitle.

Copy link
Member Author

Choose a reason for hiding this comment

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

chartTitle was not being used. It's not technically a replacement.

@@ -0,0 +1,8 @@
{
"type": "major",
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure this should NOT have been a major change. Major version bumps should be reserved for backwards-incompatible API changes, which I don't think I'm seeing in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Raghurk wanted it to be major due to the changes to IVerticalStackedBarChartState, which had previously been exported.

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, talked to @Raghurk and he said this was intentional

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.

4 participants