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(Table): fix navigation into cell which has multiple actionable elements #12533

Merged
merged 2 commits into from
Apr 3, 2020
Merged

fix(Table): fix navigation into cell which has multiple actionable elements #12533

merged 2 commits into from
Apr 3, 2020

Conversation

kolaps33
Copy link
Contributor

@kolaps33 kolaps33 commented Apr 3, 2020 •
edited

Pull request checklist

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

Description of changes

Fix related to the bug:
https://dev.azure.com/microsoftdesign/fluent-ui/_workitems/edit/6664

Focus areas to test

test keyboard navigation

Fix:
root cause was that FocusZone was not added on the cell which has more actionable elements

Microsoft Reviewers: Open in CodeFlow

@msft-github-bot msft-github-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Apr 3, 2020
@size-auditor
Copy link

size-auditor bot commented Apr 3, 2020

Asset size changes

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

Baseline commit: 16345e00d8e3ff4071793da22ee52b5a8e49ccb3 (build)

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 835 823
BaseButton (experiments) 833 931
ChoiceGroup 5315 4940
DefaultButton 1062 1041
DefaultButton (experiments) 1876 1709
DetailsRow 3291 3512
DetailsRow (fast icons) 3384 3414
DetailsRow without styles 2849 3133
DocumentCardTitle with truncation 1523 1597
MenuButton 1243 1393
MenuButton (experiments) 3351 3163
PrimaryButton 1030 1022
PrimaryButton (experiments) 1905 1745
SplitButton 3088 2660
SplitButton (experiments) 6465 6404
Stack 463 399
Stack with Intrinsic children 1039 1025
Stack with Text children 3671 3979
Text 358 353
Toggle 787 839
Toggle (experiments) 2191 2060
button 54 61

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.53 0.45 1.18:1 2000 1068
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 564
🔧 Checkbox.Fluent 0.68 0.35 1.94:1 1000 682
🔧 Dialog.Fluent 0.4 0.19 2.11:1 5000 1994
🔧 Dropdown.Fluent 3.88 0.45 8.62:1 1000 3878
🔧 Icon.Fluent 0.19 0.05 3.8:1 5000 938
🎯 Image.Fluent 0.08 0.09 0.89:1 5000 385
🔧 Slider.Fluent 1.63 0.39 4.18:1 1000 1634
🔧 Text.Fluent 0.1 0.03 3.33:1 5000 500
🦄 Tooltip.Fluent 0.12 13.94 0.01:1 5000 605

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
Button.Fluent 564 476 1.18:1
ButtonSlotsPerf.default 719 613 1.17:1
SegmentMinimalPerf.default 1228 1059 1.16:1
ButtonMinimalPerf.default 182 158 1.15:1
ChatWithPopoverPerf.default 586 522 1.12:1
CarouselMinimalPerf.default 686 616 1.11:1
GridMinimalPerf.default 965 870 1.11:1
ListMinimalPerf.default 553 505 1.1:1
Image.Fluent 385 352 1.09:1
AvatarMinimalPerf.default 593 555 1.07:1
MenuButtonMinimalPerf.default 1705 1614 1.06:1
Text.Fluent 500 470 1.06:1
AnimationMinimalPerf.default 732 699 1.05:1
CardMinimalPerf.default 398 380 1.05:1
HierarchicalTreeMinimalPerf.default 1039 991 1.05:1
ToolbarMinimalPerf.default 1229 1170 1.05:1
TreeWith60ListItems.default 247 236 1.05:1
Slider.Fluent 1634 1560 1.05:1
AttachmentMinimalPerf.default 863 827 1.04:1
ChatDuplicateMessagesPerf.default 468 449 1.04:1
CheckboxMinimalPerf.default 3379 3245 1.04:1
HeaderSlotsPerf.default 1800 1735 1.04:1
InputMinimalPerf.default 1049 1006 1.04:1
LayoutMinimalPerf.default 670 652 1.03:1
ListNestedPerf.default 1040 1012 1.03:1
ProviderMinimalPerf.default 659 638 1.03:1
StatusMinimalPerf.default 649 631 1.03:1
TextAreaMinimalPerf.default 3027 2925 1.03:1
CustomToolbarPrototype.default 3544 3437 1.03:1
ChatMinimalPerf.default 650 637 1.02:1
DividerMinimalPerf.default 1140 1123 1.02:1
DropdownMinimalPerf.default 3532 3465 1.02:1
ImageMinimalPerf.default 413 403 1.02:1
LoaderMinimalPerf.default 1071 1052 1.02:1
TextMinimalPerf.default 475 465 1.02:1
Dropdown.Fluent 3878 3813 1.02:1
FlexMinimalPerf.default 305 303 1.01:1
SliderMinimalPerf.default 1526 1514 1.01:1
TooltipMinimalPerf.default 803 793 1.01:1
AccordionMinimalPerf.default 281 281 1:1
EmbedMinimalPerf.default 5488 5470 1:1
IconMinimalPerf.default 451 449 1:1
VideoMinimalPerf.default 965 967 1:1
Icon.Fluent 938 942 1:1
ItemLayoutMinimalPerf.default 2156 2183 0.99:1
LabelMinimalPerf.default 431 434 0.99:1
PortalMinimalPerf.default 308 310 0.99:1
ProviderMergeThemesPerf.default 1380 1394 0.99:1
ListCommonPerf.default 1031 1049 0.98:1
RefMinimalPerf.default 170 174 0.98:1
TreeMinimalPerf.default 1216 1240 0.98:1
Avatar.Fluent 1068 1085 0.98:1
Tooltip.Fluent 605 620 0.98:1
AttachmentSlotsPerf.default 3472 3572 0.97:1
Dialog.Fluent 1994 2055 0.97:1
RadioGroupMinimalPerf.default 522 546 0.96:1
BoxMinimalPerf.default 434 455 0.95:1
HeaderMinimalPerf.default 533 567 0.94:1
ListWith60ListItems.default 1217 1297 0.94:1
TableMinimalPerf.default 673 718 0.94:1
Checkbox.Fluent 682 729 0.94:1
DialogMinimalPerf.default 1966 2112 0.93:1
AlertMinimalPerf.default 582 640 0.91:1
DropdownManyItemsPerf.default 1488 1642 0.91:1
ReactionMinimalPerf.default 2353 2572 0.91:1
MenuMinimalPerf.default 1843 2043 0.9:1
SplitButtonMinimalPerf.default 3455 3821 0.9:1
FormMinimalPerf.default 932 1109 0.84:1
PopupMinimalPerf.default 224 271 0.83:1

@kolaps33 kolaps33 merged commit ce4a3d5 into microsoft:master Apr 3, 2020
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
@kolaps33 @DuanShaolong
fix(Table): fix navigation into cell which has multiple actionable el…
372bcdd 
…ements (microsoft#12533)

* fixing navigation into cell which has multiple focusable elements

* updating changelog

Co-authored-by: Milan Turon <mituron@microsoft.com>
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

@layershifter layershifter layershifter approved these changes

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@mnajdova mnajdova Awaiting requested review from mnajdova

Assignees

@aneeshack4 aneeshack4

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.

None yet
5 participants
@kolaps33 @msft-github-bot @pompomon @layershifter @aneeshack4