Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

Moving zIndex values to siteVariables#2311

Merged
pompomon merged 10 commits intomasterfrom
pompomon/zindex-vars
Feb 5, 2020
Merged

Moving zIndex values to siteVariables#2311
pompomon merged 10 commits intomasterfrom
pompomon/zindex-vars

Conversation

@pompomon
Copy link
Contributor

@pompomon pompomon commented Feb 3, 2020

Extracting all existing zIndex values to siteVariables and replacing magic numbers in styles/defaults

@DustyTheBot
Copy link
Collaborator

DustyTheBot commented Feb 3, 2020 •
edited
Loading


Fails
🚫 Invalid entry format in CHANGELOG.md: >+- Move `zIndex` values from default variable values and styles to siteVariables object in Teams theme @pompomon ([#2311](https://github.com//pull/2311)) <

The correct format is: - description @githubname ([#DDDD](https://github.com/microsoft/fluent-ui-react/pull/DDDD)

Perf comparison

Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.57 0.39 1.46:1 2000 1138
🔧 Button.Fluent 1.32 0.19 6.95:1 1000 1317
🔧 Checkbox.Fluent 1.4 0.28 5:1 1000 1404
🔧 Dialog.Fluent 0.33 0.16 2.06:1 5000 1658
🔧 Dropdown.Fluent 3.77 0.36 10.47:1 1000 3767
🔧 Icon.Fluent 0.25 0.03 8.33:1 5000 1251
🔧 Image.Fluent 0.09 0.07 1.29:1 5000 466
🔧 Slider.Fluent 1.86 0.28 6.64:1 1000 1858
🦄 Text.Fluent 0.05 0.19 0.26:1 5000 273
🦄 Tooltip.Fluent 0.37 17.43 0.02:1 5000 1867

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ProviderMergeThemesPerf.default 1677 1053 1.59:1
PortalMinimalPerf.default 361 241 1.5:1
RefMinimalPerf.default 176 153 1.15:1
TextMinimalPerf.default 291 256 1.14:1
FlexMinimalPerf.default 346 313 1.11:1
CheckboxMinimalPerf.default 7008 6481 1.08:1
RadioGroupMinimalPerf.default 395 365 1.08:1
Button.Fluent 1317 1217 1.08:1
AvatarMinimalPerf.default 612 570 1.07:1
VideoMinimalPerf.default 716 667 1.07:1
Dropdown.Fluent 3767 3513 1.07:1
ToolbarMinimalPerf.default 735 692 1.06:1
ChatDuplicateMessagesPerf.default 571 550 1.04:1
FormMinimalPerf.default 661 635 1.04:1
LayoutMinimalPerf.default 470 452 1.04:1
Checkbox.Fluent 1404 1353 1.04:1
Dialog.Fluent 1658 1589 1.04:1
Tooltip.Fluent 1867 1803 1.04:1
AttachmentSlotsPerf.default 3174 3073 1.03:1
DialogMinimalPerf.default 1620 1576 1.03:1
ItemLayoutMinimalPerf.default 1558 1519 1.03:1
LabelMinimalPerf.default 1650 1606 1.03:1
ReactionMinimalPerf.default 2469 2406 1.03:1
Icon.Fluent 1251 1210 1.03:1
GridMinimalPerf.default 761 746 1.02:1
IconMinimalPerf.default 1031 1010 1.02:1
ImageMinimalPerf.default 473 464 1.02:1
PopupMinimalPerf.default 341 335 1.02:1
Text.Fluent 273 267 1.02:1
AnimationMinimalPerf.default 420 415 1.01:1
CarouselMinimalPerf.default 1758 1734 1.01:1
DividerMinimalPerf.default 824 816 1.01:1
HierarchicalTreeMinimalPerf.default 745 741 1.01:1
MenuButtonMinimalPerf.default 1408 1399 1.01:1
SplitButtonMinimalPerf.default 11538 11433 1.01:1
DropdownManyItemsPerf.default 454 453 1:1
SegmentMinimalPerf.default 1236 1236 1:1
Image.Fluent 466 467 1:1
AlertMinimalPerf.default 517 524 0.99:1
AttachmentMinimalPerf.default 859 864 0.99:1
ButtonMinimalPerf.default 1219 1229 0.99:1
ButtonSlotsPerf.default 1626 1646 0.99:1
ChatMinimalPerf.default 1596 1620 0.99:1
InputMinimalPerf.default 904 915 0.99:1
AccordionMinimalPerf.default 176 179 0.98:1
EmbedMinimalPerf.default 5865 6007 0.98:1
ListCommonPerf.default 1005 1023 0.98:1
SliderMinimalPerf.default 1902 1934 0.98:1
TreeMinimalPerf.default 822 842 0.98:1
Slider.Fluent 1858 1895 0.98:1
HeaderSlotsPerf.default 1218 1256 0.97:1
ListMinimalPerf.default 613 633 0.97:1
TextAreaMinimalPerf.default 2812 2944 0.96:1
ChatWithPopoverPerf.default 605 634 0.95:1
MenuMinimalPerf.default 1817 1912 0.95:1
StatusMinimalPerf.default 825 869 0.95:1
Avatar.Fluent 1138 1192 0.95:1
DropdownMinimalPerf.default 3221 3411 0.94:1
HeaderMinimalPerf.default 394 429 0.92:1
ProviderMinimalPerf.default 541 586 0.92:1
CustomToolbarPrototype.default 3965 4437 0.89:1
TooltipMinimalPerf.default 2060 2321 0.89:1
BoxMinimalPerf.default 204 238 0.86:1
TableMinimalPerf.default 586 695 0.84:1
LoaderMinimalPerf.default 1933 2342 0.83:1

Generated by 🚫 dangerJS

@pompomon pompomon force-pushed the pompomon/zindex-vars branch from 8abf2e4 to 69dbc7d Compare February 4, 2020 15:00
notandrew
notandrew reviewed Feb 4, 2020
View reviewed changes
mnajdova
mnajdova reviewed Feb 5, 2020
View reviewed changes
mnajdova
mnajdova reviewed Feb 5, 2020
View reviewed changes
mnajdova
mnajdova approved these changes Feb 5, 2020
View reviewed changes
Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

LGTM! Please see final comments before merging.

layershifter
layershifter reviewed Feb 5, 2020
View reviewed changes
@pompomon pompomon merged commit c501557 into master Feb 5, 2020
@pompomon pompomon deleted the pompomon/zindex-vars branch February 5, 2020 14:39
This was referenced Feb 28, 2020
Merged
Merged
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing

@ecraig12345 ecraig12345 Awaiting requested review from ecraig12345 ecraig12345 is a code owner

@JasonGore JasonGore Awaiting requested review from JasonGore JasonGore is a code owner

@jdhuntington jdhuntington Awaiting requested review from jdhuntington

@kenotron kenotron Awaiting requested review from kenotron

@levithomason levithomason Awaiting requested review from levithomason levithomason is a code owner

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny miroslavstastny is a code owner

3 more reviewers

@mnajdova mnajdova mnajdova approved these changes

@notandrew notandrew notandrew left review comments

@layershifter layershifter layershifter left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pompomon @DustyTheBot @mnajdova @notandrew @layershifter