ComboBox: React handler events happen after native browser events

[katiearriagam]

Environment Information

• Package version(s): 7.0.0
• Browser and OS versions: Google Chrome

Please provide a reproduction of the bug in a codepen:

https://codepen.io/katiearriagam/pen/ZEzgQqo

1. Go to https://codepen.io/katiearriagam/pen/ZEzgQqo and open the console.
2. Set focus inside the ComboBox
3. Trigger a keyDown event.

Actual behavior:

Because of the way React handles events, the logs will show that onKeyDown gets handled first for the pure HTML elements (from innermost to outermost), and then on the React side, also from innermost to outermost. Therefore, the logs appear in the following order:

onKeyDown called for WRAPPER-2. Event phase: 3
onKeyDown called for WRAPPER-1. Event phase: 3
onKeyDown called for ComboBox. Event phase: 3
onKeyDown called for WRAPPER-3. Event phase: 3

While both are consistent in terms of handling the innermost component first, it is not following hierarchical pattern in the DOM, because it handles the DOM native events first and then the React synthetic events.

Expected behavior:

We need a hierarchical sequence of event handlers that works well with the DOM native events. Ideally, I think we would follow React's pattern and do everything on bubbling (from innermost to outermost). Therefore, I would expect the sequence of logs to look as follows:

onKeyDown called for ComboBox. Event phase: 3
onKeyDown called for WRAPPER-3. Event phase: 3
onKeyDown called for WRAPPER-2. Event phase: 3
onKeyDown called for WRAPPER-1. Event phase: 3

Notes

Adding @jspurlin so we can both follow up on this.

[jspurlin]

@joschect, @JasonGore , and @dzearing FYI, this is an interesting/frustrating issue that we found related to native eventhandlers firing vs react eventhandlers...

I think there's a bigger discussion that needs to happen for how we should solve this. You could imagine changing the eventhandlers to hook up to the native events (via the events object), but there's potential that some consumer may be relying on the react eventing flow which could break if we switched to using native events.

We're seeing this in office online because some of the outer element (that aren't using fabric controls) have handlers during the bubble phase and are getting hit before the handlers on the combobox during the bubble event are being hit resulting in incorrect behavior

[dzearing]

There is no action we can take here. This is how React synthetic events work. Working around it would break others who rely on React event timing.

Facebook devs have been thinking about the abstraction for some time:
facebook/react#4751

Also specifically mentioned in the React Fire tracking issue:
facebook/react#13525

We hit this as well in OneDrive, where knockout based event handlers would not play well with React based ones. For example, getting a knockout marquee selection scenario to work with react lists was challenging, because of the event timing.

There isn't a great solution here, other than to get rid of old code as fast as possible, or at least remove/rethink old event handlers dependent on global DOM eventing and do something different. That's at least what we did with the marquee scenario.

[jspurlin]

That's unfortunate, but understandable...

[dzearing]

Yeah, wish I had a better answer. One thought to consider/investigate is whether Preact could work. I have heard (but not validated) that Preact uses native eventing.

[dzearing]

https://codesandbox.io/s/preact-click-eventing-p3dv3

Yes, preact does native, so the click event for the button in this example happens the way you expect:

First the button onClick fires,
Then the wrapper div in the static html receives the event

(See console.)

[msft-github-bot]

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!