Skip to content

Popup: Set tabindex to -1 to fix Esc on keydown event handler#7839

Merged
dzearing merged 3 commits intomicrosoft:masterfrom
jhellesen:u/jehell/popup-tabindex
Jan 30, 2019
Merged

Popup: Set tabindex to -1 to fix Esc on keydown event handler#7839
dzearing merged 3 commits intomicrosoft:masterfrom
jhellesen:u/jehell/popup-tabindex

Conversation

@jhellesen
Copy link
Contributor

@jhellesen jhellesen commented Jan 29, 2019

Description of changes

Popups - like Callout or Modal - have an issue where when you click inside of a Fabric popup (but not on an actual keyboard focusable item – just in the blank area of the popup) and then hit ESC, the popup does not close.

This is happening because clicking in the popup causes focus to go to the document itself which is outside of the popup so the appropriate keydown event handler on the popup is not hit.

This change adds a tabindex of -1 to the Popup component so that focus will go to the Popup itself on click, allowing the appropriate keydown event handler to be trigger.

Focus areas to test

All Popup components, especially as relates to keyboard focus, tab order and clicking.

Microsoft Reviewers: Open in CodeFlow

@size-auditor
Copy link

size-auditor bot commented Jan 29, 2019

Bundle test Size (minified) Diff from master
ActivityItem 164.456 kB ExceedsBaseline     12 bytes
Modal 111.635 kB ExceedsBaseline     12 bytes
Grid 224.179 kB ExceedsBaseline     12 bytes
Pivot 229.655 kB ExceedsBaseline     12 bytes
Pickers 322.191 kB ExceedsBaseline     12 bytes
PersonaCoin 158.208 kB ExceedsBaseline     12 bytes
Persona 158.136 kB ExceedsBaseline     12 bytes
Panel 234.328 kB ExceedsBaseline     12 bytes
Dialog 236.966 kB ExceedsBaseline     12 bytes
Nav 230.46 kB ExceedsBaseline     12 bytes
MessageBar 230.328 kB ExceedsBaseline     12 bytes
ContextualMenu 188.276 kB ExceedsBaseline     12 bytes
DocumentCard 273.867 kB ExceedsBaseline     12 bytes
Dropdown 264.089 kB ExceedsBaseline     12 bytes
ExtendedPicker 208.91 kB ExceedsBaseline     12 bytes
Tooltip 128.837 kB ExceedsBaseline     12 bytes
KeytipLayer 206.833 kB ExceedsBaseline     12 bytes
Facepile 257.413 kB ExceedsBaseline     12 bytes
Keytip 191.183 kB ExceedsBaseline     12 bytes
FloatingPicker 347.367 kB ExceedsBaseline     12 bytes
DatePicker 243.079 kB ExceedsBaseline     12 bytes
Popup 65.723 kB ExceedsBaseline     12 bytes
CommandBar 242.092 kB ExceedsBaseline     12 bytes
ComboBox 280.634 kB ExceedsBaseline     12 bytes
Breadcrumb 241.832 kB ExceedsBaseline     12 bytes
Button 220.779 kB ExceedsBaseline     12 bytes
TeachingBubble 231.969 kB ExceedsBaseline     12 bytes
SwatchColorPicker 239.587 kB ExceedsBaseline     12 bytes
Callout 122.558 kB ExceedsBaseline     12 bytes
SpinButton 234.389 kB ExceedsBaseline     12 bytes
HoverCard 133.655 kB ExceedsBaseline     12 bytes
SelectedItemsList 279.832 kB ExceedsBaseline     12 bytes
SearchBox 227.094 kB ExceedsBaseline     12 bytes

ExceedsTolerance  Exceeds Tolerance     ExceedsBaseline  Exceeds Baseline     BelowBaseline  Below Baseline     1 kB = 1000 bytes

@leddie24 leddie24 self-requested a review January 29, 2019 23:14
@leddie24 leddie24 changed the title U/jehell/popup tabindex Popup: Set tabindex to -1 to fix Esc on keydown event handler Jan 29, 2019
@dzearing dzearing merged commit ba23368 into microsoft:master Jan 30, 2019
@msft-github-bot
Copy link
Contributor

🎉office-ui-fabric-react@v6.131.1 has been released which incorporates this pull request.:tada:

Handy links:

@JasonGore
Copy link
Member

This issue will again occur with the merge of #8053. This issue will remain closed but is covered by #8055.

@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants