From 8c3f677cc2c2c9688ecf794d8b447cdae17927d6 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sun, 12 Aug 2018 02:33:41 +0200 Subject: [PATCH] Fix interaction between grouping and enter edit for text (#5153) --- package.json | 1 + src/mixins/itext_click_behavior.mixin.js | 8 ++- src/static_canvas.class.js | 17 +++++-- test/unit/brushes.js | 6 ++- test/unit/canvas_events.js | 3 ++ test/unit/canvas_static.js | 9 ++++ test/unit/itext.js | 1 + test/unit/itext_click_behaviour.js | 63 ++++++++++++++++++++++-- 8 files changed, 98 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 699af172469..3d37470cb51 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "build:watch": "onchange 'src/**/**' 'HEADER.js' 'lib/**/**' -- npm run build_export", "build_with_gestures": "node build.js modules=ALL exclude=accessors", "build_export": "npm run build:fast && npm run export_dist_to_site", + "test:single": "./node_modules/qunit/bin/qunit test/node_test_setup.js test/lib", "test": "istanbul cover ./node_modules/qunit/bin/qunit test/node_test_setup.js test/lib test/unit", "test:visual": "./node_modules/qunit/bin/qunit test/node_test_setup.js test/lib test/visual", "test:all": "npm run test && npm run test:visual", diff --git a/src/mixins/itext_click_behavior.mixin.js b/src/mixins/itext_click_behavior.mixin.js index 74e6cfe4a0d..14ceb0c4301 100644 --- a/src/mixins/itext_click_behavior.mixin.js +++ b/src/mixins/itext_click_behavior.mixin.js @@ -133,13 +133,15 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot */ mouseUpHandler: function(options) { this.__isMousedown = false; - if (!this.editable || + if (!this.editable || this.group || (options.transform && options.transform.actionPerformed) || (options.e.button && options.e.button !== 1)) { return; } if (this.__lastSelected && !this.__corner) { + this.selected = false; + this.__lastSelected = false; this.enterEditing(options.e); if (this.selectionStart === this.selectionEnd) { this.initDelayedCursor(true); @@ -148,7 +150,9 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot this.renderCursorOrSelection(); } } - this.selected = true; + else { + this.selected = true; + } }, /** diff --git a/src/static_canvas.class.js b/src/static_canvas.class.js index db85ab39529..3e2bd8f9747 100644 --- a/src/static_canvas.class.js +++ b/src/static_canvas.class.js @@ -838,6 +838,10 @@ /** * Function created to be instance bound at initialization * used in requestAnimationFrame rendering + * Let the fabricJS call it. If you call it manually you could have more + * animationFrame stacking on to of each other + * for an imperative rendering, use canvas.renderAll + * @private * @return {fabric.Canvas} instance * @chainable */ @@ -848,6 +852,7 @@ /** * Append a renderAll request to next animation frame. + * unless one is already in progress, in that case nothing is done * a boolean flag will avoid appending more. * @return {fabric.Canvas} instance * @chainable @@ -877,6 +882,13 @@ return points; }, + cancelRequestedRender: function() { + if (this.isRendering) { + fabric.util.cancelAnimFrame(this.isRendering); + this.isRendering = 0; + } + }, + /** * Renders background, objects, overlay and controls. * @param {CanvasRenderingContext2D} ctx @@ -886,10 +898,7 @@ */ renderCanvas: function(ctx, objects) { var v = this.viewportTransform; - if (this.isRendering) { - fabric.util.cancelAnimFrame(this.isRendering); - this.isRendering = 0; - } + this.cancelRequestedRender(); this.calcViewportBoundaries(); this.clearContext(ctx); this.fire('before:render'); diff --git a/test/unit/brushes.js b/test/unit/brushes.js index ee7c17895dd..0c4ded85a00 100644 --- a/test/unit/brushes.js +++ b/test/unit/brushes.js @@ -1,6 +1,10 @@ (function() { var canvas = new fabric.Canvas(); - QUnit.module('fabric.BaseBrush'); + QUnit.module('fabric.BaseBrush', { + afterEach: function() { + canvas.cancelRequestedRender(); + } + }); QUnit.test('fabric brush constructor', function(assert) { assert.ok(fabric.BaseBrush); diff --git a/test/unit/canvas_events.js b/test/unit/canvas_events.js index 8e056794a52..e59b4183589 100644 --- a/test/unit/canvas_events.js +++ b/test/unit/canvas_events.js @@ -5,6 +5,7 @@ QUnit.module('fabric.Canvas events mixin', { beforeEach: function() { + canvas.cancelRequestedRender(); upperCanvasEl.style.display = ''; canvas.controlsAboveOverlay = fabric.Canvas.prototype.controlsAboveOverlay; canvas.preserveObjectStacking = fabric.Canvas.prototype.preserveObjectStacking; @@ -17,6 +18,7 @@ canvas.off(); canvas.calcOffset(); upperCanvasEl.style.display = 'none'; + canvas.cancelRequestedRender(); } }); @@ -432,6 +434,7 @@ fabric.document.dispatchEvent(event); assert.equal(counter, 1, 'listener executed once'); fabric.Canvas.prototype._onMouseUp = originalMouseUp; + c.cancelRequestedRender(); done(); }, 200); }); diff --git a/test/unit/canvas_static.js b/test/unit/canvas_static.js index 9d4cbc34e36..fb2769c2fec 100644 --- a/test/unit/canvas_static.js +++ b/test/unit/canvas_static.js @@ -1771,6 +1771,15 @@ assert.equal(svg, expectedSVG, 'svg is as expected'); }); + QUnit.test('requestRenderAll and cancelRequestedRender', function(assert) { + var canvas2 = new fabric.StaticCanvas(); + assert.equal(canvas2.isRendering, undefined, 'no redering is in progress'); + canvas2.requestRenderAll(); + assert.notEqual(canvas2.isRendering, 0, 'a rendering is scehduled'); + canvas2.cancelRequestedRender(); + assert.equal(canvas2.isRendering, 0, 'rendering cancelled'); + }); + // QUnit.test('backgroundImage', function(assert) { // var done = assert.async(); // assert.deepEqual('', canvas.backgroundImage); diff --git a/test/unit/itext.js b/test/unit/itext.js index a06e5a8091d..9f339cd44f3 100644 --- a/test/unit/itext.js +++ b/test/unit/itext.js @@ -4,6 +4,7 @@ QUnit.module('fabric.IText', { afterEach: function() { canvas.clear(); + canvas.cancelRequestedRender(); } }); diff --git a/test/unit/itext_click_behaviour.js b/test/unit/itext_click_behaviour.js index af5ab98d31e..76220dc428a 100644 --- a/test/unit/itext_click_behaviour.js +++ b/test/unit/itext_click_behaviour.js @@ -1,4 +1,12 @@ (function(){ + + var canvas = new fabric.Canvas(); + + QUnit.module('iText click interaction', { + afterEach: function() { + canvas.cancelRequestedRender(); + } + }); QUnit.test('_getNewSelectionStartFromOffset end of line', function(assert) { var iText = new fabric.IText('test need some word\nsecond line'); var index = 10; @@ -30,10 +38,59 @@ QUnit.test('_mouseDownHandlerBefore set up selected property', function(assert) { var iText = new fabric.IText('test need some word\nsecond line'); assert.equal(iText.selected, undefined, 'iText has no selected property'); - iText.canvas = { - _activeObject: iText, - }; + canvas.setActiveObject(iText); + iText.canvas = canvas; iText._mouseDownHandlerBefore({ e: {} }); assert.equal(iText.selected, true, 'iText has selected property'); + assert.equal(iText.__lastSelected, undefined, 'iText has no __lastSelected property'); + }); + QUnit.test('_mouseUpHandler set selected as true', function(assert) { + var iText = new fabric.IText('test'); + iText.initDelayedCursor = function() {}; + iText.renderCursorOrSelection = function() {}; + assert.equal(iText.selected, undefined, 'iText has no selected property'); + assert.equal(iText.__lastSelected, undefined, 'iText has no __lastSelected property'); + canvas.setActiveObject(iText); + iText.canvas = canvas; + iText.mouseUpHandler({ e: {} }); + assert.equal(iText.selected, true, 'iText has selected property'); + }); + QUnit.test('_mouseUpHandler on a selected object enter edit', function(assert) { + var iText = new fabric.IText('test'); + iText.initDelayedCursor = function() {}; + iText.renderCursorOrSelection = function() {}; + assert.equal(iText.isEditing, false, 'iText not editing'); + iText.canvas = canvas; + iText.selected = true; + iText.__lastSelected = true; + iText.mouseUpHandler({ e: {} }); + assert.equal(iText.isEditing, true, 'iText entered editing'); + iText.exitEditing(); + }); + QUnit.test('_mouseUpHandler on a selected text in a group DOES NOT enter edit', function(assert) { + var iText = new fabric.IText('test'); + iText.initDelayedCursor = function() {}; + iText.renderCursorOrSelection = function() {}; + assert.equal(iText.isEditing, false, 'iText not editing'); + iText.canvas = canvas; + iText.selected = true; + iText.__lastSelected = true; + iText.group = true; + iText.mouseUpHandler({ e: {} }); + assert.equal(iText.isEditing, false, 'iText did not entered editing'); + iText.exitEditing(); + }); + QUnit.test('_mouseUpHandler on a corner of selected text DOES NOT enter edit', function(assert) { + var iText = new fabric.IText('test'); + iText.initDelayedCursor = function() {}; + iText.renderCursorOrSelection = function() {}; + assert.equal(iText.isEditing, false, 'iText not editing'); + iText.canvas = canvas; + iText.selected = true; + iText.__lastSelected = true; + iText.__corner = 'mt'; + iText.mouseUpHandler({ e: {} }); + assert.equal(iText.isEditing, false, 'iText did not entered editing'); + iText.exitEditing(); }); })();