Improve insertion handling for Item and Layer so insertAbove() / insertBelow() works for Layers too.

Closes #603

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) {
+    _insertSibling: function(index, item, _preserve) {
-        if (!item._parent)
+        return this._parent
-            return null;
+                ? this._parent.insertChild(index, item, _preserve)
-        var index = item._index + (above ? 1 : 0);
+                : null;
-        // 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);
     },
 
     /**
@@ -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);
     },
 
     /**

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()
+            // Notify self of the insertion change. We only need this
-            // mechanism.
+            // notification if we're tracking changes for now.
-            project._needsUpdate = true;
+            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) {
+        return !this._parent
-            this._remove(true, true);
+                ? this._project.insertChild(index, item, _preserve)
-            Base.splice(item._project.layers, [this],
+                : _insertSibling.base.call(this, index, item, _preserve);
-                    item._index + (above ? 1 : 0), 0);
-            this._setProject(item._project, true);
-            return this;
-        }
-        return _insert.base.call(this, above, item, _preserve);
     }
 });

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
+    // Project#insertChild() and #addChild() are helper functions called in
-    // It's called the same as Item#addChild so Item#copyTo does not need to
+    // Item#copyTo(), Layer#initialize(), Layer#_insertSibling()
-    // make the distinction.
+    // They are called the same as the functions on Item so duck-typing works.
-    // TODO: Consider private function with alias in Item?
+    insertChild: function(index, item, _preserve) {
-    addChild: function(child) {
+        if (item instanceof Layer) {
-        if (child instanceof Layer) {
+            item._remove(false, true);
-            Base.splice(this.layers, [child]);
+            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;
+                this._activeLayer = item;
-        } else if (child instanceof 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?