fix: fixed undo redo bug for shape menu (fixed #315 #310) (#316)

* fix: fixed undo redo bug for shape menu

* apply codereview

src/js/action.js

@@ -301,9 +301,9 @@ export default {
      */
     _shapeAction() {
         return extend({
-            changeShape: changeShapeObject => {
+            changeShape: (changeShapeObject, isSilent) => {
                 if (this.activeObjectId) {
-                    this.changeShape(this.activeObjectId, changeShapeObject);
+                    this.changeShape(this.activeObjectId, changeShapeObject, isSilent);
                 }
             },
             setDrawingShape: shapeType => {

src/js/command/changeShape.js

@@ -10,6 +10,33 @@ import consts from '../consts';
 const {componentNames, rejectMessages, commandNames} = consts;
 const {SHAPE} = componentNames;
 
+/**
+ * Make undoData
+ * @param {object} options - shape options
+ * @param {Component} targetObj - shape component
+ * @param {boolean} isSilent - is silent execution or not
+ * @returns {object} - undo data
+ */
+function makeUndoData(options, targetObj, isSilent) {
+    const undoData = {
+        object: targetObj,
+        options: {}
+    };
+
+    snippet.forEachOwnProperties(options, (value, key) => {
+        let undoValue = targetObj[key];
+
+        if (!isSilent) {
+            undoValue = targetObj.lastUndoStackForShape[key];
+            targetObj.lastUndoStackForShape[key] = targetObj[key];
+        }
+
+        undoData.options[key] = undoValue;
+    });
+
+    return undoData;
+}
+
 const command = {
     name: commandNames.CHANGE_SHAPE,
 
@@ -28,21 +55,20 @@ const command = {
      *      @param {number} [options.left] - Shape x position
      *      @param {number} [options.top] - Shape y position
      *      @param {number} [options.isRegular] - Whether resizing shape has 1:1 ratio or not
+     * @param {boolean} isSilent - is silent execution or not
      * @returns {Promise}
      */
-    execute(graphics, id, options) {
+    execute(graphics, id, options, isSilent) {
         const shapeComp = graphics.getComponent(SHAPE);
         const targetObj = graphics.getObject(id);
 
         if (!targetObj) {
             return Promise.reject(rejectMessages.noObject);
         }
 
-        this.undoData.object = targetObj;
-        this.undoData.options = {};
-        snippet.forEachOwnProperties(options, (value, key) => {
-            this.undoData.options[key] = targetObj[key];
-        });
+        if (!this.isRedo()) {
+            snippet.extend(this.undoData, makeUndoData(options, targetObj, isSilent));
+        }
 
         return shapeComp.change(targetObj, options);
     },

src/js/component/shape.js

@@ -181,7 +181,9 @@ class Shape extends Component {
         return new Promise(resolve => {
             const canvas = this.getCanvas();
             options = this._extendOptions(options);
+
             const shapeObj = this._createInstance(type, options);
+            shapeObj.lastUndoStackForShape = options;
 
             this._bindEventOnShape(shapeObj);
 

src/js/consts.js

@@ -167,7 +167,7 @@ module.exports = {
     },
 
     defaultShapeStrokeValus: {
-        realTimeEvent: false,
+        realTimeEvent: true,
         min: 2,
         max: 300,
         value: 3

src/js/imageEditor.js

@@ -947,6 +947,7 @@ class ImageEditor {
      *      @param {number} [options.rx] - Radius x value (When type option is 'circle', this options can use)
      *      @param {number} [options.ry] - Radius y value (When type option is 'circle', this options can use)
      *      @param {boolean} [options.isRegular] - Whether resizing shape has 1:1 ratio or not
+     * @param {boolean} isSilent - is silent execution or not
      * @returns {Promise}
      * @example
      * // call after selecting shape object on canvas
@@ -967,8 +968,10 @@ class ImageEditor {
      *     ry: 100
      * });
      */
-    changeShape(id, options) {
-        return this.execute(commands.CHANGE_SHAPE, id, options);
+    changeShape(id, options, isSilent) {
+        const executeMethodName = isSilent ? 'executeSilent' : 'execute';
+
+        return this[executeMethodName](commands.CHANGE_SHAPE, id, options);
     }
 
     /**

src/js/interface/command.js

@@ -78,6 +78,14 @@ class Command {
         throw new Error(createMessage(errorTypes.UN_IMPLEMENTATION, 'undo'));
     }
 
+    /**
+     * command for redo if undoData exists
+     * @returns {boolean} isRedo
+     */
+    isRedo() {
+        return Object.keys(this.undoData).length;
+    }
+
     /**
      * Attach execute callabck
      * @param {function} callback - Callback after execution

src/js/ui/shape.js

@@ -158,13 +158,14 @@ class Shape extends Submenu {
     /**
      * Change stroke range
      * @param {number} value - stroke range value
+     * @param {boolean} isLast - Is last change
      * @private
      */
-    _changeStrokeRangeHandler(value) {
+    _changeStrokeRangeHandler(value, isLast) {
         this.options.strokeWidth = toInteger(value);
         this.actions.changeShape({
             strokeWidth: value
-        });
+        }, !isLast);
 
         this.actions.setDrawingShape(this.type, this.options);
     }

test/command.spec.js

@@ -327,6 +327,37 @@ describe('commandFactory', () => {
         });
     });
 
+    describe('shapeCommand', () => {
+        let shapeObjectId;
+        const defaultStrokeWidth = 12;
+        beforeEach(done => {
+            invoker.execute(commands.ADD_SHAPE, graphics, 'rect', {
+                strokeWidth: defaultStrokeWidth
+            }).then(shapeObject => {
+                shapeObjectId = shapeObject.id;
+                done();
+            });
+        });
+        it('"changeShape" should set strokeWidth', done => {
+            invoker.execute(commands.CHANGE_SHAPE, graphics, shapeObjectId, {
+                strokeWidth: 50
+            }).then(() => {
+                const shapeObject = graphics.getObject(shapeObjectId);
+                expect(shapeObject.strokeWidth).toBe(50);
+                done();
+            });
+        });
+        it('"redo()" should restore strokeWidth', done => {
+            invoker.execute(commands.CHANGE_SHAPE, graphics, shapeObjectId, {
+                strokeWidth: 50
+            }).then(() => invoker.undo()).then(() => {
+                const shapeObject = graphics.getObject(shapeObjectId);
+                expect(shapeObject.strokeWidth).toBe(defaultStrokeWidth);
+                done();
+            });
+        });
+    });
+
     describe('clearCommand', () => {
         let canvasContext;