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.

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

fix(dropdown): scroll loading into view #13175

Merged
merged 13 commits into from
May 22, 2020
Merged

fix(dropdown): scroll loading into view #13175

merged 13 commits into from
May 22, 2020

Conversation

assuncaocharles
Copy link
Contributor

@assuncaocharles assuncaocharles commented May 15, 2020 •
edited

Pull request checklist

Description of changes

This PR set loadingMessage or headerMessage into view when they exist.

Screen Recording 2020-05-15 at 10 05 03

Focus areas to test

(optional)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented May 15, 2020 •
edited

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 795 781 5000
Checkbox 1494 1546 5000
CheckboxBase 1268 1233 5000
CheckboxNext 1519 1477 5000
ChoiceGroup 4804 4776 5000
ComboBox 897 879 1000
CommandBar 7547 7516 1000
ContextualMenu 12444 12456 1000
DefaultButton 1019 1016 5000
DetailsRow 3341 3307 5000
DetailsRow (fast icons) 3429 3357 5000
DetailsRow without styles 3242 3242 5000
Dialog 1412 1427 1000
DocumentCardTitle with truncation 1918 1904 1000
Dropdown 2388 2395 5000
FocusZone 1744 1666 5000
IconButton 1638 1634 5000
Label 310 318 5000
Link 432 429 5000
LinkNext 420 425 5000
MenuButton 1398 1378 5000
Nav 3152 3456 1000
Panel 1400 1442 1000
Persona 817 815 1000
Pivot 1334 1352 1000
PrimaryButton 1205 1218 5000
SearchBox 1177 1199 5000
Slider 1438 1423 5000
SliderNext 1473 1456 5000
Spinner 384 394 5000
SplitButton 2996 3033 5000
Stack 469 465 5000
Stack with Intrinsic children 1800 1766 5000
Stack with Text children 4740 4807 5000
TagPicker 2674 2656 5000
Text 362 379 5000
TextField 1352 1351 5000
Toggle 835 880 5000
ToggleNext 859 832 5000
button 81 74 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.67 0.47 1.43:1 2000 1331
🦄 Button.Fluent 0.1 0.18 0.56:1 5000 511
🔧 Checkbox.Fluent 1.04 0.34 3.06:1 1000 1043
🔧 Dialog.Fluent 0.55 0.2 2.75:1 5000 2736
🔧 Dropdown.Fluent 6.07 0.44 13.8:1 1000 6072
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 635
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 338
🔧 Slider.Fluent 2.77 0.33 8.39:1 1000 2770
🔧 Text.Fluent 0.06 0.02 3:1 5000 318
🦄 Tooltip.Fluent 0.09 16.68 0.01:1 5000 464

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
RefMinimalPerf.default 199 184 1.08:1
ListMinimalPerf.default 464 434 1.07:1
Text.Fluent 318 303 1.05:1
AttachmentMinimalPerf.default 152 146 1.04:1
LabelMinimalPerf.default 375 359 1.04:1
SegmentMinimalPerf.default 308 295 1.04:1
Image.Fluent 338 325 1.04:1
AttachmentSlotsPerf.default 1223 1191 1.03:1
CardMinimalPerf.default 518 503 1.03:1
CheckboxMinimalPerf.default 5058 4925 1.03:1
DialogMinimalPerf.default 2784 2714 1.03:1
FormMinimalPerf.default 362 353 1.03:1
ListWith60ListItems.default 1523 1472 1.03:1
BoxMinimalPerf.default 307 301 1.02:1
DividerMinimalPerf.default 314 308 1.02:1
FlexMinimalPerf.default 277 272 1.02:1
HierarchicalTreeMinimalPerf.default 385 379 1.02:1
ItemLayoutMinimalPerf.default 2510 2454 1.02:1
ListCommonPerf.default 1092 1070 1.02:1
PortalMinimalPerf.default 113 111 1.02:1
TableMinimalPerf.default 366 358 1.02:1
ToolbarMinimalPerf.default 742 726 1.02:1
ChatDuplicateMessagesPerf.default 537 533 1.01:1
DropdownMinimalPerf.default 6124 6062 1.01:1
LayoutMinimalPerf.default 768 762 1.01:1
MenuButtonMinimalPerf.default 1615 1598 1.01:1
ProviderMinimalPerf.default 801 791 1.01:1
StatusMinimalPerf.default 612 606 1.01:1
AlertMinimalPerf.default 310 311 1:1
AnimationMinimalPerf.default 692 689 1:1
ButtonMinimalPerf.default 169 169 1:1
DropdownManyItemsPerf.default 2071 2064 1:1
GridMinimalPerf.default 1264 1267 1:1
HeaderSlotsPerf.default 1273 1267 1:1
ImageMinimalPerf.default 337 337 1:1
InputMinimalPerf.default 1545 1547 1:1
ListNestedPerf.default 1038 1040 1:1
MenuMinimalPerf.default 640 643 1:1
PopupMinimalPerf.default 248 248 1:1
ProviderMergeThemesPerf.default 1720 1719 1:1
RadioGroupMinimalPerf.default 538 540 1:1
TextMinimalPerf.default 323 322 1:1
VideoMinimalPerf.default 572 570 1:1
Avatar.Fluent 1331 1333 1:1
Dialog.Fluent 2736 2723 1:1
Icon.Fluent 635 633 1:1
ChatMinimalPerf.default 560 568 0.99:1
SliderMinimalPerf.default 2796 2820 0.99:1
SplitButtonMinimalPerf.default 3549 3589 0.99:1
IconMinimalPerf.default 633 638 0.99:1
CustomToolbarPrototype.default 4620 4676 0.99:1
TooltipMinimalPerf.default 698 708 0.99:1
Button.Fluent 511 514 0.99:1
Checkbox.Fluent 1043 1051 0.99:1
Dropdown.Fluent 6072 6135 0.99:1
AvatarMinimalPerf.default 686 699 0.98:1
EmbedMinimalPerf.default 3267 3321 0.98:1
ReactionMinimalPerf.default 345 353 0.98:1
TreeMinimalPerf.default 1218 1246 0.98:1
Slider.Fluent 2770 2821 0.98:1
CarouselMinimalPerf.default 481 498 0.97:1
TreeWith60ListItems.default 273 281 0.97:1
Tooltip.Fluent 464 476 0.97:1
ButtonSlotsPerf.default 693 723 0.96:1
HeaderMinimalPerf.default 320 334 0.96:1
LoaderMinimalPerf.default 1125 1183 0.95:1
ChatWithPopoverPerf.default 631 679 0.93:1
TextAreaMinimalPerf.default 404 453 0.89:1
AccordionMinimalPerf.default 118 137 0.86:1

@size-auditor
Copy link

size-auditor bot commented May 15, 2020 •
edited

Asset size changes

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

Baseline commit: e3e33eb834f2288e3d4a7786800480604bbb476f (build)

@layershifter layershifter added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label May 19, 2020
@assuncaocharles
Copy link
Contributor Author

@pompomon @layershifter Any thoughts on this one?

pompomon
pompomon approved these changes May 21, 2020
View reviewed changes
Copy link
Contributor

@pompomon pompomon left a comment

Choose a reason for hiding this comment

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

Please could you add tests for both loading message and header to see that the fix is working and won't break by next changes?

@assuncaocharles assuncaocharles merged commit e0ac865 into microsoft:master May 22, 2020
@assuncaocharles assuncaocharles deleted the fix/dropdown-scroll branch May 22, 2020 10:52
miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
@assuncaocharles @miroslavstastny
fix(dropdown): scroll loading into view (microsoft#13175)
59ac488 
* fix(dropdown): scroll loading into view

* fix(dropdown): fix header condition

* fix(dropdown): format comment

* fix(dropdown): downgrade to approved version

* fix(dropdown): Screenshot

* fix(dropdown): Screenshot

* fix(dropdown): Screenshot

* fix(Dropdown): change stops

* fix(Dropdown): snapshot

* fix(Dropdown): screenshot

* fix(Dropdown): screenshot

* fix(Dropdown): changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pompomon pompomon pompomon approved these changes

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@layershifter layershifter Awaiting requested review from layershifter

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@mnajdova mnajdova Awaiting requested review from mnajdova

Assignees

@ecraig12345 ecraig12345

Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dropdown list doesn't account for Loading message when some items are present
5 participants
@assuncaocharles @msft-github-bot @pompomon @ecraig12345 @layershifter