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

[react-events] Keyboard support for virtual clicks #16780

Merged

Conversation

necolas
Copy link
Contributor

@necolas necolas commented Sep 13, 2019

This accounts for all clicks that are natively dispatched following relevant
keyboard interactions (e.g., key is "Enter"), as well as programmatic clicks,
and screen-reader virtual clicks.

(Alternative to #16755)

@necolas
Copy link
Contributor Author

necolas commented Sep 13, 2019

I think this is probably the better place for supporting these kinds of click events, as keyboard interactions also produce "virtual" clicks and you could argue the screen-reader created clicks are also "keyboard" triggered. Since most of the data in the click event is empty (just 0), it fits better with the keyboard event type than the pointer event type and doesn't need to produce a complete "tap" sequence to preserve API expectations of Tap.

@sizebot
Copy link

sizebot commented Sep 13, 2019

Details of bundled changes.

Comparing: 87eaa90...543533b

react-events

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-events-focus.development.js 0.0% +0.1% 10.91 KB 10.91 KB 2.35 KB 2.36 KB UMD_DEV
react-events-press.development.js 0.0% +0.1% 20.69 KB 20.69 KB 4.82 KB 4.82 KB UMD_DEV
react-events-focus.production.min.js 0.0% 🔺+0.2% 4.09 KB 4.09 KB 1.39 KB 1.39 KB UMD_PROD
react-events-press.production.min.js 0.0% 🔺+0.1% 6.96 KB 6.96 KB 2.61 KB 2.62 KB UMD_PROD
react-events-context-menu.development.js 0.0% +0.3% 2.67 KB 2.67 KB 998 B 1001 B UMD_DEV
react-events-input.development.js 0.0% +0.3% 4.51 KB 4.51 KB 1.44 KB 1.44 KB UMD_DEV
react-events-swipe.development.js 0.0% +0.1% 5.99 KB 5.99 KB 1.62 KB 1.62 KB UMD_DEV
react-events-context-menu.production.min.js 0.0% 🔺+0.4% 1.38 KB 1.38 KB 723 B 726 B UMD_PROD
react-events-input.production.min.js 0.0% 🔺+0.3% 1.83 KB 1.83 KB 976 B 979 B UMD_PROD
ReactEventsKeyboard-dev.js +17.7% +14.9% 5.93 KB 6.97 KB 2.07 KB 2.38 KB FB_WWW_DEV
react-events-swipe.production.min.js 0.0% 🔺+0.2% 2.43 KB 2.43 KB 1.1 KB 1.1 KB UMD_PROD
ReactEventsTap-dev.js +1.1% +3.1% 15.92 KB 16.1 KB 3.48 KB 3.59 KB FB_WWW_DEV
ReactEventsKeyboard-prod.js 🔺+15.6% 🔺+12.1% 4.69 KB 5.42 KB 1.55 KB 1.74 KB FB_WWW_PROD
react-events-context-menu.development.js 0.0% +0.3% 2.48 KB 2.48 KB 952 B 955 B NODE_DEV
react-events-input.development.js 0.0% +0.3% 4.33 KB 4.33 KB 1.39 KB 1.39 KB NODE_DEV
react-events-swipe.development.js 0.0% +0.1% 5.81 KB 5.81 KB 1.58 KB 1.58 KB NODE_DEV
react-events-context-menu.production.min.js 0.0% 🔺+0.3% 1.19 KB 1.19 KB 663 B 665 B NODE_PROD
react-events-input.production.min.js 0.0% 🔺+0.3% 1.65 KB 1.65 KB 909 B 912 B NODE_PROD
react-events-swipe.production.min.js 0.0% 🔺+0.2% 2.25 KB 2.25 KB 1.04 KB 1.04 KB NODE_PROD
react-events-hover.development.js 0.0% +0.2% 6.99 KB 6.99 KB 1.55 KB 1.55 KB UMD_DEV
react-events-scroll.development.js 0.0% +0.1% 6.29 KB 6.29 KB 1.65 KB 1.65 KB UMD_DEV
react-events-hover.production.min.js 0.0% 🔺+0.2% 3.11 KB 3.11 KB 1.13 KB 1.13 KB UMD_PROD
react-events-scroll.production.min.js 0.0% 🔺+0.3% 2.61 KB 2.61 KB 1.14 KB 1.14 KB UMD_PROD
react-events-hover.development.js 0.0% +0.1% 6.81 KB 6.81 KB 1.5 KB 1.5 KB NODE_DEV
react-events-scroll.development.js 0.0% +0.2% 6.11 KB 6.11 KB 1.6 KB 1.6 KB NODE_DEV
react-events-hover.production.min.js 0.0% 🔺+0.3% 2.93 KB 2.93 KB 1.07 KB 1.07 KB NODE_PROD
react-events-scroll.production.min.js 0.0% 🔺+0.3% 2.43 KB 2.43 KB 1.08 KB 1.09 KB NODE_PROD
react-events-focus.development.js 0.0% +0.1% 10.73 KB 10.73 KB 2.31 KB 2.31 KB NODE_DEV
react-events-press.development.js 0.0% 0.0% 20.51 KB 20.51 KB 4.77 KB 4.77 KB NODE_DEV
react-events-focus.production.min.js 0.0% 🔺+0.3% 3.92 KB 3.92 KB 1.32 KB 1.32 KB NODE_PROD
react-events-press.production.min.js 0.0% 🔺+0.1% 6.78 KB 6.78 KB 2.56 KB 2.56 KB NODE_PROD
react-events-drag.development.js 0.0% +0.2% 5.21 KB 5.21 KB 1.53 KB 1.54 KB UMD_DEV
react-events-keyboard.development.js +15.6% +13.8% 5.95 KB 6.88 KB 2.09 KB 2.38 KB UMD_DEV
react-events-tap.development.js +1.1% +3.1% 16.24 KB 16.42 KB 3.52 KB 3.63 KB UMD_DEV
react-events-drag.production.min.js 0.0% 🔺+0.3% 2.24 KB 2.24 KB 1.07 KB 1.07 KB UMD_PROD
react-events-keyboard.production.min.js 🔺+11.3% 🔺+9.1% 2.5 KB 2.79 KB 1.28 KB 1.39 KB UMD_PROD
react-events-tap.production.min.js 0.0% 🔺+0.1% 5.77 KB 5.77 KB 2.22 KB 2.22 KB UMD_PROD
react-events-drag.development.js 0.0% 0.0% 6.97 KB 6.97 KB 2.2 KB 2.2 KB NODE_DEV
react-events-keyboard.development.js +16.1% +14.2% 5.76 KB 6.69 KB 2.04 KB 2.33 KB NODE_DEV
react-events-tap.development.js +1.1% +3.1% 16.06 KB 16.24 KB 3.47 KB 3.58 KB NODE_DEV
react-events-drag.production.min.js 0.0% 🔺+0.1% 2.87 KB 2.87 KB 1.37 KB 1.37 KB NODE_PROD
react-events-keyboard.production.min.js 🔺+12.4% 🔺+10.9% 2.32 KB 2.61 KB 1.21 KB 1.34 KB NODE_PROD
react-events-tap.production.min.js 0.0% 🔺+0.1% 5.67 KB 5.67 KB 2.2 KB 2.21 KB NODE_PROD

Generated by 🚫 dangerJS against 543533b

This accounts for all clicks that are natively dispatched following relevant
keyboard interactions (e.g., key is "Enter"), as well as programmatic clicks,
and screen-reader virtual clicks.
@necolas necolas force-pushed the react-events/keyboard-virtual-click branch from 9e43f5a to 543533b Compare September 13, 2019 21:59
@necolas necolas merged commit 9691eb2 into facebook:master Sep 16, 2019
nativeEvent.clientX === 0 &&
nativeEvent.clientY === 0
);
}
Copy link

@craigkovatch craigkovatch Sep 25, 2019

Choose a reason for hiding this comment

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

(Not sure what the etiquette on a merged PR is -- please let me know if these comments would be more appropriate in e.g. a new Issue. cc @necolas @trueadm )

I don't believe this works cross-browser.

  1. in IE(11), detail is always 0, even for mouse-initiated click events. Can check for !nativeEvent.pointerType to work around this -- all click events in IE11 are PointerEvent, but only real pointer events have pointerType.

  2. in Safari, the screen and client coordinates are present even for a keyboard-initiated click. The client coordinates seem to default to the "center" of the element.

What I've used internally at Tableau for this that works cross-browser is return (nativeEvent.detail === 0 && !nativeEvent.pointerType);

Copy link
Contributor

Choose a reason for hiding this comment

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

This is really awesome feedback. Thank you cc @necolas seems like a valid thing we should be doing too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does (nativeEvent.detail === 0 && !nativeEvent.pointerType) work if that evaluates as true for mouse events as well as virtual clicks?

Copy link

@craigkovatch craigkovatch Sep 25, 2019

Choose a reason for hiding this comment

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

(nativeEvent.detail === 0 && !nativeEvent.pointerType) evaluates to true for "keyboard" clicks, not for mouse events.

My investigation showed that nativeEvent.detail === 0 is sufficient for all evergreen browsers; adding !nativeEvent.pointerEvent covers the IE11 case. Is that more clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still don't follow. The click event is of type MouseEvent which has no pointerType property. Are you saying IE11's click event is of type PointerEvent?

Copy link

@craigkovatch craigkovatch Sep 26, 2019

Choose a reason for hiding this comment

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

Are you saying IE11's click event is of type PointerEvent?

Yes, exactly. !nativeEvent.pointerType will be true for all other browsers in all scenarios, and true for IE11 when it's a keyboard-initiated click.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for confirming; we didn't know about that quirk. We'll be sure to incorporate your suggested fix. I like what Safari does here. I'm also curious to hear what you use this check for at Tableau

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

I'm also curious to hear what you use this check for at Tableau

It's a long, annoying story -- the short version is there an event normalizer (parallel to React) sitting on top of touch events that interferes with native keyboard click events on button elements. I needed this to differentiate the event sources so it could ignore those from the keyboard.

Copy link

@craigkovatch craigkovatch Nov 7, 2019

Choose a reason for hiding this comment

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

@necolas little bit of unfortunate news -- it appears detail === 0 also for click events which are dispatched to an <input> when clicked on a wrapping <label>, even when clicking with a mouse. Not sure if this is a problem for your purposes, but wanted to give you a heads-up.

@necolas necolas deleted the react-events/keyboard-virtual-click branch December 20, 2019 15:34
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.

5 participants