From 9350ee2465fb61814525f8f7868b1fe7be20f42b Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sun, 18 Aug 2024 18:19:05 +0200 Subject: [PATCH 1/6] test interaction --- .codesandbox/templates/vanilla/src/index.ts | 4 +++- .../vanilla/src/testcases/responsive.ts | 3 ++- src/canvas/Canvas.ts | 24 +++++++++++++------ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/.codesandbox/templates/vanilla/src/index.ts b/.codesandbox/templates/vanilla/src/index.ts index f0bcbd8a7a3..c0b21a7c98d 100644 --- a/.codesandbox/templates/vanilla/src/index.ts +++ b/.codesandbox/templates/vanilla/src/index.ts @@ -3,7 +3,9 @@ import './styles.css'; import { testCase } from './testcases/responsive'; const el = document.getElementById('canvas'); -const canvas = (window.canvas = new fabric.Canvas(el)); +const canvas = (window.canvas = new fabric.Canvas(el, { + allowTouchScrolling: true, +})); // edit from here canvas.setDimensions({ diff --git a/.codesandbox/templates/vanilla/src/testcases/responsive.ts b/.codesandbox/templates/vanilla/src/testcases/responsive.ts index e20fa0a40cd..1601cec3440 100644 --- a/.codesandbox/templates/vanilla/src/testcases/responsive.ts +++ b/.codesandbox/templates/vanilla/src/testcases/responsive.ts @@ -3,7 +3,8 @@ import * as fabric from 'fabric'; export async function testCase(canvas: fabric.Canvas) { const rect = new fabric.Rect({ width: 50, height: 50, fill: 'blue' }); const rect2 = new fabric.Rect({ width: 50, height: 50, fill: 'blue' }); - + canvas.allowTouchScrolling = true; canvas.add(rect, rect2); + canvas.setDimensions({ height: 4000 }, { backstoreOnly: true }); canvas.setDimensions({ width: '100%', height: 'auto' }, { cssOnly: true }); } diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index 2a5fb90e590..bcb4425828d 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -597,11 +597,19 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { * @param {Event} e Event object fired on mousedown */ _onTouchStart(e: TouchEvent) { - e.preventDefault(); + let shouldPreventScrolling = !this.allowTouchScrolling; if (this.mainTouchId === undefined) { this.mainTouchId = this.getPointerId(e); } this.__onMouseDown(e); + // after executing fabric logic for mouse down let's see + // if we have a target or we are drawing. in that case + // we want to prevent scrolling anyway + if (this._target || this.isDrawingMode) { + shouldPreventScrolling = true; + } + // prevent default, will block scrolling from start + shouldPreventScrolling && e.preventDefault(); this._resetTransformEventData(); const canvasElement = this.upperCanvasEl, eventTypePrefix = this._getEventPrefix(); @@ -612,12 +620,14 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { this._onTouchEnd as EventListener, addEventOptions, ); - addListener( - doc, - 'touchmove', - this._onMouseMove as EventListener, - addEventOptions, - ); + // if we scroll don't register the touch move event + shouldPreventScrolling && + addListener( + doc, + 'touchmove', + this._onMouseMove as EventListener, + addEventOptions, + ); // Unbind mousedown to prevent double triggers from touch devices removeListener( canvasElement, From 047e87b3f28b05270c1417c963d2f154314b35ec Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 18 Aug 2024 16:39:30 +0000 Subject: [PATCH 2/6] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62943af384c..04c406a23e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- fix(Canvas): allowTouchScrolling interactions [#10078](https://github.com/fabricjs/fabric.js/pull/10078) - feat(Canvas): Avoid styling the lower canvas with absolute positioning [#10077](https://github.com/fabricjs/fabric.js/pull/10077) - chore(TS): Add missing export type for Text events [#10076](https://github.com/fabricjs/fabric.js/pull/10076) - chore(CI): Move test actions to Node 20 [#10073](https://github.com/fabricjs/fabric.js/pull/10073) From 318bb7b0689f81fc1663586d146f46f9b14e29d9 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sun, 18 Aug 2024 20:46:11 +0200 Subject: [PATCH 3/6] fix --- src/canvas/Canvas.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index bcb4425828d..108f8d39bb1 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -597,15 +597,20 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { * @param {Event} e Event object fired on mousedown */ _onTouchStart(e: TouchEvent) { + // we will prevent scrolling if allowTouchScrolling is not enabled and let shouldPreventScrolling = !this.allowTouchScrolling; + const currentActiveObject = this._activeObject; if (this.mainTouchId === undefined) { this.mainTouchId = this.getPointerId(e); } this.__onMouseDown(e); // after executing fabric logic for mouse down let's see - // if we have a target or we are drawing. in that case + // if we didn't change target or if we are drawing // we want to prevent scrolling anyway - if (this._target || this.isDrawingMode) { + if ( + this.isDrawingMode || + (currentActiveObject && this._target === currentActiveObject) + ) { shouldPreventScrolling = true; } // prevent default, will block scrolling from start From f871043e75d545df447eccdd783d6af7ecee81ce Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sun, 18 Aug 2024 21:57:27 +0200 Subject: [PATCH 4/6] initial test --- e2e/tests/controls/touch-scrolling/index.ts | 10 +++++ src/canvas/Canvas.spec.ts | 42 +++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 e2e/tests/controls/touch-scrolling/index.ts create mode 100644 src/canvas/Canvas.spec.ts diff --git a/e2e/tests/controls/touch-scrolling/index.ts b/e2e/tests/controls/touch-scrolling/index.ts new file mode 100644 index 00000000000..400fbc462ad --- /dev/null +++ b/e2e/tests/controls/touch-scrolling/index.ts @@ -0,0 +1,10 @@ +import { Rect } from 'fabric'; +import { beforeAll } from '../../test'; + +beforeAll((canvas) => { + const rect = new Rect({ width: 50, height: 50, fill: 'blue' }); + canvas.allowTouchScrolling = true; + canvas.setDimensions({ width: 600, height: 4000 }); + canvas.add(rect); + return { rect }; +}); diff --git a/src/canvas/Canvas.spec.ts b/src/canvas/Canvas.spec.ts new file mode 100644 index 00000000000..57aa3dc72f2 --- /dev/null +++ b/src/canvas/Canvas.spec.ts @@ -0,0 +1,42 @@ +import { Canvas } from './Canvas'; + +describe('Canvas', () => { + describe('touchStart', () => { + test('will prevent default to allow touch scrolling', () => { + const canvas = new Canvas(undefined, { + allowTouchScrolling: false, + }); + const touch = { + clientX: 10, + clientY: 0, + identifier: 1, + target: canvas.upperCanvasEl, + }; + const evt = new TouchEvent('touchstart', { + touches: [touch], + changedTouches: [touch], + }); + evt.preventDefault = jest.fn(); + canvas._onTouchStart(evt); + expect(evt.preventDefault).toHaveBeenCalled(); + }); + test('will not prevent default if allow touch scrolling is true', () => { + const canvas = new Canvas(undefined, { + allowTouchScrolling: true, + }); + const touch = { + clientX: 10, + clientY: 0, + identifier: 1, + target: canvas.upperCanvasEl, + }; + const evt = new TouchEvent('touchstart', { + touches: [touch], + changedTouches: [touch], + }); + evt.preventDefault = jest.fn(); + canvas._onTouchStart(evt); + expect(evt.preventDefault).not.toHaveBeenCalled(); + }); + }); +}); From 61b78e6c54a0b9c986109cd42dcb6eaaf1dd11e8 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sat, 12 Oct 2024 15:24:16 +0200 Subject: [PATCH 5/6] more tests --- src/canvas/Canvas.spec.ts | 76 ++++++++++++++++++++++++++++++- src/canvas/StaticCanvasOptions.ts | 3 ++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/canvas/Canvas.spec.ts b/src/canvas/Canvas.spec.ts index 57aa3dc72f2..abd4c2da767 100644 --- a/src/canvas/Canvas.spec.ts +++ b/src/canvas/Canvas.spec.ts @@ -1,8 +1,9 @@ import { Canvas } from './Canvas'; +import { Rect } from '../shapes/Rect'; describe('Canvas', () => { describe('touchStart', () => { - test('will prevent default to allow touch scrolling', () => { + test('will prevent default to not allow dom scrolling on canvas touch drag', () => { const canvas = new Canvas(undefined, { allowTouchScrolling: false, }); @@ -20,7 +21,7 @@ describe('Canvas', () => { canvas._onTouchStart(evt); expect(evt.preventDefault).toHaveBeenCalled(); }); - test('will not prevent default if allow touch scrolling is true', () => { + test('will not prevent default when allowTouchScrolling is true and there is no action', () => { const canvas = new Canvas(undefined, { allowTouchScrolling: true, }); @@ -38,5 +39,76 @@ describe('Canvas', () => { canvas._onTouchStart(evt); expect(evt.preventDefault).not.toHaveBeenCalled(); }); + test('will prevent default when allowTouchScrolling is true but we are drawing', () => { + const canvas = new Canvas(undefined, { + allowTouchScrolling: true, + isDrawingMode: true, + }); + const touch = { + clientX: 10, + clientY: 0, + identifier: 1, + target: canvas.upperCanvasEl, + }; + const evt = new TouchEvent('touchstart', { + touches: [touch], + changedTouches: [touch], + }); + evt.preventDefault = jest.fn(); + canvas._onTouchStart(evt); + expect(evt.preventDefault).toHaveBeenCalled(); + }); + test('will prevent default when allowTouchScrolling is true and we are dragging an object', () => { + const canvas = new Canvas(undefined, { + allowTouchScrolling: true, + }); + const rect = new Rect({ + width: 2000, + height: 2000, + left: -500, + top: -500, + }); + canvas.add(rect); + canvas.setActiveObject(rect); + const touch = { + clientX: 10, + clientY: 0, + identifier: 1, + target: canvas.upperCanvasEl, + }; + const evt = new TouchEvent('touchstart', { + touches: [touch], + changedTouches: [touch], + }); + evt.preventDefault = jest.fn(); + canvas._onTouchStart(evt); + expect(evt.preventDefault).toHaveBeenCalled(); + }); + test('will NOT prevent default when allowTouchScrolling is true and we just lost selection', () => { + const canvas = new Canvas(undefined, { + allowTouchScrolling: true, + }); + const rect = new Rect({ + width: 200, + height: 200, + left: 1000, + top: 1000, + }); + canvas.add(rect); + canvas.setActiveObject(rect); + const touch = { + clientX: 10, + clientY: 0, + identifier: 1, + target: canvas.upperCanvasEl, + }; + const evt = new TouchEvent('touchstart', { + touches: [touch], + changedTouches: [touch], + }); + evt.preventDefault = jest.fn(); + canvas._onTouchStart(evt); + expect(evt.preventDefault).not.toHaveBeenCalled(); + }); }); }); diff --git a/src/canvas/StaticCanvasOptions.ts b/src/canvas/StaticCanvasOptions.ts index f544088e82a..c999665fbf8 100644 --- a/src/canvas/StaticCanvasOptions.ts +++ b/src/canvas/StaticCanvasOptions.ts @@ -152,6 +152,9 @@ export interface StaticCanvasOptions /** * Indicates whether the browser can be scrolled when using a touchscreen and dragging on the canvas + * It gives PRIORITY to DOM scrolling, it doesn't make it always possible. + * If is true, when using a touch event on the canvas, the canvas will scroll if scroll is possible. + * If we are in drawing mode or if we are selecting an object the canvas preventDefault and so it won't scroll * @type Boolean * @default * From 28d94ac62f9a47e14acd7f20ae4632bb59d81a66 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sat, 12 Oct 2024 15:43:38 +0200 Subject: [PATCH 6/6] no changes --- .codesandbox/templates/vanilla/src/index.ts | 4 +--- .codesandbox/templates/vanilla/src/testcases/responsive.ts | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.codesandbox/templates/vanilla/src/index.ts b/.codesandbox/templates/vanilla/src/index.ts index ef4c55acad8..cafc616d9d6 100644 --- a/.codesandbox/templates/vanilla/src/index.ts +++ b/.codesandbox/templates/vanilla/src/index.ts @@ -3,9 +3,7 @@ import './styles.css'; import { testCase } from './testcases/loadingSvgs'; const el = document.getElementById('canvas'); -const canvas = (window.canvas = new fabric.Canvas(el, { - allowTouchScrolling: true, -})); +const canvas = (window.canvas = new fabric.Canvas(el)); // edit from here canvas.setDimensions({ diff --git a/.codesandbox/templates/vanilla/src/testcases/responsive.ts b/.codesandbox/templates/vanilla/src/testcases/responsive.ts index 1601cec3440..e20fa0a40cd 100644 --- a/.codesandbox/templates/vanilla/src/testcases/responsive.ts +++ b/.codesandbox/templates/vanilla/src/testcases/responsive.ts @@ -3,8 +3,7 @@ import * as fabric from 'fabric'; export async function testCase(canvas: fabric.Canvas) { const rect = new fabric.Rect({ width: 50, height: 50, fill: 'blue' }); const rect2 = new fabric.Rect({ width: 50, height: 50, fill: 'blue' }); - canvas.allowTouchScrolling = true; + canvas.add(rect, rect2); - canvas.setDimensions({ height: 4000 }, { backstoreOnly: true }); canvas.setDimensions({ width: '100%', height: 'auto' }, { cssOnly: true }); }