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: removed SVGs and update fill style in accordion #17527

Merged

Conversation

khamudom
Copy link
Contributor

@khamudom khamudom commented Mar 23, 2021

Pull request checklist

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

Description of changes

In the example file, I removed the SVGs to use the default expand and collapsed icons and updated the CSS selector to target the icon's fill property to set the color.

image

Focus areas to test

(optional)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 23, 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 cf2c0b1:

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

@size-auditor
Copy link

size-auditor bot commented Mar 23, 2021

Asset size changes

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

Baseline commit: 6d000dd560441cc0d29e61e10a7b93d25e7a0997 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 23, 2021

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1088 1131 5000
BaseButton mount 878 876 5000
Breadcrumb mount 43143 43249 5000
ButtonNext mount 1220 1208 5000
Checkbox mount 1512 1497 5000
CheckboxBase mount 1267 1283 5000
ChoiceGroup mount 4711 4672 5000
ComboBox mount 932 979 1000
CommandBar mount 10042 9949 1000
ContextualMenu mount 6146 6073 1000
DefaultButton mount 1100 1130 5000
DetailsRow mount 3572 3505 5000
DetailsRowFast mount 3612 3533 5000
DetailsRowNoStyles mount 3399 3352 5000
Dialog mount 1433 1450 1000
DocumentCardTitle mount 1822 1805 1000
Dropdown mount 3215 3475 5000
FocusTrapZone mount 1758 1800 5000
FocusZone mount 1744 1788 5000
IconButton mount 1716 1696 5000
Label mount 329 329 5000
Layer mount 1752 1764 5000
Link mount 456 469 5000
MakeStyles mount 1962 1980 50000
MenuButton mount 1410 1443 5000
MessageBar mount 1989 1966 5000
Nav mount 3201 3197 1000
OverflowSet mount 1045 1026 5000
Panel mount 1414 1388 1000
Persona mount 802 819 1000
Pivot mount 1375 1369 1000
PrimaryButton mount 1255 1280 5000
Rating mount 7492 7428 5000
SearchBox mount 1303 1290 5000
Shimmer mount 2444 2495 5000
Slider mount 1946 1971 5000
SpinButton mount 4863 4891 5000
Spinner mount 421 425 5000
SplitButton mount 3094 3087 5000
Stack mount 491 496 5000
StackWithIntrinsicChildren mount 1500 1513 5000
StackWithTextChildren mount 4452 4487 5000
SwatchColorPicker mount 10026 10125 5000
Tabs mount 1343 1393 1000
TagPicker mount 2714 2763 5000
TeachingBubble mount 11371 11380 5000
Text mount 424 423 5000
TextField mount 1351 1352 5000
ThemeProvider mount 1163 1161 5000
ThemeProvider virtual-rerender 600 584 5000
ThemeProviderNext mount 15785 15826 5000
Toggle mount 801 799 5000
buttonNative mount 114 108 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.18 0.49 0.37:1 2000 354
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 564
🔧 Checkbox.Fluent 0.61 0.35 1.74:1 1000 614
🎯 Dialog.Fluent 0.16 0.21 0.76:1 5000 816
🔧 Dropdown.Fluent 3.11 0.38 8.18:1 1000 3113
🔧 Icon.Fluent 0.14 0.06 2.33:1 5000 681
🦄 Image.Fluent 0.08 0.13 0.62:1 5000 397
🔧 Slider.Fluent 1.58 0.46 3.43:1 1000 1576
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 364
🦄 Tooltip.Fluent 0.14 0.89 0.16:1 5000 687

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ImageMinimalPerf.default 420 388 1.08:1
PortalMinimalPerf.default 185 172 1.08:1
AccordionMinimalPerf.default 171 160 1.07:1
ListCommonPerf.default 699 656 1.07:1
SkeletonMinimalPerf.default 401 374 1.07:1
Avatar.Fluent 354 331 1.07:1
ButtonMinimalPerf.default 194 183 1.06:1
CarouselMinimalPerf.default 500 477 1.05:1
FlexMinimalPerf.default 326 313 1.04:1
HeaderMinimalPerf.default 387 371 1.04:1
ListWith60ListItems.default 638 614 1.04:1
ReactionMinimalPerf.default 422 407 1.04:1
TreeWith60ListItems.default 184 177 1.04:1
AttachmentSlotsPerf.default 1224 1193 1.03:1
CardMinimalPerf.default 566 550 1.03:1
DividerMinimalPerf.default 388 377 1.03:1
FormMinimalPerf.default 426 415 1.03:1
TextAreaMinimalPerf.default 509 495 1.03:1
Dialog.Fluent 816 791 1.03:1
AvatarMinimalPerf.default 210 205 1.02:1
CheckboxMinimalPerf.default 2826 2781 1.02:1
DialogMinimalPerf.default 811 798 1.02:1
DropdownManyItemsPerf.default 749 731 1.02:1
ListMinimalPerf.default 530 520 1.02:1
RadioGroupMinimalPerf.default 468 460 1.02:1
SegmentMinimalPerf.default 379 370 1.02:1
TableMinimalPerf.default 433 426 1.02:1
Dropdown.Fluent 3113 3057 1.02:1
Icon.Fluent 681 667 1.02:1
BoxMinimalPerf.default 379 376 1.01:1
ButtonUseCssNestingPerf.default 1082 1076 1.01:1
ChatDuplicateMessagesPerf.default 295 292 1.01:1
ChatMinimalPerf.default 629 620 1.01:1
ChatWithPopoverPerf.default 384 379 1.01:1
DatepickerMinimalPerf.default 45423 45137 1.01:1
DropdownMinimalPerf.default 3137 3108 1.01:1
LayoutMinimalPerf.default 433 427 1.01:1
MenuMinimalPerf.default 903 897 1.01:1
MenuButtonMinimalPerf.default 1572 1556 1.01:1
PopupMinimalPerf.default 718 712 1.01:1
SplitButtonMinimalPerf.default 3784 3748 1.01:1
TableManyItemsPerf.default 2051 2029 1.01:1
TextMinimalPerf.default 370 368 1.01:1
ToolbarMinimalPerf.default 975 966 1.01:1
Text.Fluent 364 361 1.01:1
AlertMinimalPerf.default 306 306 1:1
AnimationMinimalPerf.default 415 414 1:1
ButtonUseCssPerf.default 826 826 1:1
HeaderSlotsPerf.default 802 799 1:1
InputMinimalPerf.default 1296 1291 1:1
StatusMinimalPerf.default 743 742 1:1
IconMinimalPerf.default 664 663 1:1
CustomToolbarPrototype.default 3809 3812 1:1
ButtonOverridesMissPerf.default 1693 1710 0.99:1
ButtonSlotsPerf.default 577 581 0.99:1
EmbedMinimalPerf.default 4212 4260 0.99:1
ItemLayoutMinimalPerf.default 1208 1220 0.99:1
LoaderMinimalPerf.default 733 741 0.99:1
ProviderMergeThemesPerf.default 1638 1657 0.99:1
ProviderMinimalPerf.default 983 991 0.99:1
SliderMinimalPerf.default 1554 1569 0.99:1
TooltipMinimalPerf.default 951 961 0.99:1
VideoMinimalPerf.default 645 650 0.99:1
Tooltip.Fluent 687 697 0.99:1
AttachmentMinimalPerf.default 170 174 0.98:1
GridMinimalPerf.default 359 368 0.98:1
RosterPerf.default 1147 1165 0.98:1
RefMinimalPerf.default 253 257 0.98:1
TreeMinimalPerf.default 788 805 0.98:1
Slider.Fluent 1576 1610 0.98:1
ListNestedPerf.default 578 593 0.97:1
Button.Fluent 564 588 0.96:1
Checkbox.Fluent 614 637 0.96:1
Image.Fluent 397 414 0.96:1
LabelMinimalPerf.default 428 449 0.95:1

@@ -85,6 +85,7 @@ export const AccordionItemStyles = css`
grid-column: 4;
z-index: 2;
pointer-events: none;
fill: var(--accent-fill-rest);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should use accentFillRestBehavior.var

Copy link
Member

Choose a reason for hiding this comment

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

Good catch @nicholasrice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @nicholasrice . updated

@chrisdholt chrisdholt merged commit b8e8697 into microsoft:master Mar 24, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

joshualamusga1 pushed a commit to joshualamusga1/fluentui that referenced this pull request Mar 25, 2021
* removed SVGs to use the default

* Change files

* set the fill color of the icon in style

* using accentFillRestBehavior.var on fill
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
* removed SVGs to use the default

* Change files

* set the fill color of the icon in style

* using accentFillRestBehavior.var on fill
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.

7 participants