Fluentui: iOS13, Split Buttons when tapped twice return focus back to the last focused input

[Lego6245]

Environment Information

• Package version(s): 7.103.0
• Browser and OS versions: iOS 13 Safari. Does not reproduce in iOS 12 or emulated browser scenarios.

Please provide a reproduction of the bug in a codepen:

https://codepen.io/saulnier/pen/eYNxorZ

1) Focus into the CED and type some text. Also reproduces with a regular <input>
2) Tap on a split button.
3) Tap on the same split button again.

Actual behavior:

The split button's menu dismisses, and focus remains outside the input.

Expected behavior:

The split button's menu dismisses, but focus returns to the input, showing the keyboard

Priorities and help requested:

Are you willing to submit a PR to fix? Yes, with guidance.

Requested priority: High

Products/sites affected: Outlook on the Web is impacted in our touch scenarios.

[aneeshack4]

@Lego6245 Thanks for reaching out to us. Pretty sure that standard expected behavior is that focus stays on the split button because you've tapped that (even if the menu dismisses). This is the behavior on the web - chrome - macos: If you keyboard and press enter once the splitbutton's in focus, it will open the menu and if you enter again, it will dismiss and the focus rect is still on the splitbutton itself as you can see here:

https://developer.microsoft.com/en-us/fluentui#/controls/web/button

[Lego6245]

I believe you're misunderstanding me.

The focus is not staying on the button when tapped twice on iOS 13 after having your focus in the text box.

It is returning to the text box immediately after tapping the split button again.

This is divergent from behaviors on browsers and on iOS 12, where when the button is tapped twice, focus does not return to the last focused input element.

Let me get a recording added...

[Lego6245]

This recording was made on an iPad 7 running iOS 13 through Browserstack, so it's running on real hardware.

[Lego6245]

@aneeshack4 Here is another recording of the behavior on an iPad Pro running iOS 12 (also via Browserstack). Notice how focus does not return to the last focused input element.

[aneeshack4]

@Lego6245 Ah I see - so I just tired this on my phone running the latest iOS. I could only repro if I didn't hit the "Done" button that Safari & Chrome provide on iOS:

I also repro'd this on another touch device - on my Surface Book so I think is a generic touch behavior problem. I looked into how BaseButton (which is what SplitButton is built on) is handling touch events, there's an _onTouchStart helper which calls _handleTouchAndPointerEvent - I don't see anything about focus here after the touch times out. @khmakoto any thoughts on this?

[khmakoto]

@aneeshack4 hmmm, we maybe need to add an imperative focus call on onTouchStart.

[Lego6245]

I'm seeing similar behavior on iOS13 with other buttons like Icon buttons, so it would line up with the expectation that it's something in BaseButton. The problem is most easily experienced with buttons that have menus, as a page scroll will dismiss the button's menu and tapping it again to try to open the menu resumes focus to the input (which may trigger another scroll and dismiss the menu again)

[Lego6245]

I was able to also reproduce this issue on my surface using touch. I have an even more stable reproduction where closing the keyboard using the 'x' button and then tapping on a split button returns focus back to the last focused input and shows the keyboard again.

We're getting a few reports a day, so I'd like to see this issue root caused so that a fix can be made, whether that's by me or someone closer to the code's day to day.

[Lego6245]

I did some digging based on @khmakoto's suggestion of adding an imperative focus call to _handleTouchAndPointerEvent. From my analysis, clicking with a mouse focuses the button immediately, whereas the pointer events don't focus their targets as part of the event handling flow. Adding a focus call seems to fix the problem and prevents the behavior as described. I'm going to open a PR for this fix so we can get this in ASAP.

[ecraig12345]

Reopening because the fix for this caused an infinite loop in tests and had to be reverted to unblock other builds. @Lego6245 and/or @khmakoto you'll need to investigate the failure and resubmit the change with a fix.

[Lego6245]

I looked at the test that's failing, Nothing in it seems to indicate on a surface level that it should cause an infinite loop, but I don't know enough about ReactTestUtils to understand what a focus call during execution would do to it. Considering the change's scope, and the fact we were getting green builds at least some of the time, it does point to a race condition. Any ideas @khmakoto?

[khmakoto]

I'll take a look later today, it is definitely weird that we're not seeing consistent failures in the tests.

[ecraig12345]

For me locally and as far as I could tell in builds after the PR merged, it was failing consistently.

[Lego6245]

Which is weird, because we were able to get a green build, at least as part of the PR flow.

[Lego6245]

Any insights from your investigation @khmakoto? Any way I can assist?

[aneeshack4]

Looks like @khmakoto and @Lego6245 are following up offline about this. Looking into why it's breaking tests...

[msft-github-bot]

:tada:This issue was addressed in #13361, which has now been successfully released as [email protected].:tada:

Handy links:

• Release Notes
• Official NPM Release
• Test the release with Developer Demo Site