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

fix(ChoiceGroup): add better guidance for image + text strings, and update the example #19758

Merged
merged 3 commits into from
Sep 10, 2021

Conversation

smhigley
Copy link
Contributor

@smhigley smhigley commented Sep 10, 2021

Pull request checklist

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

Description of changes

The ChoiceGroup image variant doesn't accommodate longer text strings, so I added guidance to keep text short, since translation or user styles can lengthen it. This was the solution proposed when consulting with design, since the current design doesn't have a way to handle longer text without significant alteration.

I also update the example to use the correct icon for pie chart, because why not :D

selectedImageSrc: TestImages.choiceGroupBarSelected,
imageSize: { width: 32, height: 32 },
text: 'Clustered bar chart', // This text is long to show text wrapping.
text: 'Bar', // This text is long to show text wrapping.
Copy link
Member

Choose a reason for hiding this comment

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

Should the comment after be removed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, yup, fixed! 😄

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 10, 2021

📊 Bundle size report

🤖 This report was generated against 7edc8fe8dccfa4e63b24d38a25ba5f8af06701dd

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 10, 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 8f43e40:

Sandbox Source
Fluent UI React Starter Configuration

@size-auditor
Copy link

size-auditor bot commented Sep 10, 2021

Asset size changes

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

Baseline commit: 7edc8fe8dccfa4e63b24d38a25ba5f8af06701dd (build)

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 977 952 5000
BaseButton mount 941 959 5000
Breadcrumb mount 2638 2588 1000
ButtonNext mount 472 455 5000
Checkbox mount 1617 1703 5000
CheckboxBase mount 1382 1432 5000
ChoiceGroup mount 5108 5067 5000
ComboBox mount 1006 996 1000
CommandBar mount 10576 10381 1000
ContextualMenu mount 6566 6622 1000
DefaultButton mount 1201 1177 5000
DetailsRow mount 4055 3972 5000
DetailsRowFast mount 3983 4060 5000
DetailsRowNoStyles mount 3817 3884 5000
Dialog mount 2559 2596 1000
DocumentCardTitle mount 146 154 1000
Dropdown mount 3400 3417 5000
FluentProviderNext mount 7285 7204 5000
FluentProviderWithTheme mount 353 345 10
FluentProviderWithTheme virtual-rerender 89 94 10
FluentProviderWithTheme virtual-rerender-with-unmount 485 461 10
FocusTrapZone mount 1860 1875 5000
FocusZone mount 1847 1955 5000
IconButton mount 1942 1964 5000
Label mount 355 335 5000
Layer mount 3270 3163 5000
Link mount 490 500 5000
MakeStyles mount 1863 1896 50000
MenuButton mount 1647 1568 5000
MessageBar mount 2097 2010 5000
Nav mount 3518 3630 1000
OverflowSet mount 1168 1168 5000
Panel mount 2461 2502 1000
Persona mount 877 922 1000
Pivot mount 1495 1494 1000
PrimaryButton mount 1363 1369 5000
Rating mount 8505 8489 5000
SearchBox mount 1511 1505 5000
Shimmer mount 2769 2777 5000
Slider mount 2088 2186 5000
SpinButton mount 5462 5422 5000
Spinner mount 453 475 5000
SplitButton mount 3506 3536 5000
Stack mount 568 562 5000
StackWithIntrinsicChildren mount 1764 1776 5000
StackWithTextChildren mount 5172 5104 5000
SwatchColorPicker mount 11349 11304 5000
Tabs mount 1631 1635 1000
TagPicker mount 2889 2890 5000
TeachingBubble mount 14817 14370 5000
Text mount 461 493 5000
TextField mount 1506 1490 5000
ThemeProvider mount 1229 1232 5000
ThemeProvider virtual-rerender 635 629 5000
ThemeProvider virtual-rerender-with-unmount 2103 2056 5000
Toggle mount 903 908 5000
buttonNative mount 127 119 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 185 158 1.17:1
PortalMinimalPerf.default 202 178 1.13:1
TextMinimalPerf.default 403 361 1.12:1
TextAreaMinimalPerf.default 602 536 1.12:1
CarouselMinimalPerf.default 525 483 1.09:1
IconMinimalPerf.default 695 637 1.09:1
SegmentMinimalPerf.default 391 365 1.07:1
AttachmentSlotsPerf.default 1205 1142 1.06:1
BoxMinimalPerf.default 405 383 1.06:1
ButtonMinimalPerf.default 192 181 1.06:1
DropdownManyItemsPerf.default 765 723 1.06:1
FormMinimalPerf.default 498 471 1.06:1
RadioGroupMinimalPerf.default 506 476 1.06:1
DividerMinimalPerf.default 412 391 1.05:1
ListCommonPerf.default 699 668 1.05:1
StatusMinimalPerf.default 757 722 1.05:1
TreeMinimalPerf.default 874 835 1.05:1
CardMinimalPerf.default 624 601 1.04:1
DialogMinimalPerf.default 823 790 1.04:1
ItemLayoutMinimalPerf.default 1365 1316 1.04:1
ToolbarMinimalPerf.default 1041 1001 1.04:1
AccordionMinimalPerf.default 159 154 1.03:1
ChatMinimalPerf.default 716 693 1.03:1
ChatWithPopoverPerf.default 394 383 1.03:1
CheckboxMinimalPerf.default 2883 2800 1.03:1
DatepickerMinimalPerf.default 6004 5855 1.03:1
ImageMinimalPerf.default 440 426 1.03:1
LabelMinimalPerf.default 447 435 1.03:1
ListWith60ListItems.default 701 682 1.03:1
MenuButtonMinimalPerf.default 1825 1791 1.02:1
ProviderMergeThemesPerf.default 1790 1753 1.02:1
ButtonOverridesMissPerf.default 1871 1847 1.01:1
GridMinimalPerf.default 380 378 1.01:1
HeaderSlotsPerf.default 844 834 1.01:1
ListMinimalPerf.default 557 553 1.01:1
ListNestedPerf.default 611 606 1.01:1
ReactionMinimalPerf.default 427 424 1.01:1
ButtonSlotsPerf.default 605 603 1:1
RefMinimalPerf.default 232 232 1:1
SplitButtonMinimalPerf.default 4506 4528 1:1
TableManyItemsPerf.default 2108 2109 1:1
TableMinimalPerf.default 435 436 1:1
CustomToolbarPrototype.default 4182 4167 1:1
AvatarMinimalPerf.default 221 224 0.99:1
DropdownMinimalPerf.default 3180 3198 0.99:1
EmbedMinimalPerf.default 4409 4468 0.99:1
MenuMinimalPerf.default 928 942 0.99:1
ProviderMinimalPerf.default 1079 1091 0.99:1
SliderMinimalPerf.default 1632 1651 0.99:1
TooltipMinimalPerf.default 1082 1098 0.99:1
TreeWith60ListItems.default 184 185 0.99:1
AnimationMinimalPerf.default 436 447 0.98:1
InputMinimalPerf.default 1317 1350 0.98:1
PopupMinimalPerf.default 640 650 0.98:1
SkeletonMinimalPerf.default 371 378 0.98:1
HeaderMinimalPerf.default 401 413 0.97:1
LoaderMinimalPerf.default 745 767 0.97:1
RosterPerf.default 1289 1335 0.97:1
FlexMinimalPerf.default 304 318 0.96:1
VideoMinimalPerf.default 668 695 0.96:1
AlertMinimalPerf.default 286 302 0.95:1
LayoutMinimalPerf.default 385 404 0.95:1
ChatDuplicateMessagesPerf.default 299 333 0.9:1

@smhigley smhigley merged commit 39625b3 into microsoft:master Sep 10, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

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.

5 participants