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

allowTouchScrolling doesn't work #5903

Closed
0ro opened this issue Oct 1, 2019 · 44 comments · Fixed by #10078
Closed

allowTouchScrolling doesn't work #5903

0ro opened this issue Oct 1, 2019 · 44 comments · Fixed by #10078
Labels

Comments

@0ro
Copy link

0ro commented Oct 1, 2019

Version

3.4.0

Test Case

https://codepen.io/den-bogdanov/pen/eYOwbPj

Information about the environment

mobile browser chrome, safari

Steps to reproduce

Open test case and try to scroll with help device emulation in devtools or from a mobile device

Expected Behavior

Must be scrolling

Actual Behavior

No touch scrolling

@Rumthorp
Copy link

Rumthorp commented Dec 6, 2019

Is anyone able to merge this?

@asturur
Copy link
Member

asturur commented Dec 9, 2019

I got stuck in understanding if there is something fundamentally wrong in my last touch events pr.

@Lyqxyz
Copy link

Lyqxyz commented Dec 16, 2019

How to solve this problem @asturur

@0ro
Copy link
Author

0ro commented Dec 16, 2019

@asturur is there any problems with my pr? I think you've just forgotten to add a condition with allowTouchScrolling because we have this same line in _onMouseMove event here

@Lyqxyz
Copy link

Lyqxyz commented Dec 16, 2019

"we have this same line in _onMouseMove event here?" What does this sentence mean @0ro

@asturur
Copy link
Member

asturur commented Dec 16, 2019

@Oro your pr is probably good and i did not forget about it.
I'm currently busy getting in the PR about upperCanvas retina scaling and the sub targets mouse in/out event, and that is taking all my free time.

@0ro
Copy link
Author

0ro commented Dec 17, 2019

"we have this same line in _onMouseMove event here?" What does this sentence mean @0ro

I meant we have the same behavior in _onMouseMove method and in onTouchStart we don't. I added this behavior in my pr.

@asturur
Copy link
Member

asturur commented Jan 6, 2020

I will look into this as soon as i m having back my touch device.
The point of preventing default there was to avoid mousedown and mouseup to fire because of touchstart and touchend.

not firing preventDefault would mean add back the removal of mousedown listener when touchstart fires. Stuff i was happy to have removed.

Seems silly to me that is possible to stop the mouse events fire but just at the cost of preventing everything.

@stale
Copy link

stale bot commented Jan 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue marked as stale by the stale bot label Jan 25, 2020
@stale stale bot closed this as completed Feb 1, 2020
@flexeo
Copy link

flexeo commented Feb 10, 2020

Same problem here, scrolling doesn't work on mobile devices with allowTouchScrolling = true.

@asturur asturur added bug and removed stale Issue marked as stale by the stale bot labels Feb 15, 2020
@asturur asturur reopened this Feb 15, 2020
@asturur
Copy link
Member

asturur commented Feb 15, 2020

We have a PR tracking this

@linewei
Copy link

linewei commented Mar 2, 2020

Same problem.

@CarlosSamp
Copy link

Same Problem here

@CarlosSamp
Copy link

CarlosSamp commented Mar 27, 2020

could fix it adding this to the Script

// first canvas element jQuery('#canvasshow').css('pointer-events', 'none'); jQuery('#canvasshow').css('touch-action', 'manipulation'); // second canvas element jQuery('.upper-canvas').css('pointer-events', 'none');

@suman
Copy link

suman commented Apr 9, 2020

Same problem in version 3.6.2, does anybody has any solution for this ?

@0ro
Copy link
Author

0ro commented Apr 9, 2020

Same problem in version 3.6.2, does anybody has any solution for this ?

you can see my solution in pull request above #5904

@A-ZC-Lau
Copy link

@0ro does that mean I need to go into the code and update it myself? I'm using the 3.6.3 npm package.

@lynmije
Copy link

lynmije commented Jun 6, 2020

Same problem in version 3.6.2, does anybody has any solution for this ?

you can see my solution in pull request above #5904

I tried your solution but its still not working.

@de-don
Copy link

de-don commented Jun 11, 2020

The same problem

RasyadiAbdoellah pushed a commit to RasyadiAbdoellah/fabric.js that referenced this issue Aug 14, 2020
uses fix outlined in fabricjs#5903 to address touch issues
@Abd3lwahab
Copy link

Same Problem here, Is there any fix ??

@yangjiang3973
Copy link

Is there any hack solution while waiting for this issue being solved? thanks!

@alexey-altteam
Copy link

alexey-altteam commented Sep 16, 2020

Found the cause of this bug. First line (e.preventDefault();) in the _onTouchStart function prevents the event propagation. FabricJS version 4.1.0

@javinladish
Copy link

Has anyone found a fix to allow mobile scrolling to work? I've tried a bunch of workarounds but couldn't get any to work.

@alexey-altteam
Copy link

Has anyone found a fix to allow mobile scrolling to work? I've tried a bunch of workarounds but couldn't get any to work.

Try this code in your project. It works with fabric 4.1.0

        (function() {
            var addListener = fabric.util.addListener,
                removeListener = fabric.util.removeListener,
                addEventOptions = { passive: false };

            fabric.util.object.extend(fabric.Canvas.prototype, /** @lends fabric.Canvas.prototype */ {
                _onTouchStart: function(e) {
                    //e.preventDefault();
                    if (this.mainTouchId === null) {
                        this.mainTouchId = this.getPointerId(e);
                    }
                    this.__onMouseDown(e);
                    this._resetTransformEventData();
                    var canvasElement = this.upperCanvasEl,
                    eventTypePrefix = this._getEventPrefix();
                    addListener(fabric.document, 'touchend', this._onTouchEnd, addEventOptions);
                    addListener(fabric.document, 'touchmove', this._onMouseMove, addEventOptions);
                    // Unbind mousedown to prevent double triggers from touch devices
                    removeListener(canvasElement, eventTypePrefix + 'down', this._onMouseDown);
                }
            });
        })();

Also set touch-action for canvases:

$('canvas').css('touch-action', 'manipulation');

@wnbittle
Copy link

Thanks @AlexShmalex for your solution, very helpful. My solution was similar - I wanted to allow object selection/move and drawing - I think for a general solution there could be more considerations.

(function(){
  var defaultOnTouchStartHandler = fabric.Canvas.prototype._onTouchStart;
  fabric.util.object.extend(fabric.Canvas.prototype, { 
    _onTouchStart: function(e) { 
      var target = this.findTarget(e); 
      // if allowTouchScrolling is enabled, no object was at the
      // the touch position and we're not in drawing mode, then 
      // let the event skip the fabricjs canvas and do default
      // behavior
      if (this.allowTouchScrolling && !target && !this.isDrawingMode) { 
        // returning here should allow the event to propagate and be handled
        // normally by the browser
        return; 
      } 

      // otherwise call the default behavior
      defaultOnTouchStartHandler.call(this, e); 
    } 
  });
})();

@javinladish
Copy link

javinladish commented Sep 25, 2020

@AlexShmalex I wasn't able to get your code above to work. My create canvas function looks like:

function createCanvas(w, h) {
          canvasContainerEl = document.getElementById("canvas-container");
          canvas = new fabric.Canvas("meme-canvas",{
	      allowTouchScrolling: true   
          });
          canvas.selection = false;
          canvas.setBackgroundColor("white", canvas.renderAll.bind(canvas));

And then I put your code into a <script> at the bottom of the body. It had no effect so I probably did something incorrect. I'm not too well versed in Javascript.

@Hoody123
Copy link

Thanks @AlexShmalex for your solution, very helpful. My solution was similar - I wanted to allow object selection/move and drawing - I think for a general solution there could be more considerations.

(function(){
  var defaultOnTouchStartHandler = fabric.Canvas.prototype._onTouchStart;
  fabric.util.object.extend(fabric.Canvas.prototype, { 
    _onTouchStart: function(e) { 
      var target = this.findTarget(e); 
      // if allowTouchScrolling is enabled, no object was at the
      // the touch position and we're not in drawing mode, then 
      // let the event skip the fabricjs canvas and do default
      // behavior
      if (this.allowTouchScrolling && !target && !this.isDrawingMode) { 
        // returning here should allow the event to propagate and be handled
        // normally by the browser
        return; 
      } 

      // otherwise call the default behavior
      defaultOnTouchStartHandler.call(this, e); 
    } 
  });
})();

It works well, thank you

@javinladish
Copy link

Has anyone found a fix to allow mobile scrolling to work? I've tried a bunch of workarounds but couldn't get any to work.

Try this code in your project. It works with fabric 4.1.0

        (function() {
            var addListener = fabric.util.addListener,
                removeListener = fabric.util.removeListener,
                addEventOptions = { passive: false };

            fabric.util.object.extend(fabric.Canvas.prototype, /** @lends fabric.Canvas.prototype */ {
                _onTouchStart: function(e) {
                    //e.preventDefault();
                    if (this.mainTouchId === null) {
                        this.mainTouchId = this.getPointerId(e);
                    }
                    this.__onMouseDown(e);
                    this._resetTransformEventData();
                    var canvasElement = this.upperCanvasEl,
                    eventTypePrefix = this._getEventPrefix();
                    addListener(fabric.document, 'touchend', this._onTouchEnd, addEventOptions);
                    addListener(fabric.document, 'touchmove', this._onMouseMove, addEventOptions);
                    // Unbind mousedown to prevent double triggers from touch devices
                    removeListener(canvasElement, eventTypePrefix + 'down', this._onMouseDown);
                }
            });
        })();

Also set touch-action for canvases:

$('canvas').css('touch-action', 'manipulation');

I got this code to work, but if I select and try to move an object the canvas is still scrolling.

How can I stop the scrolling behavior if an object is selected?

@alexey-altteam
Copy link

How can I stop the scrolling behavior if an object is selected?

Try this:

    var tmf = function(e) {
        e.preventDefault();
    };
    canvas.on('mouse:down', function(e) {
        if (canvas.getActiveObject()) {
            $('*').bind('touchmove', tmf);
        }
    });
    canvas.on('mouse:up', function(e) {
        $('*').unbind('touchmove', tmf);
    });

@javinladish
Copy link

javinladish commented Sep 30, 2020

How can I stop the scrolling behavior if an object is selected?

Try this:

    var tmf = function(e) {
        e.preventDefault();
    };
    canvas.on('mouse:down', function(e) {
        if (canvas.getActiveObject()) {
            $('*').bind('touchmove', tmf);
        }
    });
    canvas.on('mouse:up', function(e) {
        $('*').unbind('touchmove', tmf);
    });

Uncaught TypeError: Cannot read property 'on' of undefined. Seems there is an issue around "canvas.on"

@NandeeshG
Copy link

NandeeshG commented Nov 4, 2020

Thanks @AlexShmalex for your solution, very helpful. My solution was similar - I wanted to allow object selection/move and drawing - I think for a general solution there could be more considerations.

(function(){
  var defaultOnTouchStartHandler = fabric.Canvas.prototype._onTouchStart;
  fabric.util.object.extend(fabric.Canvas.prototype, { 
    _onTouchStart: function(e) { 
      var target = this.findTarget(e); 
      // if allowTouchScrolling is enabled, no object was at the
      // the touch position and we're not in drawing mode, then 
      // let the event skip the fabricjs canvas and do default
      // behavior
      if (this.allowTouchScrolling && !target && !this.isDrawingMode) { 
        // returning here should allow the event to propagate and be handled
        // normally by the browser
        return; 
      } 

      // otherwise call the default behavior
      defaultOnTouchStartHandler.call(this, e); 
    } 
  });
})();

Hi @wnbittle , where exactly should I add this code? Sorry if this is a bad question, I am new to fabric.js library.
Thanks

@wnbittle
Copy link

wnbittle commented Nov 5, 2020

@NandeeshG This should be run after you've loaded the fabric.js script but before you interact with the library.

@cby016
Copy link

cby016 commented Nov 12, 2020

Any update on this? It's pretty important to have this functionality on mobile and it's been over a year with no fix.

@cby016
Copy link

cby016 commented Nov 12, 2020

How can I stop the scrolling behavior if an object is selected?

@javinladish

Try this code (basically @AlexShmalex's code with a minor change):

(function() {
    var addListener = fabric.util.addListener,
        removeListener = fabric.util.removeListener,
        addEventOptions = { passive: false };

    fabric.util.object.extend(fabric.Canvas.prototype, /** @lends fabric.Canvas.prototype */ {
        _onTouchStart: function(e) {

          (!this.allowTouchScrolling || this.getActiveObject()) && e.preventDefault && e.preventDefault();

          if (this.mainTouchId === null) {
            this.mainTouchId = this.getPointerId(e);
          }
          this.__onMouseDown(e);
          this._resetTransformEventData();
          var canvasElement = this.upperCanvasEl,
              eventTypePrefix = this._getEventPrefix();
          addListener(fabric.document, 'touchend', this._onTouchEnd, addEventOptions);
          addListener(fabric.document, 'touchmove', this._onMouseMove, addEventOptions);
          // Unbind mousedown to prevent double triggers from touch devices
          removeListener(canvasElement, eventTypePrefix + 'down', this._onMouseDown);
        }
    });
})();

@Gerwin-prog
Copy link
Contributor

Gerwin-prog commented Mar 31, 2021

This issue is still relevant in version 3.4.1, I'm experiencing the same issue in #6980
#6980

edit:

var defaultOnTouchStartHandler = fabric.Canvas.prototype._onTouchStart;
fabric.util.object.extend(fabric.Canvas.prototype, {
_onTouchStart: function(e) {
var target = this.findTarget(e);
// if allowTouchScrolling is enabled, no object was at the
// the touch position and we're not in drawing mode, then
// let the event skip the fabricjs canvas and do default
// behavior
if (this.allowTouchScrolling && !target && !this.isDrawingMode) {
// returning here should allow the event to propagate and be handled
// normally by the browser
return;
}

  // otherwise call the default behavior
  defaultOnTouchStartHandler.call(this, e); 
} 

});

Seems to work well, perhaps this could be integrated into the master branch?

@1248499257
Copy link

Same Problem here

@moghwan
Copy link

moghwan commented Dec 2, 2021

@alexey-altteam's code and @wnbittle's edit work like a charm in fabricjs 4.2.0, thank you!
#5903 (comment)

@zmiju
Copy link

zmiju commented Apr 21, 2022

(function() {
var addListener = fabric.util.addListener,
removeListener = fabric.util.removeListener,
addEventOptions = { passive: false };

        fabric.util.object.extend(fabric.Canvas.prototype, /** @lends fabric.Canvas.prototype */ {
            _onTouchStart: function(e) {
                //e.preventDefault();
                if (this.mainTouchId === null) {
                    this.mainTouchId = this.getPointerId(e);
                }
                this.__onMouseDown(e);
                this._resetTransformEventData();
                var canvasElement = this.upperCanvasEl,
                eventTypePrefix = this._getEventPrefix();
                addListener(fabric.document, 'touchend', this._onTouchEnd, addEventOptions);
                addListener(fabric.document, 'touchmove', this._onMouseMove, addEventOptions);
                // Unbind mousedown to prevent double triggers from touch devices
                removeListener(canvasElement, eventTypePrefix + 'down', this._onMouseDown);
            }
        });
    })();

Hi,

do you know how to make it work in Typescript?

@rasel-xs
Copy link

@asturur
I'm facing same issue and trying to fix many way but doesn't work. can anyone help please

@maxcmoi89
Copy link

Thanks @AlexShmalex for your solution, very helpful. My solution was similar - I wanted to allow object selection/move and drawing - I think for a general solution there could be more considerations.

(function(){
  var defaultOnTouchStartHandler = fabric.Canvas.prototype._onTouchStart;
  fabric.util.object.extend(fabric.Canvas.prototype, { 
    _onTouchStart: function(e) { 
      var target = this.findTarget(e); 
      // if allowTouchScrolling is enabled, no object was at the
      // the touch position and we're not in drawing mode, then 
      // let the event skip the fabricjs canvas and do default
      // behavior
      if (this.allowTouchScrolling && !target && !this.isDrawingMode) { 
        // returning here should allow the event to propagate and be handled
        // normally by the browser
        return; 
      } 

      // otherwise call the default behavior
      defaultOnTouchStartHandler.call(this, e); 
    } 
  });
})();

This fix is needed for fabric 5.3 too

@alexisbar
Copy link

Does anybody has a working solution to use in fabric v6 ? fabric.util.object does not exists anymore as stated here #8299

@jayzambrano
Copy link

I am too looking for a solution for V6. Is allowTouchScrolling not working at all? or is there a specific set of parameters that need to be added/avoided for it to work properly? @0ro your solution works perfectly for v 5.3, do you know how it can be recreated for v6?

@jayzambrano
Copy link

jayzambrano commented Aug 12, 2024

This solution works for my scenario in V6, @alexisbar not sure if it will work for everybody, but you can try this:

  (function () {
    var defaultOnTouchStartHandler = Canvas.prototype._onTouchStart;
    Object.assign(Canvas.prototype, {
      _onTouchStart: function (e) {
        var target = this.findTarget(e);
        if (this.allowTouchScrolling && !target && !this.isDrawingMode) {
          return;
        }
        defaultOnTouchStartHandler.call(this, e);
      }
    });
  })();

@asturur
Copy link
Member

asturur commented Aug 18, 2024

For everyone having issue with allowTouchScrolling check this
#10078

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

Successfully merging a pull request may close this issue.