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.

Sign up for GitHub

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

Move Mousewheel Event Type Detection to BrowserConstants (#11585) #11594

Merged
merged 2 commits into from
Nov 20, 2017
Merged

Move Mousewheel Event Type Detection to BrowserConstants (#11585) #11594

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2006880
Move Mousewheel Event Type Detection to BrowserConstants (#11585)
cristidrg Nov 19, 2017
2a0ed89
Drop check for wheel event type
cristidrg Nov 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/react-dom/src/events/BrowserEventConstants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import getVendorPrefixedEventName from './getVendorPrefixedEventName';
import isEventSupported from './isEventSupported';

/**
* Types of raw signals from the browser caught at the top level.
Expand Down Expand Up @@ -85,7 +86,9 @@ var topLevelTypes = {
getVendorPrefixedEventName('transitionend') || 'transitionend',
topVolumeChange: 'volumechange',
topWaiting: 'waiting',
topWheel: 'wheel',
topWheel: isEventSupported('wheel')
? 'wheel'
: isEventSupported('mousewheel') ? 'mousewheel' : 'DOMMouseScroll',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Did some quick checks. We have DOMMouseScroll for Firefox, but MDN claims that wheel is supported as of Firefox 17.0. We support the Firefox Extended Support Release (ESR), which is presently at 52.5.0. I think this is a safe change that will not affect any user.

Also it also looks like IE9 supports wheel (though it's not on the window according to MDN, the event is still supported).

So we can drop this check all-together. That would mean we'd revert the changes to this file and just keep the code removal in ReactBrowserEventEmitter.

@aweary, @jquense, or @gaearon any concerns with sending this out in a patch release?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't mind if your test it in the minimal supported browsers.


export type TopLevelTypes = $Enum<typeof topLevelTypes>;
Expand Down
12 changes: 1 addition & 11 deletions packages/react-dom/src/events/ReactBrowserEventEmitter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,7 @@ export function listenTo(registrationName, contentDocumentHandle) {
for (var i = 0; i < dependencies.length; i++) {
var dependency = dependencies[i];
if (!(isListening.hasOwnProperty(dependency) && isListening[dependency])) {
if (dependency === 'topWheel') {
if (isEventSupported('wheel')) {
trapBubbledEvent('topWheel', 'wheel', mountAt);
} else if (isEventSupported('mousewheel')) {
trapBubbledEvent('topWheel', 'mousewheel', mountAt);
} else {
// Firefox needs to capture a different mouse scroll event.
// @see http://www.quirksmode.org/dom/events/tests/scroll.html
trapBubbledEvent('topWheel', 'DOMMouseScroll', mountAt);
}
} else if (dependency === 'topScroll') {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

if (dependency === 'topScroll') {
trapCapturedEvent('topScroll', 'scroll', mountAt);
} else if (dependency === 'topFocus' || dependency === 'topBlur') {
trapCapturedEvent('topFocus', 'focus', mountAt);
Expand Down