Skip to content

Update focus handling in DatePicker and FocusTrapZone#8875

Merged
msft-github-bot merged 2 commits intomicrosoft:masterfrom
phtucker:phtucker/focus_fix
May 2, 2019
Merged

Update focus handling in DatePicker and FocusTrapZone#8875
msft-github-bot merged 2 commits intomicrosoft:masterfrom
phtucker:phtucker/focus_fix

Conversation

@phtucker
Copy link
Copy Markdown
Contributor

Pull request checklist

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

Description of changes

Issues encountered when DatePicker flyout is expanded and another element on the page requires focus ie: a modal alert dialog. In these instances the current focus handling will prevent another element from focusing and will return focus to the DatePicker TextField whenever the flyout is collapsed regardless of where the current focus is on the page ie: an entirely different element.

To support the above scenario DatePicker flyout should not aggressively keep focus and therefore should set the forceFocusInsideTrap to false of its FocusTrapZone. In addition, FocusTrapZone should not handle return focus if forceFocusInsideTrap is false and the currently focused element is not within the FocusTrapZone.

DatePicker should also allow FocusTrapZone to handle return focus exclusively. Removing out-of-date custom IE code wherein DatePicker interferes with return focus logic that should be the responsibility of FocusTrapZone.

Focus areas to test

FocusTrapZone
DatePicker

@phtucker phtucker requested review from a team and JasonGore as code owners April 29, 2019 19:50
@msft-github-bot
Copy link
Copy Markdown
Contributor

msft-github-bot commented Apr 29, 2019

Component perf results:

Scenario Target branch avg total (ms) PR avg total (ms) Target branch avg per item (ms) PR avg per item (ms)
PrimaryButton 108.473 100.975 1.085 1.010
BaseButton 37.400 36.504 0.374 0.365
NewButton 122.365 125.450 1.224 1.255
button 5.975 5.162 0.060 0.052
DetailsRows without styles 232.875 195.662 2.329 1.956
DetailsRows 219.822 218.070 2.198 2.181
Toggles 53.410 52.305 0.534 0.523
NewToggle 71.088 71.519 0.711 0.715

@size-auditor
Copy link
Copy Markdown

size-auditor bot commented Apr 29, 2019

Bundle test Size (minified) Diff from master
Panel 184.191 kB ExceedsBaseline     91 bytes
Dialog 186.429 kB ExceedsBaseline     90 bytes
Dropdown 215.054 kB ExceedsBaseline     90 bytes
Modal 79.353 kB ExceedsBaseline     80 bytes
Callout 83.5 kB ExceedsBaseline     80 bytes
HoverCard 99.156 kB ExceedsBaseline     80 bytes
FocusTrapZone 17.741 kB ExceedsBaseline     80 bytes
Coachmark 97.656 kB ExceedsBaseline     80 bytes
DatePicker 203.742 kB ExceedsBaseline     11 bytes

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

@JasonGore JasonGore closed this Apr 30, 2019
@JasonGore JasonGore reopened this Apr 30, 2019
@msft-github-bot
Copy link
Copy Markdown
Contributor

Hello @KevinTCoughlin!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me and give me an instruction to get started! Learn more here.

@msft-github-bot msft-github-bot merged commit b4d5e70 into microsoft:master May 2, 2019
@msft-github-bot
Copy link
Copy Markdown
Contributor

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

Handy links:

@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.

5 participants