Skip to content

Commit

Permalink
Fix/ie 11 scroll listener (#646)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexreardon authored Jul 18, 2018
1 parent a4f9ac1 commit b276e47
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 18 deletions.
18 changes: 10 additions & 8 deletions src/view/drag-handle/util/bind-events.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
// @flow
import type { EventBinding, EventOptions } from './event-types';

const getOptions = (
shared?: EventOptions,
fromBinding: ?EventOptions,
): EventOptions => ({
...shared,
...fromBinding,
});

export const bindEvents = (
el: HTMLElement,
bindings: EventBinding[],
sharedOptions?: EventOptions,
) => {
bindings.forEach((binding: EventBinding) => {
const options: Object = {
...sharedOptions,
...binding.options,
};
const options: Object = getOptions(sharedOptions, binding.options);

el.addEventListener(binding.eventName, binding.fn, options);
});
Expand All @@ -22,10 +27,7 @@ export const unbindEvents = (
sharedOptions?: EventOptions,
) => {
bindings.forEach((binding: EventBinding) => {
const options: Object = {
...sharedOptions,
...binding.options,
};
const options: Object = getOptions(sharedOptions, binding.options);

el.removeEventListener(binding.eventName, binding.fn, options);
});
Expand Down
13 changes: 8 additions & 5 deletions src/view/drag-handle/util/focus-retainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ type FocusRetainer = {|
// our shared state
let retainingFocusFor: ?DraggableId = null;

// Using capture: true as focus events do not bubble
// Additionally doing this prevents us from intercepting the initial
// focus event as it does not bubble up to this listener
const listenerOptions = { capture: true };

// If we focus on
const clearRetentionOnFocusChange = (() => {
let isBound: boolean = false;
Expand All @@ -23,11 +28,9 @@ const clearRetentionOnFocusChange = (() => {
}

isBound = true;
// Using capture: true as focus events do not bubble
// Additionally doing this prevents us from intercepting the initial
// focus event as it does not bubble up to this listener

// eslint-disable-next-line no-use-before-define
window.addEventListener('focus', onWindowFocusChange, { capture: true });
window.addEventListener('focus', onWindowFocusChange, listenerOptions);
};

const unbind = () => {
Expand All @@ -37,7 +40,7 @@ const clearRetentionOnFocusChange = (() => {

isBound = false;
// eslint-disable-next-line no-use-before-define
window.removeEventListener('focus', onWindowFocusChange, { capture: true });
window.removeEventListener('focus', onWindowFocusChange, listenerOptions);
};

// focusin will fire after the focus event fires on the element
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @flow
import { Component, type Node } from 'react';
import React, { type Node } from 'react';
import PropTypes from 'prop-types';
import memoizeOne from 'memoize-one';
import invariant from 'tiny-invariant';
Expand Down Expand Up @@ -81,7 +81,13 @@ type WatchingScroll = {|
options: ScrollOptions,
|};

export default class DroppableDimensionPublisher extends Component<Props> {
const listenerOptions = {
passive: true,
};

export default class DroppableDimensionPublisher extends React.Component<
Props,
> {
/* eslint-disable react/sort-comp */
watchingScroll: ?WatchingScroll = null;
callbacks: DroppableCallbacks;
Expand Down Expand Up @@ -165,9 +171,11 @@ export default class DroppableDimensionPublisher extends Component<Props> {
closestScrollable,
};

closestScrollable.addEventListener('scroll', this.onClosestScroll, {
passive: true,
});
closestScrollable.addEventListener(
'scroll',
this.onClosestScroll,
listenerOptions,
);
};

unwatchScroll = () => {
Expand All @@ -184,6 +192,7 @@ export default class DroppableDimensionPublisher extends Component<Props> {
watching.closestScrollable.removeEventListener(
'scroll',
this.onClosestScroll,
listenerOptions,
);
this.watchingScroll = null;
};
Expand Down
44 changes: 44 additions & 0 deletions test/unit/view/droppable-dimension-publisher.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,50 @@ describe('DraggableDimensionPublisher', () => {
callbacks.unwatchScroll();
wrapper.unmount();
});

// if this is not the case then it will break in IE11
it('should add and remove events with the same event options', () => {
const marshal: DimensionMarshal = getMarshalStub();
const wrapper = mount(<ScrollableItem />, withDimensionMarshal(marshal));
const container: HTMLElement = wrapper.getDOMNode();
jest.spyOn(container, 'addEventListener');
jest.spyOn(container, 'removeEventListener');

// tell the droppable to watch for scrolling
const callbacks: DroppableCallbacks =
marshal.registerDroppable.mock.calls[0][1];

// watch scroll will only be called after the dimension is requested
callbacks.getDimensionAndWatchScroll(preset.windowScroll, scheduled);

// assertion
const expectedOptions = {
passive: true,
};
expect(container.addEventListener).toHaveBeenCalledWith(
'scroll',
expect.any(Function),
expectedOptions,
);
expect(container.removeEventListener).not.toHaveBeenCalled();
container.addEventListener.mockReset();

// unwatching scroll
callbacks.unwatchScroll();

// assertion
expect(container.removeEventListener).toHaveBeenCalledWith(
'scroll',
expect.any(Function),
expectedOptions,
);
expect(container.removeEventListener).toHaveBeenCalledTimes(1);
expect(container.addEventListener).not.toHaveBeenCalled();

// cleanup
container.addEventListener.mockRestore();
container.removeEventListener.mockRestore();
});
});

describe('forced scroll', () => {
Expand Down

0 comments on commit b276e47

Please sign in to comment.