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

Fix allow touch scrolling #5904

Closed
wants to merge 1 commit into from
Closed

Conversation

0ro
Copy link

@0ro 0ro commented Oct 1, 2019

Copy link

@wegneto wegneto left a comment

Choose a reason for hiding this comment

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

I've applied this to my code and it's working fine.

@iherasymenko
Copy link
Contributor

iherasymenko commented Jan 24, 2020

Works just fine for me.

@asturur asturur added the bug label Feb 2, 2020
@RinaldoNani
Copy link

allowTouchScrolling is not working on iPad as of last version of fabric.js. BUT works perfectly with e.g. version 2.3.3. This is driving me crazy.
I thought the issue was fixed!...
Please, hints are definitely needed!

@asturur
Copy link
Member

asturur commented Feb 15, 2020

This should fix it.
I have no idea what kind of regression testing have been done and i m actually concentrated on other things. But you can probably apply this fix safely

@emily-yoon
Copy link

This worked for me as well.

@royercross
Copy link

This fix works to fix the scroll, But... When you put an object and moveit arround, the scroll canvas also move with it, anyone worked on a solution on that?.

Copy link

@wegneto wegneto left a comment

Choose a reason for hiding this comment

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

I've added this fix to my code I have been using it for 6 months without noticing any side-effects.

@wegneto
Copy link

wegneto commented Jun 2, 2020

This fix works to fix the scroll, But... When you put an object and moveit arround, the scroll canvas also move with it, anyone worked on a solution on that?.

Hey @royercross ,

I had the same issue and fixed it by using a logic to enable and disable the scrolling. Like this:

function configScroll (canvas) {
    var disableScroll = function () {
        canvas.allowTouchScrolling = false;
    };

    var enableScroll = function () {
        canvas.allowTouchScrolling = true;
    };

    canvas.on('selection:created', disableScroll);

    canvas.on('mouse:down', function (event) {
        if (event.target && event.target.asset) {
            disableScroll();
        } else {
            enableScroll();
        }
    });

    canvas.on('selection:cleared', enableScroll);
}

@royercross
Copy link

royercross commented Jun 2, 2020

This fix works to fix the scroll, But... When you put an object and moveit arround, the scroll canvas also move with it, anyone worked on a solution on that?.

Hey @royercross ,

I had the same issue and fixed it by using a logic to enable and disable the scrolling. Like this:

function configScroll (canvas) {
    var disableScroll = function () {
        canvas.allowTouchScrolling = false;
    };

    var enableScroll = function () {
        canvas.allowTouchScrolling = true;
    };

    canvas.on('selection:created', disableScroll);

    canvas.on('mouse:down', function (event) {
        if (event.target && event.target.asset) {
            disableScroll();
        } else {
            enableScroll();
        }
    });

    canvas.on('selection:cleared', enableScroll);
}

i tried that, but the problem is on mobile only, if you move an element to quickly and fast, you get an error because you cannot prevent scrolling event if it is alrready happening.

So i found another solution in case anyone need it.

_onTouchStart: function(e) {
      var targetR = this.findTarget(e);
      !this.allowTouchScrolling && e.preventDefault && e.preventDefault();
      targetR && e.preventDefault && e.preventDefault();

@lynmije
Copy link

lynmije commented Jun 6, 2020

i tried the solutions above but scrolling is still not working on my end with mobile devices

@juxhinNM
Copy link

Will it take more to merged?

@allweek
Copy link

allweek commented Dec 17, 2020

well, combined peoples solutions I got result with this code

(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) {
              var targetR = this.findTarget(e);
              !this.allowTouchScrolling && e.preventDefault && e.preventDefault();
              targetR && 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);
            }
          });
        })();

//(!) Important, this code above should be before fabric canvas init and "allowTouchScrolling: true" should be added (v.4.2.0):
canvas = new fabric.Canvas('canvasId', {allowTouchScrolling: true});

Thanks for all !

@RakeshYadvanshi
Copy link

well, combined peoples solutions I got result with this code

(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) {
              var targetR = this.findTarget(e);
              !this.allowTouchScrolling && e.preventDefault && e.preventDefault();
              targetR && 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);
            }
          });
        })();

//(!) Important, this code above should be before fabric canvas init and "allowTouchScrolling: true" should be added (v.4.2.0): canvas = new fabric.Canvas('canvasId', {allowTouchScrolling: true});

Thanks for all !

it does not work for me

@squireaj
Copy link

squireaj commented Nov 11, 2022

Any reason why this has never been merged? We could really use it in our project. That fix above worked but still, why can't the PR just be merged?

@yiukamsum
Copy link

well, combined peoples solutions I got result with this code

(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) {
              var targetR = this.findTarget(e);
              !this.allowTouchScrolling && e.preventDefault && e.preventDefault();
              targetR && 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);
            }
          });
        })();

//(!) Important, this code above should be before fabric canvas init and "allowTouchScrolling: true" should be added (v.4.2.0): canvas = new fabric.Canvas('canvasId', {allowTouchScrolling: true});
Thanks for all !

it does not work for me

    _onTouchStart: function(e) {
      let targetR = this.findTarget(e);
      if(!this.allowTouchScrolling || targetR){
        e.preventDefault && e.preventDefault();
      }
      if (this.mainTouchId === null) {
        this.mainTouchId = this.getPointerId(e);
      }
      // ...
    },
    _onMouseMove: function (e) {
      let targetR = this.findTarget(e);
      if(!this.allowTouchScrolling || targetR){
        e.preventDefault && e.preventDefault();
      }
      this.__onMouseMove(e);
    },

This work for me

@asturur asturur closed this Aug 18, 2024
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 this pull request may close these issues.