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] Add ContextMenu responder #16296

Merged
merged 1 commit into from
Aug 6, 2019

Conversation

necolas
Copy link
Contributor

@necolas necolas commented Aug 5, 2019

A module for responding to contextmenu events. This functionality will be
removed from the Press responder in the future.

@sizebot
Copy link

sizebot commented Aug 5, 2019

Details of bundled changes.

Comparing: 6b565ce...8077f45

react-events

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-events-scroll.development.js 0.0% +0.1% 5.94 KB 5.94 KB 1.59 KB 1.59 KB UMD_DEV
ReactEventsContextMenu-dev.js n/a n/a 0 B 2.48 KB 0 B 937 B FB_WWW_DEV
react-events-drag.production.min.js 0.0% 🔺+0.1% 3.12 KB 3.12 KB 1.45 KB 1.45 KB NODE_PROD
react-events-context-menu.development.js n/a n/a 0 B 2.69 KB 0 B 1003 B UMD_DEV
react-events-swipe.development.js 0.0% +0.1% 6.59 KB 6.59 KB 1.75 KB 1.75 KB UMD_DEV
react-events-context-menu.production.min.js n/a n/a 0 B 1.39 KB 0 B 727 B UMD_PROD
react-events-input.production.min.js 0.0% 🔺+0.1% 1.82 KB 1.82 KB 973 B 974 B UMD_PROD
react-events-swipe.production.min.js 0.0% 🔺+0.1% 2.63 KB 2.63 KB 1.19 KB 1.19 KB UMD_PROD
react-events-context-menu.development.js n/a n/a 0 B 2.5 KB 0 B 958 B NODE_DEV
react-events-input.development.js 0.0% +0.1% 4.35 KB 4.35 KB 1.39 KB 1.39 KB NODE_DEV
react-events-context-menu.production.min.js n/a n/a 0 B 1.2 KB 0 B 666 B NODE_PROD
react-events-swipe.production.min.js 0.0% 🔺+0.1% 2.45 KB 2.45 KB 1.14 KB 1.14 KB NODE_PROD
ReactEventsContextMenu-prod.js n/a n/a 0 B 2.2 KB 0 B 797 B FB_WWW_PROD
react-events-hover.production.min.js 0.0% -0.1% 2.86 KB 2.86 KB 1.15 KB 1.15 KB NODE_PROD
react-events-scroll.production.min.js 0.0% 🔺+0.1% 2.2 KB 2.2 KB 1.02 KB 1.02 KB NODE_PROD
react-events-focus.development.js 0.0% 0.0% 10.16 KB 10.16 KB 2.12 KB 2.12 KB UMD_DEV
react-events-press.development.js 0.0% -0.0% 21.69 KB 21.69 KB 5.05 KB 5.04 KB UMD_DEV

Generated by 🚫 dangerJS

@necolas necolas force-pushed the react-events/context-menu branch 2 times, most recently from e8c8aaf to aa3ed37 Compare August 5, 2019 22:52
@necolas
Copy link
Contributor Author

necolas commented Aug 5, 2019

Note: this responder uses a different approach to defining the native events it listens to, and surfaces an issue our existing responders.

The ContextMenu responder listens to either pointer events or fallback events. Doing this makes it easier to keep the PointerEvents and fallback implementations separate in more complicated responders, as the fallback events can be completely ignored. And it also makes it clear that the test suites needs to be run twice, with the correct environment simulated.

In contrast, the existing responders listen to pointer events by default, and if those aren't supported they add fallback events. In the unit test environment, pointer events aren't supported so the responder always adds the fallback events! However, in the unit tests we still dispatch native pointer events in almost all the scenarios. This creates an unrealistic environment, where a browser is dispatching pointer events but React is also listening to the fallback events! As such, the existing unit tests are both imposing greater complexity on the PointerEvents implementation that would occur in reality (i.e., listening to fallback events and having to deal with them) and potentially covering up bugs if the PointerEvents path accidentally ends up depending on state changes caused by the fallback events (because those events won't be listened to outside of the unit test env).

A module for responding to contextmenu events. This functionality will be
removed from the Press responder in the future.
@necolas necolas merged commit 23405c9 into facebook:master Aug 6, 2019
@engprodigy
Copy link

Hi @necolas is there any way I can test this experimental module in a react app? Thanks.

@necolas necolas deleted the react-events/context-menu branch December 20, 2019 15:36
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