Skip to content

Button: Setting focus visibility to true in scenarios where it should be supported#17820

Merged
khmakoto merged 2 commits intomicrosoft:masterfrom
khmakoto:buttonFocus
Apr 15, 2021
Merged

Button: Setting focus visibility to true in scenarios where it should be supported#17820
khmakoto merged 2 commits intomicrosoft:masterfrom
khmakoto:buttonFocus

Conversation

@khmakoto
Copy link
Member

@khmakoto khmakoto commented Apr 14, 2021

Pull request checklist

Description of changes

This PR fixes 2 scenarios where keyboard focus should appear but wasn't when using the Button component in the @fluentui/react package.

The first scenario was that when programmatically focusing a Button the focus styling wouldn't be set.

Before:
ButtonProgrammaticallyFocusBefore

After:
ButtonProgrammaticallyFocusAfter

The second scenario was that when clicking on a Button with a menu (so that the focus was already on the element), dismissing the menu and then opening it via pressing the Enter or Space keys, the focus indicator was not appearing:

Before:
MenuButtonBefore

After:
MenuButton

@size-auditor
Copy link

size-auditor bot commented Apr 14, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Dropdown 215.782 kB 215.848 kB ExceedsBaseline     66 bytes
office-ui-fabric-react fluentui-react-DocumentCard 199.893 kB 199.959 kB ExceedsBaseline     66 bytes
office-ui-fabric-react fluentui-react-Dialog 195.12 kB 195.186 kB ExceedsBaseline     66 bytes
office-ui-fabric-react fluentui-react-FloatingPicker 225.125 kB 225.189 kB ExceedsBaseline     64 bytes
office-ui-fabric-react fluentui-react-ComboBox 229.495 kB 229.559 kB ExceedsBaseline     64 bytes
office-ui-fabric-react fluentui-react-TeachingBubble 189.77 kB 189.832 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-Pivot 173.647 kB 173.709 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-SelectedItemsList 214.704 kB 214.766 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-CommandBar 185.862 kB 185.924 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-SwatchColorPicker 175.262 kB 175.324 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-Grid 165.762 kB 165.824 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-Breadcrumb 184.33 kB 184.392 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-ButtonGrid 165.762 kB 165.824 kB ExceedsBaseline     62 bytes
office-ui-fabric-react fluentui-react-MessageBar 173.437 kB 173.495 kB ExceedsBaseline     58 bytes
office-ui-fabric-react fluentui-react-Pickers 269.021 kB 269.079 kB ExceedsBaseline     58 bytes
office-ui-fabric-react fluentui-react-Facepile 194.455 kB 194.513 kB ExceedsBaseline     58 bytes
office-ui-fabric-react fluentui-react-SearchBox 172.377 kB 172.435 kB ExceedsBaseline     58 bytes
office-ui-fabric-react fluentui-react-Nav 173.707 kB 173.763 kB ExceedsBaseline     56 bytes
office-ui-fabric-react fluentui-react-Panel 185.038 kB 185.092 kB ExceedsBaseline     54 bytes
office-ui-fabric-react fluentui-react-SpinButton 176.526 kB 176.58 kB ExceedsBaseline     54 bytes
office-ui-fabric-react fluentui-react-Button 179.657 kB 179.711 kB ExceedsBaseline     54 bytes

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

Baseline commit: 9a01ff085821286d7b684cb37a5a83eb195d56a7 (build)

@codesandbox-ci
Copy link

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 5ad895f:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@fabricteam
Copy link
Collaborator

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 958 937 5000
BaseButton mount 941 963 5000
Breadcrumb mount 42930 42875 5000
ButtonNext mount 600 625 5000
Checkbox mount 1627 1624 5000
CheckboxBase mount 1382 1372 5000
ChoiceGroup mount 4972 4957 5000
ComboBox mount 1006 1004 1000
CommandBar mount 10034 10613 1000
ContextualMenu mount 6266 6201 1000
DefaultButton mount 1191 1170 5000
DetailsRow mount 3789 3727 5000
DetailsRowFast mount 3806 3794 5000
DetailsRowNoStyles mount 3550 3585 5000
Dialog mount 1505 1530 1000
DocumentCardTitle mount 1794 1811 1000
Dropdown mount 3369 3323 5000
FocusTrapZone mount 1838 1834 5000
FocusZone mount 1811 1789 5000
IconButton mount 1834 1840 5000
Label mount 338 334 5000
Layer mount 1827 1846 5000
Link mount 469 484 5000
MakeStyles mount 1820 1839 50000
MenuButton mount 1530 1550 5000
MessageBar mount 2059 2029 5000
Nav mount 3576 3421 1000
OverflowSet mount 1032 1032 5000
Panel mount 1348 1434 1000
Persona mount 853 864 1000
Pivot mount 1408 1444 1000
PrimaryButton mount 1303 1373 5000
Rating mount 8178 8161 5000
SearchBox mount 1379 1460 5000
Shimmer mount 2678 2672 5000
Slider mount 2060 2058 5000
SpinButton mount 5144 5143 5000
Spinner mount 433 404 5000
SplitButton mount 3298 3240 5000
Stack mount 506 552 5000
StackWithIntrinsicChildren mount 1605 1600 5000
StackWithTextChildren mount 4856 4861 5000
SwatchColorPicker mount 10521 10647 5000
Tabs mount 1426 1425 1000
TagPicker mount 2543 2548 5000
TeachingBubble mount 11905 11914 5000
Text mount 436 438 5000
TextField mount 1440 1442 5000
ThemeProvider mount 1195 1208 5000
ThemeProvider virtual-rerender 603 608 5000
ThemeProviderNext mount 15658 15418 5000
Toggle mount 853 835 5000
buttonNative mount 112 106 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.18 0.5 0.36:1 2000 362
🦄 Button.Fluent 0.12 0.21 0.57:1 5000 602
🔧 Checkbox.Fluent 0.66 0.38 1.74:1 1000 657
🎯 Dialog.Fluent 0.16 0.23 0.7:1 5000 794
🔧 Dropdown.Fluent 3.12 0.44 7.09:1 1000 3116
🔧 Icon.Fluent 0.13 0.06 2.17:1 5000 651
🦄 Image.Fluent 0.09 0.14 0.64:1 5000 435
🔧 Slider.Fluent 1.63 0.47 3.47:1 1000 1631
🔧 Text.Fluent 0.08 0.04 2:1 5000 401
🦄 Tooltip.Fluent 0.16 0.9 0.18:1 5000 776

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AvatarMinimalPerf.default 240 209 1.15:1
IconMinimalPerf.default 709 632 1.12:1
AlertMinimalPerf.default 331 303 1.09:1
TextMinimalPerf.default 401 372 1.08:1
Tooltip.Fluent 776 720 1.08:1
FormMinimalPerf.default 479 446 1.07:1
SegmentMinimalPerf.default 403 377 1.07:1
ButtonMinimalPerf.default 213 201 1.06:1
HeaderMinimalPerf.default 418 395 1.06:1
ImageMinimalPerf.default 445 425 1.05:1
LabelMinimalPerf.default 448 428 1.05:1
PopupMinimalPerf.default 780 742 1.05:1
PortalMinimalPerf.default 173 165 1.05:1
StatusMinimalPerf.default 768 733 1.05:1
TreeWith60ListItems.default 204 194 1.05:1
VideoMinimalPerf.default 690 658 1.05:1
ButtonSlotsPerf.default 619 597 1.04:1
FlexMinimalPerf.default 318 305 1.04:1
ReactionMinimalPerf.default 432 415 1.04:1
Text.Fluent 401 384 1.04:1
ChatMinimalPerf.default 672 653 1.03:1
DropdownManyItemsPerf.default 769 744 1.03:1
ItemLayoutMinimalPerf.default 1357 1322 1.03:1
ListMinimalPerf.default 560 544 1.03:1
LoaderMinimalPerf.default 742 719 1.03:1
MenuButtonMinimalPerf.default 1713 1670 1.03:1
TooltipMinimalPerf.default 1052 1024 1.03:1
Avatar.Fluent 362 350 1.03:1
Image.Fluent 435 422 1.03:1
AnimationMinimalPerf.default 453 445 1.02:1
ChatDuplicateMessagesPerf.default 321 316 1.02:1
DatepickerMinimalPerf.default 48600 47633 1.02:1
LayoutMinimalPerf.default 418 409 1.02:1
ListWith60ListItems.default 716 699 1.02:1
RadioGroupMinimalPerf.default 484 476 1.02:1
TableMinimalPerf.default 452 444 1.02:1
AttachmentSlotsPerf.default 1198 1190 1.01:1
BoxMinimalPerf.default 400 397 1.01:1
ButtonUseCssPerf.default 870 860 1.01:1
ButtonUseCssNestingPerf.default 1136 1122 1.01:1
MenuMinimalPerf.default 975 963 1.01:1
RosterPerf.default 1302 1288 1.01:1
SplitButtonMinimalPerf.default 3992 3963 1.01:1
Button.Fluent 602 594 1.01:1
Checkbox.Fluent 657 650 1.01:1
ButtonOverridesMissPerf.default 1783 1778 1:1
DialogMinimalPerf.default 771 770 1:1
DropdownMinimalPerf.default 3095 3102 1:1
GridMinimalPerf.default 373 373 1:1
HeaderSlotsPerf.default 829 833 1:1
RefMinimalPerf.default 261 262 1:1
TableManyItemsPerf.default 2064 2071 1:1
TextAreaMinimalPerf.default 576 574 1:1
CustomToolbarPrototype.default 3954 3953 1:1
Slider.Fluent 1631 1635 1:1
AttachmentMinimalPerf.default 184 185 0.99:1
CarouselMinimalPerf.default 510 515 0.99:1
ListCommonPerf.default 689 694 0.99:1
SliderMinimalPerf.default 1633 1644 0.99:1
ToolbarMinimalPerf.default 1040 1048 0.99:1
TreeMinimalPerf.default 854 865 0.99:1
Dropdown.Fluent 3116 3143 0.99:1
CardMinimalPerf.default 616 627 0.98:1
ChatWithPopoverPerf.default 418 425 0.98:1
CheckboxMinimalPerf.default 2832 2880 0.98:1
DividerMinimalPerf.default 389 396 0.98:1
EmbedMinimalPerf.default 4265 4359 0.98:1
ProviderMergeThemesPerf.default 1662 1688 0.98:1
ProviderMinimalPerf.default 1037 1057 0.98:1
Icon.Fluent 651 663 0.98:1
InputMinimalPerf.default 1298 1343 0.97:1
Dialog.Fluent 794 822 0.97:1
AccordionMinimalPerf.default 169 176 0.96:1
ListNestedPerf.default 594 616 0.96:1
SkeletonMinimalPerf.default 402 417 0.96:1

@khmakoto khmakoto merged commit e3aca0d into microsoft:master Apr 15, 2021
@khmakoto khmakoto deleted the buttonFocus branch April 15, 2021 23:08
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react@v8.11.0 has been released which incorporates this pull request.:tada:

Handy links:

miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
… be supported (microsoft#17820)

* Button: Setting focus visibility to true in scenarios where it should be supported.

* Change files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No visible focus when button programmatically focused Focus indicator not visible after activating the "Contextual Menu button" using keyboard

4 participants