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: remove margins on host elements #20710

Merged
merged 2 commits into from
Nov 22, 2021
Merged

Conversation

eljefe223
Copy link
Contributor

Pull request checklist

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

Description of changes

Removes margins from host elements.

Focus areas to test

(optional)

Copy link
Member

@chrisdholt chrisdholt left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @eljefe223!

@fabricteam
Copy link
Collaborator

📊 Bundle size report

🤖 This report was generated against 5ade517ac0f1ba61cee8c387627c2b33c92eb112

@codesandbox-ci
Copy link

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 746343e:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Nov 22, 2021

Asset size changes

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

Baseline commit: 5ade517ac0f1ba61cee8c387627c2b33c92eb112 (build)

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
Dialog mount 2743 1476 1000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1047 1032 5000
BaseButton mount 1017 1045 5000
Breadcrumb mount 2790 2778 1000
ButtonNext mount 604 577 5000
Checkbox mount 1726 1721 5000
CheckboxBase mount 1480 1504 5000
ChoiceGroup mount 5332 5216 5000
ComboBox mount 1060 1058 1000
CommandBar mount 10802 11033 1000
ContextualMenu mount 8997 9162 1000
DefaultButton mount 1309 1301 5000
DetailsRow mount 4209 4151 5000
DetailsRowFast mount 4160 4186 5000
DetailsRowNoStyles mount 4014 3933 5000
Dialog mount 2743 1476 1000 Possible regression
DocumentCardTitle mount 175 188 1000
Dropdown mount 3597 3554 5000
FluentProviderNext mount 4132 4091 5000
FluentProviderWithTheme mount 244 251 10
FluentProviderWithTheme virtual-rerender 111 110 10
FluentProviderWithTheme virtual-rerender-with-unmount 285 279 10
FocusTrapZone mount 1984 1999 5000
FocusZone mount 1929 1935 5000
IconButton mount 1955 1980 5000
Label mount 376 357 5000
Layer mount 3293 3268 5000
Link mount 522 525 5000
MakeStyles mount 1939 1913 50000
MenuButton mount 1658 1688 5000
MessageBar mount 2172 2182 5000
Nav mount 3699 3645 1000
OverflowSet mount 1229 1241 5000
Panel mount 2647 2604 1000
Persona mount 916 925 1000
Pivot mount 1569 1585 1000
PrimaryButton mount 1407 1475 5000
Rating mount 8719 8687 5000
SearchBox mount 1507 1512 5000
Shimmer mount 2869 2889 5000
Slider mount 2245 2226 5000
SpinButton mount 5416 5548 5000
Spinner mount 474 483 5000
SplitButton mount 3543 3568 5000
Stack mount 568 574 5000
StackWithIntrinsicChildren mount 1944 1954 5000
StackWithTextChildren mount 5348 5306 5000
SwatchColorPicker mount 11485 11438 5000
TagPicker mount 2830 2899 5000
TeachingBubble mount 14901 14126 5000
Text mount 485 496 5000
TextField mount 1541 1635 5000
ThemeProvider mount 1308 1326 5000
ThemeProvider virtual-rerender 651 667 5000
ThemeProvider virtual-rerender-with-unmount 2003 2088 5000
Toggle mount 925 908 5000
buttonNative mount 143 144 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 192 166 1.16:1
AvatarMinimalPerf.default 236 206 1.15:1
PortalMinimalPerf.default 203 180 1.13:1
TextMinimalPerf.default 415 378 1.1:1
TreeWith60ListItems.default 212 195 1.09:1
ChatMinimalPerf.default 773 725 1.07:1
ChatWithPopoverPerf.default 428 401 1.07:1
DividerMinimalPerf.default 431 401 1.07:1
DropdownManyItemsPerf.default 809 759 1.07:1
FlexMinimalPerf.default 362 337 1.07:1
SegmentMinimalPerf.default 408 381 1.07:1
CarouselMinimalPerf.default 544 515 1.06:1
LabelMinimalPerf.default 452 427 1.06:1
HeaderSlotsPerf.default 895 851 1.05:1
ListWith60ListItems.default 747 713 1.05:1
MenuMinimalPerf.default 999 947 1.05:1
FormMinimalPerf.default 487 467 1.04:1
ImageMinimalPerf.default 425 409 1.04:1
ListMinimalPerf.default 594 573 1.04:1
MenuButtonMinimalPerf.default 1875 1808 1.04:1
SkeletonMinimalPerf.default 411 395 1.04:1
TreeMinimalPerf.default 903 868 1.04:1
AnimationMinimalPerf.default 460 445 1.03:1
ButtonMinimalPerf.default 201 198 1.02:1
ButtonOverridesMissPerf.default 1910 1871 1.02:1
DatepickerMinimalPerf.default 6099 5974 1.02:1
InputMinimalPerf.default 1409 1376 1.02:1
RefMinimalPerf.default 256 252 1.02:1
SliderMinimalPerf.default 1806 1775 1.02:1
TextAreaMinimalPerf.default 570 558 1.02:1
ToolbarMinimalPerf.default 1041 1016 1.02:1
AccordionMinimalPerf.default 173 171 1.01:1
AttachmentSlotsPerf.default 1189 1173 1.01:1
BoxMinimalPerf.default 406 400 1.01:1
ChatDuplicateMessagesPerf.default 342 340 1.01:1
ListCommonPerf.default 737 732 1.01:1
LoaderMinimalPerf.default 755 745 1.01:1
RadioGroupMinimalPerf.default 532 528 1.01:1
IconMinimalPerf.default 694 685 1.01:1
AlertMinimalPerf.default 324 324 1:1
CheckboxMinimalPerf.default 2907 2909 1:1
EmbedMinimalPerf.default 4518 4501 1:1
HeaderMinimalPerf.default 424 426 1:1
LayoutMinimalPerf.default 422 423 1:1
PopupMinimalPerf.default 641 639 1:1
ReactionMinimalPerf.default 449 448 1:1
SplitButtonMinimalPerf.default 4714 4698 1:1
TableManyItemsPerf.default 2145 2139 1:1
CustomToolbarPrototype.default 4440 4448 1:1
TooltipMinimalPerf.default 1137 1140 1:1
CardMinimalPerf.default 654 662 0.99:1
DialogMinimalPerf.default 835 843 0.99:1
DropdownMinimalPerf.default 3190 3228 0.99:1
ProviderMinimalPerf.default 1247 1256 0.99:1
StatusMinimalPerf.default 752 758 0.99:1
ButtonSlotsPerf.default 625 637 0.98:1
GridMinimalPerf.default 391 400 0.98:1
ProviderMergeThemesPerf.default 1787 1828 0.98:1
TableMinimalPerf.default 452 459 0.98:1
VideoMinimalPerf.default 691 705 0.98:1
ItemLayoutMinimalPerf.default 1320 1357 0.97:1
RosterPerf.default 1346 1381 0.97:1
ListNestedPerf.default 621 651 0.95:1

Copy link
Contributor

@bheston bheston left a comment

Choose a reason for hiding this comment

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

Those values had less rationale that I thought some of them should have. Others I'm not sure where they even came from. Good fix.

@eljefe223 eljefe223 merged commit 2dbe9ea into master Nov 22, 2021
@ecraig12345 ecraig12345 deleted the users/jes/margins-on-host branch January 4, 2022 21:46
mlp73 pushed a commit to mlp73/fluentui that referenced this pull request Jan 17, 2022
* fix: remove margins from host elements

* Change files
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.

6 participants