Skip to content

ThemeProvider: clean up public APIs and ensure default theme is always provided #15229

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
xugao merged 6 commits into from
Sep 28, 2020
Merged

ThemeProvider: clean up public APIs and ensure default theme is always provided #15229

xugao merged 6 commits into from
Sep 28, 2020

Conversation

@xugao
Copy link
Contributor

@xugao xugao commented Sep 25, 2020

Pull request checklist

Description of changes

  • Removed FluentTheme and TeamsTheme from react-theme-provider
  • Ensure a full default theme is provided

Focus areas to test

(optional)

@size-auditor
Copy link

size-auditor bot commented Sep 25, 2020 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-next-ToggleButton 22.938 kB 39.711 kB ExceedsTolerance     16.773 kB
office-ui-fabric-react fluentui-react-next-MenuButton 28.779 kB 45.547 kB ExceedsTolerance     16.768 kB
office-ui-fabric-react fluentui-react-next-SplitButton 31.737 kB 48.505 kB ExceedsTolerance     16.768 kB
office-ui-fabric-react fluentui-react-next-Button 20.839 kB 37.532 kB ExceedsTolerance     16.693 kB
office-ui-fabric-react fluentui-react-next-Styling 62.562 kB 61.382 kB BelowBaseline     -1.18 kB

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 8eef5f6d39659d2f64519e00ae91b2908b1373a3 (build)

@xugao xugao force-pushed the xgao/themeprovider-cleanup-2 branch from 0eda7fd to 15a320a Compare September 25, 2020 23:13
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 25, 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 c7183db:

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

dzearing
dzearing reviewed Sep 28, 2020
View reviewed changes
@xugao xugao force-pushed the xgao/themeprovider-cleanup-2 branch from 15a320a to 4b35b87 Compare September 28, 2020 16:14
@msft-github-bot
Copy link
Contributor

msft-github-bot commented Sep 28, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 854 848 5000
BaseButton mount 956 936 5000
Breadcrumb mount 40289 40471 5000
BreadcrumbNext mount 154338 154828 5000
ButtonNext mount 585 575 5000
Checkbox mount 1610 1636 5000
CheckboxBase mount 1335 1373 5000
CheckboxNext mount 1567 1585 5000
ChoiceGroup mount 5198 5182 5000
ChoiceGroupNext mount 9576 9486 5000
ComboBox mount 936 961 1000
CommandBar mount 7663 7576 1000
ContextualMenu mount 12453 12449 1000
DefaultButton mount 1156 1156 5000
DetailsRow mount 3648 3671 5000
DetailsRowFast mount 3676 3723 5000
DetailsRowNoStyles mount 3582 3551 5000
Dialog mount 1532 1535 1000
DocumentCardTitle mount 1778 1805 1000
Dropdown mount 2620 2627 5000
FocusTrapZone mount 1686 1689 5000
FocusTrapZoneNext mount 1797 1784 5000
FocusZone mount 1817 1790 5000
IconButton mount 1810 1803 5000
Label mount 344 346 5000
Layer mount 1994 2014 5000
LayerNext mount 1871 1899 5000
Link mount 461 449 5000
LinkNext mount 478 450 5000
MenuButton mount 1484 1510 5000
MessageBar mount 2029 2007 5000
MessageBarNext mount 2015 1978 5000
Nav mount 3453 3305 1000
OverflowSet mount 1412 1430 5000
OverflowSetNext mount 1041 1033 5000
Panel mount 1461 1469 1000
Persona mount 819 846 1000
Pivot mount 1618 1441 1000
PivotNext mount 1391 1389 1000
PrimaryButton mount 1308 1317 5000
Rating mount 7917 7913 5000
RatingNext mount 7993 7854 5000
SearchBox mount 1374 1320 5000
SearchBoxNext mount 1382 1393 5000
Shimmer mount 2660 2656 5000
ShimmerNext mount 2597 2602 5000
Slider mount 1522 1522 5000
SliderNext mount 1968 1921 5000
SpinButton mount 4950 5135 5000
SpinButtonNext mount 5199 5170 5000
Spinner mount 421 426 5000
SplitButton mount 3288 3326 5000
Stack mount 538 538 5000
StackWithIntrinsicChildren mount 2040 2044 5000
StackWithTextChildren mount 5233 5186 5000
SwatchColorPicker mount 10761 10370 5000
SwatchColorPickerNext mount 10408 10646 5000
TagPicker mount 2752 2786 5000
TeachingBubble mount 48970 49179 5000
TeachingBubbleNext mount 11383 11476 5000
Text mount 467 427 5000
TextField mount 1458 1421 5000
ThemeProvider mount 3149 3103 5000
ThemeProvider virtual-rerender 586 598 5000
Toggle mount 861 839 5000
ToggleNext mount 833 819 5000
button mount 118 112 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.47 0.49 0.96:1 2000 947
🦄 Button.Fluent 0.13 0.2 0.65:1 5000 659
🔧 Checkbox.Fluent 0.67 0.35 1.91:1 1000 673
🎯 Dialog.Fluent 0.17 0.23 0.74:1 5000 825
🔧 Dropdown.Fluent 2.96 0.48 6.17:1 1000 2958
🔧 Icon.Fluent 0.15 0.06 2.5:1 5000 761
🎯 Image.Fluent 0.09 0.12 0.75:1 5000 428
🔧 Slider.Fluent 1.64 0.38 4.32:1 1000 1644
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 411
🦄 Tooltip.Fluent 0.12 14.59 0.01:1 5000 581

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 159 0 Infinity:1
AlertMinimalPerf.default 321 0 Infinity:1
AnimationMinimalPerf.default 438 0 Infinity:1
AttachmentMinimalPerf.default 176 0 Infinity:1
AttachmentSlotsPerf.default 1205 0 Infinity:1
AvatarMinimalPerf.default 510 0 Infinity:1
BoxMinimalPerf.default 412 0 Infinity:1
ButtonMinimalPerf.default 204 0 Infinity:1
ButtonOverridesMissPerf.default 1762 0 Infinity:1
ButtonSlotsPerf.default 635 0 Infinity:1
ButtonUseCssPerf.default 896 0 Infinity:1
ButtonUseCssNestingPerf.default 1160 0 Infinity:1
CardMinimalPerf.default 610 0 Infinity:1
CarouselMinimalPerf.default 481 0 Infinity:1
ChatDuplicateMessagesPerf.default 441 0 Infinity:1
ChatMinimalPerf.default 661 0 Infinity:1
CheckboxMinimalPerf.default 2892 0 Infinity:1
DialogMinimalPerf.default 837 0 Infinity:1
DividerMinimalPerf.default 411 0 Infinity:1
DropdownManyItemsPerf.default 811 0 Infinity:1
DropdownMinimalPerf.default 2954 0 Infinity:1
EmbedMinimalPerf.default 2041 0 Infinity:1
FlexMinimalPerf.default 329 0 Infinity:1
FormMinimalPerf.default 489 0 Infinity:1
HeaderSlotsPerf.default 873 0 Infinity:1
ImageMinimalPerf.default 447 0 Infinity:1
ItemLayoutMinimalPerf.default 1404 0 Infinity:1
LabelMinimalPerf.default 475 0 Infinity:1
LayoutMinimalPerf.default 475 0 Infinity:1
ListCommonPerf.default 733 0 Infinity:1
ListMinimalPerf.default 557 0 Infinity:1
ListNestedPerf.default 675 0 Infinity:1
ListWith60ListItems.default 1006 0 Infinity:1
LoaderMinimalPerf.default 790 0 Infinity:1
MenuMinimalPerf.default 925 0 Infinity:1
MenuButtonMinimalPerf.default 1672 0 Infinity:1
PortalMinimalPerf.default 176 0 Infinity:1
ProviderMergeThemesPerf.default 2033 0 Infinity:1
ProviderMinimalPerf.default 1091 0 Infinity:1
RadioGroupMinimalPerf.default 508 0 Infinity:1
ReactionMinimalPerf.default 462 0 Infinity:1
RefMinimalPerf.default 258 0 Infinity:1
SegmentMinimalPerf.default 413 0 Infinity:1
SkeletonMinimalPerf.default 483 0 Infinity:1
SliderMinimalPerf.default 1626 0 Infinity:1
SplitButtonMinimalPerf.default 3984 0 Infinity:1
IconMinimalPerf.default 778 0 Infinity:1
TableManyItemsPerf.default 2420 0 Infinity:1
TextMinimalPerf.default 401 0 Infinity:1
TextAreaMinimalPerf.default 559 0 Infinity:1
CustomToolbarPrototype.default 4044 0 Infinity:1
ToolbarMinimalPerf.default 1018 0 Infinity:1
TreeMinimalPerf.default 941 0 Infinity:1
TreeWith60ListItems.default 209 0 Infinity:1
VideoMinimalPerf.default 681 0 Infinity:1
Avatar.Fluent 947 0 Infinity:1
Button.Fluent 659 0 Infinity:1
Checkbox.Fluent 673 0 Infinity:1
Dialog.Fluent 825 0 Infinity:1
Dropdown.Fluent 2958 0 Infinity:1
Icon.Fluent 761 0 Infinity:1
Image.Fluent 428 0 Infinity:1
Slider.Fluent 1644 0 Infinity:1
Text.Fluent 411 0 Infinity:1
Tooltip.Fluent 581 0 Infinity:1
InputMinimalPerf.default 1359 1 1359:1
TooltipMinimalPerf.default 840 1 840:1
StatusMinimalPerf.default 822 1 822:1
PopupMinimalPerf.default 731 1 731:1
ChatWithPopoverPerf.default 504 1 504:1
TableMinimalPerf.default 462 1 462:1
HeaderMinimalPerf.default 412 1 412:1
GridMinimalPerf.default 385 1 385:1

@khmakoto
Copy link
Member

khmakoto commented Sep 28, 2020

Should we remove the getTokens file in react-theme-provider as part of this PR?

@xugao xugao force-pushed the xgao/themeprovider-cleanup-2 branch 2 times, most recently from 9a1c6c6 to 8d00d00 Compare September 28, 2020 18:31
@xugao xugao force-pushed the xgao/themeprovider-cleanup-2 branch from 8d00d00 to c7183db Compare September 28, 2020 18:56
@dzearing
Copy link
Member

dzearing commented Sep 28, 2020

oof on the bundle size; is it because Button depends on ThemeProvider?

@xugao
Copy link
Contributor Author

xugao commented Sep 28, 2020

@dzearing - the bundle size is increased due to createTheme is added useTheme and useTheme is used by button

@xugao xugao merged commit c773138 into microsoft:master Sep 28, 2020
xugao added a commit to xugao/office-ui-fabric-react that referenced this pull request Oct 9, 2020
@xugao
ThemeProvider: clean up public APIs and ensure default theme is alway…
debfd6d 
…s provided (microsoft#15229)

* themeprovider cleanups

* move theme instances out of react-theme-provider

* Change files

* cleanups

* merge

* fixes
@xugao xugao mentioned this pull request Oct 9, 2020
Merged
2 tasks
xugao added a commit that referenced this pull request Oct 9, 2020
@xugao
Cherry-pick react-theme-provider related fixes from master to 7.0 (#1…
40edbe0 
…5449)

* ThemeProvider: clean up public APIs and ensure default theme is always provided (#15229)

* themeprovider cleanups

* move theme instances out of react-theme-provider

* Change files

* cleanups

* merge

* fixes

* react-stylesheets: fix error when target is undefined (in case of SSR). (#15342)

* fix stylesheet

* Change files

* nit

* set dir (#15347)

* fix merge conflicts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dzearing dzearing dzearing approved these changes

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@khmakoto khmakoto khmakoto approved these changes

Assignees

@miroslavstastny miroslavstastny

Labels

Area: Theming

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

React-Theme-Provider Storybook broken

6 participants

@xugao @msft-github-bot @khmakoto @dzearing @ecraig12345 @miroslavstastny