Skip to content

Commit

Permalink
Merge pull request #2181 from zpao/move-onscroll-detection
Browse files Browse the repository at this point in the history
Move onscroll detection
  • Loading branch information
zpao committed Sep 11, 2014
2 parents 584e6bd + def41df commit af485d9
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 20 deletions.
11 changes: 11 additions & 0 deletions src/browser/ui/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ var ReactPerf = require('ReactPerf');

var escapeTextForBrowser = require('escapeTextForBrowser');
var invariant = require('invariant');
var isEventSupported = require('isEventSupported');
var keyOf = require('keyOf');
var merge = require('merge');
var mixInto = require('mixInto');
var monitorCodeUse = require('monitorCodeUse');

var deleteListener = ReactBrowserEventEmitter.deleteListener;
var listenTo = ReactBrowserEventEmitter.listenTo;
Expand Down Expand Up @@ -66,6 +68,15 @@ function assertValidProps(props) {
}

function putListener(id, registrationName, listener, transaction) {
if (__DEV__) {
// IE8 has no API for event capturing and the `onScroll` event doesn't
// bubble.
if (registrationName === 'onScroll' &&
!isEventSupported('scroll', true)) {
monitorCodeUse('react_no_scroll_event');
console.warn('This browser doesn\'t support the `onScroll` event');
}
}
var container = ReactMount.findReactContainerForID(id);
if (container) {
var doc = container.nodeType === ELEMENT_NODE_TYPE ?
Expand Down
22 changes: 22 additions & 0 deletions src/browser/ui/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,26 @@ describe('ReactDOMComponent', function() {
});
});

describe('onScroll warning', function() {
it('should warn about the `onScroll` issue when unsupported (IE8)', () => {
// Mock this here so we can mimic IE8 support. We require isEventSupported
// before React so it's pre-mocked before React qould require it.
require('mock-modules')
.dumpCache()
.mock('isEventSupported');
var isEventSupported = require('isEventSupported');
isEventSupported.mockReturnValueOnce(false);

var React = require('React');
var ReactTestUtils = require('ReactTestUtils');

spyOn(console, 'warn');
ReactTestUtils.renderIntoDocument(<div onScroll={function(){}} />);
expect(console.warn.callCount).toBe(1);
expect(console.warn.mostRecentCall.args[0]).toBe(
'This browser doesn\'t support the `onScroll` event'
);
});
});

});
11 changes: 0 additions & 11 deletions src/event/EventPluginHub.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ var EventPluginUtils = require('EventPluginUtils');
var accumulateInto = require('accumulateInto');
var forEachAccumulated = require('forEachAccumulated');
var invariant = require('invariant');
var isEventSupported = require('isEventSupported');
var monitorCodeUse = require('monitorCodeUse');

/**
* Internal store for event listeners
Expand Down Expand Up @@ -159,15 +157,6 @@ var EventPluginHub = {
registrationName, typeof listener
);

if (__DEV__) {
// IE8 has no API for event capturing and the `onScroll` event doesn't
// bubble.
if (registrationName === 'onScroll' &&
!isEventSupported('scroll', true)) {
monitorCodeUse('react_no_scroll_event');
console.warn('This browser doesn\'t support the `onScroll` event');
}
}
var bankForRegistrationName =
listenerBank[registrationName] || (listenerBank[registrationName] = {});
bankForRegistrationName[id] = listener;
Expand Down
9 changes: 0 additions & 9 deletions src/event/__tests__/EventPluginHub-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,6 @@ describe('EventPluginHub', function() {
isEventSupported.mockReturnValueOnce(false);
});

it('should warn about the `onScroll` issue on IE8', function() {
spyOn(console, 'warn');
EventPluginHub.putListener(1, 'onScroll', function(){});
expect(console.warn.callCount).toBe(1);
expect(console.warn.mostRecentCall.args[0]).toBe(
'This browser doesn\'t support the `onScroll` event'
);
});

it("should prevent non-function listeners", function() {
expect(function() {
EventPluginHub.putListener(1, 'onClick', 'not a function');
Expand Down

0 comments on commit af485d9

Please sign in to comment.