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

passivefalseeverywhere #3690

Merged
merged 4 commits into from Feb 14, 2017
Merged

passivefalseeverywhere #3690

merged 4 commits into from Feb 14, 2017

Conversation

asturur
Copy link
Member

@asturur asturur commented Feb 13, 2017

close #3687

Add { passive: false } to every touch event.
Add relevant code to bring down the option object to the actual addEventListener function
Add check to make sure old browser do not get the option object ( that would equal to useCapture true )

@@ -228,8 +228,8 @@
removeListener(fabric.document, 'mousemove', this._onMouseMove);
removeListener(fabric.document, 'touchmove', this._onMouseMove);

addListener(this.upperCanvasEl, 'mousemove', this._onMouseMove);
addListener(this.upperCanvasEl, 'touchmove', this._onMouseMove);
addListener(this.upperCanvasEl, 'mousemove', this._onMouseMove, { passive: false });

Choose a reason for hiding this comment

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

Only needed on touch events, so this one is redundant :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

right!
copy paste speed error

@asturur asturur merged commit 7ff1063 into master Feb 14, 2017
@asturur asturur deleted the passivefalse branch February 17, 2017 01:39
@samuelkilada
Copy link

samuelkilada commented Feb 27, 2017

Are we sure this is fixed? Chrome seems to be scrolling the window now when you use touch and drag. I confirmed this only started happening after updating to the latest version of Chrome on an Android phone. I just tested with fabricJS 1.7.6 to see if it would fix it (I think I was running 1.5 before) and it still doesn't work.

@asturur
Copy link
Member Author

asturur commented Feb 27, 2017

I have nothing scrollable, so i cannot really confirm. i fixed reading the specs from the mdn/ and chrome website.

i ll get a scrollable device soon

@samuelkilada
Copy link

samuelkilada commented Feb 27, 2017

Thank you. FYI, if you open this demo on an Android phone and zoom in a bit until the window can scroll up and down, you can see the error when you try moving the object: http://fabricjs.com/touch-events

For some reason, this doesn't happen in the kitchen sink demo! I'd love to know why.

@asturur
Copy link
Member Author

asturur commented Feb 28, 2017 via email

@samuelkilada
Copy link

Interesting! Kitchen sink uses http://fabricjs.com/lib/fabric.js, which is currently 1.7.6 and uses all modules except gestures and a couple others!

/* build: node build.js modules=ALL exclude=json,gestures minifier=uglifyjs */

The broken touch events demo uses http://fabricjs.com/lib/fabric_with_gestures.js, but yes it is 1.5.0, which explains it. I wonder if I should not use the gestures module since in the kitchen sink dmeo it seems to avoid the error without that module and works fine on touch. :S

@asturur
Copy link
Member Author

asturur commented Feb 28, 2017 via email

@samuelkilada
Copy link

Well, I must say I just made a really stupid mistake. In thinking that I was applying the updated version of Fabric I was actually not. 1.7.6 does indeed fix the problem Chrome introduced. I think the touch events demo running on 1.5.0 is what threw me off. Thanks very much for the quick responses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chrome update 55 in Android - fabric js zoom breaking
3 participants