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

Jspurlin/combo box focus fix 7.0 #15784

Merged
merged 2 commits into from
Oct 30, 2020

Conversation

jspurlin
Copy link
Collaborator

Pull request checklist

  • Include a change request file using $ yarn change

Description of changes

Previous to this change there was a case when a controlled comboBox could move focus as part of either onItemClick or onChange prop callbacks, but then the comboBox would refocus the input afterwards leading to unexpected focus behavior.

This PR allows the comboBox to handle its internal focus handling and potential menu closing prior to calling the onItemClick and onChange callbacks. This allows the comboBox to deal with its inner focus handling before communicating outwardly fixing the potential unexpected stealing of focus back to the input

Focus areas to test

Verified that the comboBox functionality didn't change and the comboBox no longer steals focus back if it was moved as part of onItemClick or onChange

…before calling the passed in onItemClick callback or calling setSelectedItem
@msft-github-bot msft-github-bot added the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Oct 30, 2020

// Continue processing the click only after
// performing menu close / control focus(inner working)
onItemClick && onItemClick(ev, item, index);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only things changed as part of this PR is moving lines 1593 and 1594 to here (lines 1609 and 1610)

@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 f3a0214:

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

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
BaseButton mount 979 968 5000
Breadcrumb mount 42021 42262 5000
Checkbox mount 1667 1674 5000
CheckboxBase mount 1438 1394 5000
ChoiceGroup mount 5251 5393 5000
ComboBox mount 956 982 1000
CommandBar mount 7806 7897 1000
ContextualMenu mount 13735 13897 1000
DefaultButton mount 1177 1169 5000
DetailsRow mount 3779 3792 5000
DetailsRowFast mount 3784 3937 5000
DetailsRowNoStyles mount 3526 3669 5000
Dialog mount 1586 1558 1000
DocumentCardTitle mount 1836 1858 1000
Dropdown mount 2737 2741 5000
FocusTrapZone mount 1739 1779 5000
FocusZone mount 1871 1867 5000
IconButton mount 1893 1936 5000
Label mount 327 333 5000
Layer mount 2048 2033 5000
Link mount 471 466 5000
MenuButton mount 1578 1594 5000
MessageBar mount 2172 2133 5000
Nav mount 3473 3464 1000
OverflowSet mount 1490 1471 5000
Panel mount 1537 1536 1000
Persona mount 882 885 1000
Pivot mount 1498 1510 1000
PrimaryButton mount 1365 1357 5000
Rating mount 8174 8122 5000
SearchBox mount 1408 1430 5000
Shimmer mount 2761 2765 5000
Slider mount 1600 1603 5000
SpinButton mount 5278 5279 5000
Spinner mount 441 410 5000
SplitButton mount 3352 3405 5000
Stack mount 532 531 5000
StackWithIntrinsicChildren mount 1620 1711 5000
StackWithTextChildren mount 5184 5126 5000
SwatchColorPicker mount 10914 10891 5000
TagPicker mount 2896 2904 5000
TeachingBubble mount 50975 50692 5000
Text mount 467 487 5000
TextField mount 1488 1477 5000
Toggle mount 877 848 5000
button mount 112 107 5000

@size-auditor
Copy link

size-auditor bot commented Oct 30, 2020

Asset size changes

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

Baseline commit: 9c0596c67f5fad10823f6b626926f0444873bc8c (build)

@jspurlin jspurlin merged commit c77eec3 into microsoft:7.0 Oct 30, 2020
@jspurlin jspurlin mentioned this pull request Oct 30, 2020
1 task
@msft-github-bot
Copy link
Contributor

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

Handy links:

@ecraig12345 ecraig12345 removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Jan 26, 2021
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.

None yet

5 participants