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

iOS 11.3 fix #416

Merged
merged 9 commits into from
Apr 3, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
1 change: 1 addition & 0 deletions docs/guides/how-we-use-dom-events.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# How we use DOM events

> This page details how we use DOM input events, what we do with them, and how you can build things on top of our usage. **Generally you will not need to know this information** but it can be helpful if you are also binding your own event handlers to the window or to a *drag handle*.
> ⚠️ Note: due to a [bug in webkit](https://bugs.webkit.org/show_bug.cgi?id=184250), particular events such as `mousemove` will not correctly set `event.defaultPrevented` to `true` when `event.preventDefault()` is called. You call follow this here: #413

## Prior knowledge

Expand Down
75 changes: 72 additions & 3 deletions src/view/drag-handle/sensor/create-touch-sensor.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,75 @@ type TouchWithForce = Touch & {
force: number
}

type SafariHack = {|
start: () => void,
stop: () => void,
|}

export const timeForLongPress: number = 150;
export const forcePressThreshold: number = 0.15;
const touchStartMarshal: EventMarshal = createEventMarshal();

const noop = (): void => { };

// Safari does not allow event.preventDefault() in dynamically added handlers
// So we add an always listening event handler to get around this :(
// webkit bug: https://bugs.webkit.org/show_bug.cgi?id=184250
const safariHack: SafariHack = (() => {
const shouldBlock: boolean = (() => {
// Do nothing when server side rendering
if (typeof window === 'undefined') {
return false;
}

const isUsingSafari11: RegExp = /AppleWebKit.*Version\/11/g;

// Not using Safari 11
if (!isUsingSafari11.test(window.navigator.userAgent)) {
return false;
}

// Using Safari 11 with no touch support - no point adding the touch listeners
if (!('ontouchstart' in window)) {
return false;
}

return true;
})();

if (!shouldBlock || typeof window === 'undefined') {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: The typeof window part of the conditional won't get hit since it's being checked within shouldBlock.

return { start: noop, stop: noop };
}

let isBlocking: boolean = false;
const start = () => {
isBlocking = true;
};
const stop = () => {
isBlocking = false;
};

// Adding a persistent event handler
window.addEventListener('touchmove', (event: TouchEvent) => {
if (!isBlocking) {
return;
}

// Our normal event handler should have done this
if (event.defaultPrevented) {
return;
}

// Okay, now we need to step in and fix things
event.preventDefault();

// Forcing this to be non-passive so we can get every touchmove
// Not activating in the capture phase like the dynamic touchmove we add.
// Technically it would not matter if we did this in the capture phase
}, { passive: false, capture: false });

return { start, stop };
})();

const initial: State = {
isDragging: false,
pending: null,
Expand Down Expand Up @@ -82,6 +145,7 @@ export default ({
const stopDragging = (fn?: Function = noop) => {
schedule.cancel();
touchStartMarshal.reset();
safariHack.stop();
unbindWindowEvents();
postDragEventPreventer.preventNext();
setState(initial);
Expand Down Expand Up @@ -113,6 +177,7 @@ export default ({
}
schedule.cancel();
touchStartMarshal.reset();
safariHack.stop();
unbindWindowEvents();

setState(initial);
Expand All @@ -138,7 +203,7 @@ export default ({
const windowBindings: EventBinding[] = [
{
eventName: 'touchmove',
// opting out of passive touchmove (default) so as to prevent scrolling while moving
// Opting out of passive touchmove (default) so as to prevent scrolling while moving
// Not worried about performance as effect of move is throttled in requestAnimationFrame
options: { passive: false },
fn: (event: TouchEvent) => {
Expand All @@ -163,7 +228,9 @@ export default ({
y: clientY,
};

// already dragging
// We need to prevent the default event in order to block native scrolling
// Also because we are using it as part of a drag we prevent the default action
// as a sign that we are using the event
event.preventDefault();
schedule.move(point);
},
Expand Down Expand Up @@ -311,8 +378,10 @@ export default ({
// We need to stop parents from responding to this event - which may cause a double lift
// We also need to NOT call event.preventDefault() so as to maintain as much standard
// browser interactions as possible.
// This includes navigation on anchors which we want to preserve
touchStartMarshal.handle();

safariHack.start();
startPendingDrag(event);
};

Expand Down