From 66c67fbe940c6c419f6ba503bbfbc65e9ee73b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ju=CC=88rg=20Lehni?= Date: Mon, 29 Dec 2014 23:16:13 +0100 Subject: [PATCH] Improve insertion handling for Item and Layer so insertAbove() / insertBelow() works for Layers too. Closes #603 --- src/item/Item.js | 33 +++++++++++++++++++-------------- src/item/Layer.js | 31 +++++++++++++++++-------------- src/project/Project.js | 34 ++++++++++++++++++++++------------ 3 files changed, 58 insertions(+), 40 deletions(-) diff --git a/src/item/Item.js b/src/item/Item.js index 9532eac3..be9e379c 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -2153,9 +2153,13 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ if (_proto && !(item instanceof _proto)) { items.splice(i, 1); } else { + // If the item is removed and inserted it again further + /// above, the index needs to be adjusted accordingly. + var shift = item._parent === this && item._index < index; // Notify parent of change. Don't notify item itself yet, // as we're doing so when adding it to the new parent below. - item._remove(false, true); + if (item._remove(false, true) && shift) + index--; } } Base.splice(children, items, index, 0); @@ -2181,15 +2185,10 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ }, // Private helper for #insertAbove() / #insertBelow() - _insert: function(above, item, _preserve) { - if (!item._parent) - return null; - var index = item._index + (above ? 1 : 0); - // If the item is removed and inserted it again further above, - // the index needs to be adjusted accordingly. - if (item._parent === this._parent && index > this._index) - index--; - return item._parent.insertChild(index, this, _preserve); + _insertSibling: function(index, item, _preserve) { + return this._parent + ? this._parent.insertChild(index, item, _preserve) + : null; }, /** @@ -2200,7 +2199,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ * possible. */ insertAbove: function(item, _preserve) { - return this._insert(true, item, _preserve); + return item._insertSibling(item._index + 1, this, _preserve); }, /** @@ -2211,21 +2210,27 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ * possible. */ insertBelow: function(item, _preserve) { - return this._insert(false, item, _preserve); + return item._insertSibling(item._index, this, _preserve); }, /** * Sends this item to the back of all other items within the same parent. */ sendToBack: function() { - return this._parent.insertChild(0, this); + // If there is no parent and the item is a layer, delegate to project + // instead. + return (this._parent || this instanceof Layer && this._project) + .insertChild(0, this); }, /** * Brings this item to the front of all other items within the same parent. */ bringToFront: function() { - return this._parent.addChild(this); + // If there is no parent and the item is a layer, delegate to project + // instead. + return (this._parent || this instanceof Layer && this._project) + .addChild(this); }, /** diff --git a/src/item/Layer.js b/src/item/Layer.js index 3fa0a465..a74187c5 100644 --- a/src/item/Layer.js +++ b/src/item/Layer.js @@ -81,9 +81,9 @@ var Layer = Group.extend(/** @lends Layer# */{ * Removes the layer from its project's layers list * or its parent's children list. */ - _remove: function _remove(notify) { + _remove: function _remove(notifySelf, notifyParent) { if (this._parent) - return _remove.base.call(this, notify); + return _remove.base.call(this, notifySelf, notifyParent); if (this._index != null) { var project = this._project; if (project._activeLayer === this) @@ -91,9 +91,17 @@ var Layer = Group.extend(/** @lends Layer# */{ || this.getPreviousSibling(); Base.splice(project.layers, null, this._index, 1); this._installEvents(false); - // Tell project we need a redraw. This is similar to _changed() - // mechanism. - project._needsUpdate = true; + // Notify self of the insertion change. We only need this + // notification if we're tracking changes for now. + if (notifySelf && project._changes) + this._changed(/*#=*/Change.INSERTION); + // Notify parent of changed children + if (notifyParent) { + // TODO: project._changed(/*#=*/Change.LAYERS); + // Tell project we need a redraw. This is similar to _changed() + // mechanism. + project._needsUpdate = true; + } return true; } return false; @@ -128,16 +136,11 @@ var Layer = Group.extend(/** @lends Layer# */{ }, // Private helper for #insertAbove() / #insertBelow() - _insert: function _insert(above, item, _preserve) { + _insertSibling: function _insertSibling(index, item, _preserve) { // If the item is a layer and contained within Project#layers, use // our own version of move(). - if (item instanceof Layer && !item._parent) { - this._remove(true, true); - Base.splice(item._project.layers, [this], - item._index + (above ? 1 : 0), 0); - this._setProject(item._project, true); - return this; - } - return _insert.base.call(this, above, item, _preserve); + return !this._parent + ? this._project.insertChild(index, item, _preserve) + : _insertSibling.base.call(this, index, item, _preserve); } }); diff --git a/src/project/Project.js b/src/project/Project.js index 9a9b93bd..5510cdd8 100644 --- a/src/project/Project.js +++ b/src/project/Project.js @@ -230,27 +230,37 @@ var Project = PaperScopeItem.extend(/** @lends Project# */{ return items; }, - // Helper function used in Item#copyTo and Layer#initialize - // It's called the same as Item#addChild so Item#copyTo does not need to - // make the distinction. - // TODO: Consider private function with alias in Item? - addChild: function(child) { - if (child instanceof Layer) { - Base.splice(this.layers, [child]); + // Project#insertChild() and #addChild() are helper functions called in + // Item#copyTo(), Layer#initialize(), Layer#_insertSibling() + // They are called the same as the functions on Item so duck-typing works. + insertChild: function(index, item, _preserve) { + if (item instanceof Layer) { + item._remove(false, true); + Base.splice(this.layers, [item], index, 0); + item._setProject(this, true); + // See Item#_remove() for an explanation of this: + if (this._changes) + item._changed(/*#=*/Change.INSERTION); + // TODO: this._changed(/*#=*/Change.LAYERS); // Also activate this layer if there was none before if (!this._activeLayer) - this._activeLayer = child; - } else if (child instanceof Item) { + this._activeLayer = item; + } else if (item instanceof Item) { // Anything else than layers needs to be added to a layer first (this._activeLayer // NOTE: If there is no layer and this project is not the active // one, passing insert: false and calling addChild on the // project will handle it correctly. - || this.addChild(new Layer(Item.NO_INSERT))).addChild(child); + || this.insertChild(index, new Layer(Item.NO_INSERT))) + .insertChild(index, item, _preserve); } else { - child = null; + item = null; } - return child; + return item; + }, + + addChild: function(item, _preserve) { + return this.insertChild(undefined, item, _preserve); }, // TODO: Implement setSelectedItems?