fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser
http://fabricjs.com
Other
28.74k stars 3.49k forks source link

allowTouchScrolling doesn't work #5903

Open 0ro opened 4 years ago

0ro commented 4 years ago

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 commented 4 years ago

Is anyone able to merge this?

asturur commented 4 years ago

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

Lyqxyz commented 4 years ago

How to solve this problem @asturur

0ro commented 4 years ago

@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 commented 4 years ago

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

asturur commented 4 years ago

@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 commented 4 years ago

"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 commented 4 years ago

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[bot] commented 4 years ago

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.

flexeo commented 4 years ago

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

asturur commented 4 years ago

We have a PR tracking this

linewei commented 4 years ago

Same problem.

CarlosSamp commented 4 years ago

Same Problem here

CarlosSamp commented 4 years ago

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 commented 4 years ago

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

0ro commented 4 years ago

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

you can see my solution in pull request above https://github.com/fabricjs/fabric.js/pull/5904

A-ZC-Lau commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

The same problem

Abd3lwahab commented 4 years ago

Same Problem here, Is there any fix ??

yangjiang3973 commented 4 years ago

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

alexey-altteam commented 4 years ago

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

javinladish commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 Githubissues.

  • Githubissues is a development platform for aggregating issues.