Skip to content

Fix Azure theme Text + Label: font size clean up, disabled state value #14534

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
Merged
Jacqueline-ms merged 9 commits into from
Aug 22, 2020
Merged

Fix Azure theme Text + Label: font size clean up, disabled state value #14534

Jacqueline-ms merged 9 commits into from
Aug 22, 2020

Conversation

@Jacqueline-ms
Copy link
Contributor

@Jacqueline-ms Jacqueline-ms commented Aug 14, 2020

Pull request checklist

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

Description of changes

Missed a few areas for font, address in theme several theme files.
image
image

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 14, 2020 •
edited
Loading

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

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

@size-auditor
Copy link

size-auditor bot commented Aug 14, 2020 •
edited
Loading

Asset size changes

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

Baseline commit: 47f5465f1ee881062cfc0ac93158bbce5a39ef83 (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Aug 14, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 874 850 5000
ButtonNext mount 561 586 5000
Checkbox mount 1500 1517 5000
CheckboxBase mount 1267 1249 5000
CheckboxNext mount 1600 1560 5000
ChoiceGroup mount 4764 4854 5000
ChoiceGroupNext mount 4722 4749 5000
ComboBox mount 896 887 1000
CommandBar mount 7346 7355 1000
ContextualMenu mount 13003 12975 1000
DefaultButton mount 1072 1065 5000
DetailsRow mount 3411 3389 5000
DetailsRowFast mount 3403 3417 5000
DetailsRowNoStyles mount 3297 3260 5000
Dialog mount 1465 1450 1000
DocumentCardTitle mount 1752 1742 1000
Dropdown mount 2508 2514 5000
FocusZone mount 1767 1766 5000
IconButton mount 1650 1659 5000
Label mount 332 336 5000
Link mount 436 431 5000
LinkNext mount 472 481 5000
MenuButton mount 1402 1392 5000
Nav mount 3095 3113 1000
OverflowSet mount 1320 1332 5000
OverflowSetNext mount 986 1001 5000
Panel mount 1419 1393 1000
Persona mount 802 823 1000
Pivot mount 1387 1391 1000
PivotNext mount 1319 1398 1000
PrimaryButton mount 1213 1207 5000
SearchBox mount 1252 1201 5000
SearchBoxNext mount 1285 1256 5000
Slider mount 1426 1426 5000
SliderNext mount 1895 1849 5000
SpinButton mount 4737 4809 5000
SpinButtonNext mount 4863 4871 5000
Spinner mount 427 412 5000
SplitButton mount 2986 2994 5000
Stack mount 508 518 5000
StackWithIntrinsicChildren mount 1816 1789 5000
StackWithTextChildren mount 4763 4739 5000
TagPicker mount 2664 2628 5000
Text mount 418 409 5000
TextField mount 1350 1325 5000
ThemeProvider mount 3724 3759 5000
ThemeProvider virtual-rerender 459 454 5000
Toggle mount 794 800 5000
ToggleNext mount 821 817 5000
button mount 107 98 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonOverridesMissPerf.default 100 43 2.33:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.41 0.45 0.91:1 2000 823
🦄 Button.Fluent 0.1 0.17 0.59:1 5000 511
🔧 Checkbox.Fluent 0.6 0.33 1.82:1 1000 601
🦄 Dialog.Fluent 0.14 0.21 0.67:1 5000 714
🔧 Dropdown.Fluent 2.83 0.44 6.43:1 1000 2831
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 674
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 340
🔧 Slider.Fluent 1.53 0.35 4.37:1 1000 1526
🔧 Text.Fluent 0.06 0.03 2:1 5000 314
🦄 Tooltip.Fluent 0.1 16.05 0.01:1 5000 506

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonUseCssNestingPerf.default 52 39 1.33:1
ButtonUseCssPerf.default 58 44 1.32:1
RefMinimalPerf.default 213 191 1.12:1
ChatWithPopoverPerf.default 489 450 1.09:1
PortalMinimalPerf.default 129 118 1.09:1
TableMinimalPerf.default 383 350 1.09:1
AlertMinimalPerf.default 289 268 1.08:1
AttachmentMinimalPerf.default 151 140 1.08:1
SegmentMinimalPerf.default 329 304 1.08:1
FlexMinimalPerf.default 275 257 1.07:1
GridMinimalPerf.default 328 307 1.07:1
TextMinimalPerf.default 324 302 1.07:1
ButtonMinimalPerf.default 165 156 1.06:1
HeaderSlotsPerf.default 760 715 1.06:1
PopupMinimalPerf.default 669 633 1.06:1
ProviderMergeThemesPerf.default 1915 1806 1.06:1
RadioGroupMinimalPerf.default 391 370 1.06:1
ProviderMinimalPerf.default 881 838 1.05:1
SkeletonMinimalPerf.default 377 360 1.05:1
Icon.Fluent 674 640 1.05:1
AnimationMinimalPerf.default 384 371 1.04:1
CarouselMinimalPerf.default 449 430 1.04:1
DropdownManyItemsPerf.default 734 709 1.04:1
LabelMinimalPerf.default 394 379 1.04:1
LayoutMinimalPerf.default 381 365 1.04:1
StatusMinimalPerf.default 640 614 1.04:1
IconMinimalPerf.default 613 590 1.04:1
TreeMinimalPerf.default 826 792 1.04:1
Button.Fluent 511 489 1.04:1
BoxMinimalPerf.default 330 319 1.03:1
ChatMinimalPerf.default 591 572 1.03:1
MenuMinimalPerf.default 807 787 1.03:1
ReactionMinimalPerf.default 349 340 1.03:1
CustomToolbarPrototype.default 3615 3519 1.03:1
ToolbarMinimalPerf.default 888 859 1.03:1
Avatar.Fluent 823 799 1.03:1
AccordionMinimalPerf.default 140 137 1.02:1
ButtonSlotsPerf.default 584 575 1.02:1
CheckboxMinimalPerf.default 2752 2710 1.02:1
DropdownMinimalPerf.default 2885 2827 1.02:1
ListMinimalPerf.default 458 451 1.02:1
TableManyItemsPerf.default 2054 2011 1.02:1
VideoMinimalPerf.default 588 575 1.02:1
Checkbox.Fluent 601 592 1.02:1
Image.Fluent 340 333 1.02:1
Text.Fluent 314 307 1.02:1
AvatarMinimalPerf.default 453 450 1.01:1
DialogMinimalPerf.default 722 714 1.01:1
EmbedMinimalPerf.default 1823 1802 1.01:1
HeaderMinimalPerf.default 345 343 1.01:1
MenuButtonMinimalPerf.default 1479 1468 1.01:1
Slider.Fluent 1526 1515 1.01:1
Tooltip.Fluent 506 500 1.01:1
InputMinimalPerf.default 1247 1241 1:1
ItemLayoutMinimalPerf.default 1187 1183 1:1
LoaderMinimalPerf.default 704 703 1:1
TextAreaMinimalPerf.default 424 425 1:1
TooltipMinimalPerf.default 737 739 1:1
Dialog.Fluent 714 717 1:1
AttachmentSlotsPerf.default 1084 1100 0.99:1
ChatDuplicateMessagesPerf.default 400 403 0.99:1
FormMinimalPerf.default 361 366 0.99:1
SplitButtonMinimalPerf.default 3508 3526 0.99:1
Dropdown.Fluent 2831 2850 0.99:1
CardMinimalPerf.default 515 526 0.98:1
SliderMinimalPerf.default 1496 1519 0.98:1
DividerMinimalPerf.default 334 345 0.97:1
ImageMinimalPerf.default 342 354 0.97:1
TreeWith60ListItems.default 190 205 0.93:1
ListWith60ListItems.default 922 1050 0.88:1
ListCommonPerf.default 630 896 0.7:1
ListNestedPerf.default 551 833 0.66:1

@wsmd
Copy link

wsmd commented Aug 17, 2020

@Jacqueline-ms looks like we have a merge conflict after #14459 was merged

@wsmd wsmd self-assigned this Aug 17, 2020
wsmd
wsmd approved these changes Aug 17, 2020
View reviewed changes
Copy link

@wsmd wsmd left a comment

Choose a reason for hiding this comment

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

:shipit:

@q-xg
Copy link
Contributor

q-xg commented Aug 18, 2020

Inside component.styles.ts, we should replace FontSizes.size13 with theme.fonts.medium.fontSize, or remove fontSize property directly because office-ui-fabric-react uses theme.fonts.medium.fontSize many places.

Jacqueline-ms
Jacqueline-ms commented Aug 18, 2020
View reviewed changes
packages/azure-themes/src/azure/styles/Modal.styles.ts
root: {
color: semanticColors.bodyText,
fontSize: FontSizes.size13,
fontSize: theme.fonts.medium.fontSize,
Copy link
Contributor Author

@Jacqueline-ms Jacqueline-ms Aug 18, 2020

Choose a reason for hiding this comment

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

@q-xg example of using theme font sizes per your comment

wsmd
wsmd reviewed Aug 19, 2020
View reviewed changes
Copy link

@wsmd wsmd left a comment

Choose a reason for hiding this comment

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

Thank you for making those changes @Jacqueline-ms. I left one question for you.

@Jacqueline-ms Jacqueline-ms changed the title Body text font size clean up, <Text> Fix Azure theme Text + Label: font size clean up, disabled state value Aug 19, 2020
wsmd
wsmd approved these changes Aug 19, 2020
View reviewed changes
Copy link

@wsmd wsmd 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 doing this @Jacqueline-ms!

@Jacqueline-ms Jacqueline-ms mentioned this pull request Aug 21, 2020
Closed
@Jacqueline-ms Jacqueline-ms merged commit b18df8a into master Aug 22, 2020
@msft-github-bot
Copy link
Contributor

msft-github-bot commented Aug 25, 2020

🎉@uifabric/azure-themes@v7.4.3 has been released which incorporates this pull request.:tada:

Handy links:

@ecraig12345 ecraig12345 deleted the jackieg/bodytext branch August 28, 2020 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hyoshis hyoshis Awaiting requested review from hyoshis

1 more reviewer

@wsmd wsmd wsmd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@wsmd wsmd

@khmakoto khmakoto

Labels

Package: azure-themes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Jacqueline-ms @msft-github-bot @wsmd @q-xg @khmakoto