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.

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

fix(RadioGroup): Update examples to enable input for in the tab order inside Radio Group #13110

Merged

Conversation

assuncaocharles
Copy link
Contributor

@assuncaocharles assuncaocharles commented May 12, 2020

Pull request checklist

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

Description of changes

Update examples to enable input for in the tab order inside Radio Group

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@msft-github-bot
Copy link
Contributor

msft-github-bot commented May 12, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 828 852 5000
Checkbox 1627 1551 5000
CheckboxBase 1286 1325 5000
CheckboxNext 1577 1579 5000
ChoiceGroup 4895 4955 5000
ComboBox 911 865 1000
CommandBar 7084 7097 1000
ContextualMenu 12593 11808 1000
DefaultButton 1089 1061 5000
DetailsRow 3303 3407 5000
DetailsRow (fast icons) 3362 3390 5000
DetailsRow without styles 3194 3266 5000
Dialog 1463 1510 1000
DocumentCardTitle with truncation 1548 1562 1000
Dropdown 2472 2438 5000
FocusZone 1638 1592 5000
IconButton 1724 1738 5000
Label 343 302 5000
Link 478 461 5000
MenuButton 1442 1428 5000
Nav 3129 3517 1000
Panel 1425 1401 1000
Persona 809 830 1000
Pivot 1422 1462 1000
PrimaryButton 1278 1273 5000
SearchBox 1324 1262 5000
Slider 1566 1523 5000
Spinner 399 393 5000
SplitButton 3379 3383 5000
Stack 493 505 5000
Stack with Intrinsic children 1105 1174 5000
Stack with Text children 4284 4212 5000
TagPicker 2734 2752 5000
Text 399 407 5000
TextField 1375 1409 5000
Toggle 893 895 5000
button 69 73 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.47 0.46 1.02:1 2000 943
🦄 Button.Fluent 0.11 0.18 0.61:1 5000 526
🔧 Checkbox.Fluent 0.61 0.36 1.69:1 1000 607
🔧 Dialog.Fluent 0.36 0.19 1.89:1 5000 1805
🔧 Dropdown.Fluent 3.23 0.44 7.34:1 1000 3226
🔧 Icon.Fluent 0.14 0.05 2.8:1 5000 709
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 353
🔧 Slider.Fluent 1.35 0.32 4.22:1 1000 1352
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 343
🦄 Tooltip.Fluent 0.09 14.02 0.01:1 5000 454

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonMinimalPerf.default 185 159 1.16:1
CarouselMinimalPerf.default 485 446 1.09:1
AnimationMinimalPerf.default 659 613 1.08:1
AttachmentMinimalPerf.default 148 137 1.08:1
ItemLayoutMinimalPerf.default 1780 1654 1.08:1
LayoutMinimalPerf.default 620 576 1.08:1
RefMinimalPerf.default 208 193 1.08:1
HeaderMinimalPerf.default 572 535 1.07:1
GridMinimalPerf.default 714 672 1.06:1
ButtonSlotsPerf.default 618 590 1.05:1
CardMinimalPerf.default 587 561 1.05:1
ListMinimalPerf.default 483 462 1.05:1
TooltipMinimalPerf.default 727 695 1.05:1
VideoMinimalPerf.default 660 629 1.05:1
AttachmentSlotsPerf.default 1166 1118 1.04:1
AvatarMinimalPerf.default 518 498 1.04:1
ChatWithPopoverPerf.default 577 557 1.04:1
AlertMinimalPerf.default 318 309 1.03:1
ImageMinimalPerf.default 387 377 1.03:1
ProviderMinimalPerf.default 656 637 1.03:1
ReactionMinimalPerf.default 1973 1916 1.03:1
SliderMinimalPerf.default 1358 1319 1.03:1
TreeWith60ListItems.default 224 217 1.03:1
Avatar.Fluent 943 916 1.03:1
Slider.Fluent 1352 1311 1.03:1
Tooltip.Fluent 454 441 1.03:1
ChatDuplicateMessagesPerf.default 400 392 1.02:1
LabelMinimalPerf.default 414 407 1.02:1
PopupMinimalPerf.default 260 256 1.02:1
SplitButtonMinimalPerf.default 3397 3319 1.02:1
TableMinimalPerf.default 392 385 1.02:1
TextAreaMinimalPerf.default 499 490 1.02:1
ToolbarMinimalPerf.default 861 841 1.02:1
CheckboxMinimalPerf.default 2857 2826 1.01:1
DividerMinimalPerf.default 332 329 1.01:1
DropdownManyItemsPerf.default 1316 1298 1.01:1
DropdownMinimalPerf.default 3157 3134 1.01:1
EmbedMinimalPerf.default 1971 1952 1.01:1
FlexMinimalPerf.default 283 280 1.01:1
FormMinimalPerf.default 756 749 1.01:1
HeaderSlotsPerf.default 1560 1551 1.01:1
ListWith60ListItems.default 1183 1177 1.01:1
LoaderMinimalPerf.default 773 762 1.01:1
MenuButtonMinimalPerf.default 1657 1634 1.01:1
RadioGroupMinimalPerf.default 595 591 1.01:1
Checkbox.Fluent 607 600 1.01:1
Text.Fluent 343 341 1.01:1
BoxMinimalPerf.default 315 316 1:1
DialogMinimalPerf.default 1733 1737 1:1
ListCommonPerf.default 984 983 1:1
PortalMinimalPerf.default 306 307 1:1
ProviderMergeThemesPerf.default 1575 1575 1:1
StatusMinimalPerf.default 685 687 1:1
Dialog.Fluent 1805 1810 1:1
ChatMinimalPerf.default 638 644 0.99:1
HierarchicalTreeMinimalPerf.default 1031 1037 0.99:1
InputMinimalPerf.default 975 986 0.99:1
MenuMinimalPerf.default 1862 1881 0.99:1
SegmentMinimalPerf.default 932 942 0.99:1
TreeMinimalPerf.default 1253 1266 0.99:1
Dropdown.Fluent 3226 3251 0.99:1
Icon.Fluent 709 717 0.99:1
Image.Fluent 353 356 0.99:1
ListNestedPerf.default 917 939 0.98:1
CustomToolbarPrototype.default 3412 3520 0.97:1
Button.Fluent 526 540 0.97:1
TextMinimalPerf.default 332 348 0.95:1
IconMinimalPerf.default 643 687 0.94:1
AccordionMinimalPerf.default 141 161 0.88:1

@size-auditor
Copy link

size-auditor bot commented May 12, 2020

Asset size changes

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

Baseline commit: 767c1b15e2dbc67d98d1f36d93d027e7cf881459 (build)

@silviuaavram
Copy link
Contributor

silviuaavram commented May 12, 2020

This example is just so weird. We removed the input from the tab order just to put some custom keyboard handling in this PR.

Why was the input removed from the tab order from the first place?

Edit: maybe because we are focusing the whole wrapper on tab instead of the radio button. Which is not a radio button. This looks just so different from what an <input type="radio"> is.

@assuncaocharles
Copy link
Contributor Author

This example is just so weird. We removed the input from the tab order just to put some custom keyboard handling in this PR.

Why was the input removed from the tab order from the first place?

Edit: maybe because we are focusing the whole wrapper on tab instead of the radio button. Which is not a radio button. This looks just so different from what an <input type="radio"> is.

I was also wondering if it was really a problem with the behavior or just a matter of the example that explicitly remove the input from tab order

@assuncaocharles assuncaocharles changed the title fix(RadioGroup): focus internal input when press tab on item fix(RadioGroup): Update examples to enable input for in the tab order inside Radio Group May 12, 2020
@assuncaocharles assuncaocharles merged commit 26f0acf into microsoft:master May 12, 2020
@assuncaocharles assuncaocharles deleted the fix/radio-group-focus branch May 12, 2020 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants