diff --git a/src/item/ChangeFlags.js b/src/item/ChangeFlags.js index b4fad921..6d7dfdb3 100644 --- a/src/item/ChangeFlags.js +++ b/src/item/ChangeFlags.js @@ -17,6 +17,8 @@ var ChangeFlags = { GEOMETRY: 1, // Item geometry (path, bounds) STROKE: 2, // Stroke geometry (excluding color) - STYLE: 4, // Fille style or stroke color / dash, - HIERARCHY: 8 // Change in item hierarchy + STYLE: 4, // Fill style or stroke color / dash, + APPEARANCE: 8, // Visible item attributes: visible, blendMode, opacity ... + ATTRIBUTE: 16, // Any attributes, also inviislbe ones: locked, name, ... + HIERARCHY: 32 // Change in item hierarchy }; diff --git a/src/item/Group.js b/src/item/Group.js index f3fc54af..044aeb72 100644 --- a/src/item/Group.js +++ b/src/item/Group.js @@ -69,6 +69,7 @@ var Group = this.Group = Item.extend({ */ initialize: function(items) { this.base(); + // Allow Group to have children and named children this._children = []; this._namedChildren = {}; this.setChildren(!items || !Array.isArray(items) diff --git a/src/item/Item.js b/src/item/Item.js index 7cfe0d2d..40117c98 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -85,22 +85,22 @@ var Item = this.Item = Base.extend({ }, setName: function(name) { - // Empty name '' is stored as undefined internally - if ((name || '') === (this._name || '')) - return; - var children = this._parent._children, - namedChildren = this._parent._namedChildren; - // If the item already had a name, - // remove its property from the parent's children object: + // Note: Don't check if the name has changed and bail out if it has not, + // because setName is used internally also to update internal structures + // when an item is moved from one parent to another. + + // If the item already had a name, remove the reference to it from the + // parent's children object: if (this._name) this._removeFromNamed(); + this._name = name || undefined; if (name) { + var children = this._parent._children, + namedChildren = this._parent._namedChildren; (namedChildren[name] = namedChildren[name] || []).push(this); children[name] = this; - } else if (this._name) { - delete children[this._name]; } - this._name = name || undefined; + this._changed(ChangeFlags.ATTRIBUTE); }, /** @@ -235,6 +235,7 @@ var Item = this.Item = Base.extend({ this._selected = selected; this._project._updateSelection(this); } + this._changed(ChangeFlags.ATTRIBUTE | ChangeFlags.APPEARANCE); }, _selected: false, @@ -243,8 +244,6 @@ var Item = this.Item = Base.extend({ // TODO: Change to getter / setters for these below that notify of changes // through _changed() - // TODO: Item#isLocked is currently ignored in the documentation, as - // locking an item currently has no effect /** * Specifies whether the item is locked. * @@ -294,6 +293,7 @@ var Item = this.Item = Base.extend({ this.setFillColor(null); this.setStrokeColor(null); } + this._changed(ChangeFlags.ATTRIBUTE | ChangeFlags.APPEARANCE); }, /** @@ -514,10 +514,15 @@ var Item = this.Item = Base.extend({ var children = this._parent._children, namedChildren = this._parent._namedChildren, name = this._name, - namedArray = namedChildren[name]; - if (children[name] = this) + namedArray = namedChildren[name], + index = namedArray ? namedArray.indexOf(this) : -1; + if (index == -1) + return; + // Remove the named reference + if (children[name] == this) delete children[name]; - namedArray.splice(namedArray.indexOf(this), 1); + // Remove this entry + namedArray.splice(index, 1); // If there are any items left in the named array, set // the last of them to be this.parent.children[this.name] if (namedArray.length) { diff --git a/src/path/CompoundPath.js b/src/path/CompoundPath.js index e137cb26..6fa23036 100644 --- a/src/path/CompoundPath.js +++ b/src/path/CompoundPath.js @@ -42,7 +42,9 @@ var CompoundPath = this.CompoundPath = PathItem.extend({ */ initialize: function(paths) { this.base(); + // Allow CompoundPath to have children and named children. this._children = []; + this._namedChildren = {}; // Do not reassign to paths, since arguments would get modified, which // we potentially use as array, depending on what is passed. var items = !paths || !Array.isArray(paths) diff --git a/test/tests/Item.js b/test/tests/Item.js index 1de5c25a..33d9885f 100644 --- a/test/tests/Item.js +++ b/test/tests/Item.js @@ -304,7 +304,7 @@ test('Check parent children object for named item', function() { }, true); }); -test('Named child access', function() { +test('Named child access 1', function() { var path = new Path(); path.name = 'test'; @@ -346,7 +346,7 @@ test('Named child access 2', function() { }, true); }); -test('Named child access 2', function() { +test('Named child access 3', function() { var path = new Path(); path.name = 'test'; @@ -360,28 +360,41 @@ test('Named child access 2', function() { equals(function() { return paper.project.activeLayer.children['test'] == path; }, true); - + + // TODO: Tests should not access internal properties equals(function() { - return paper.project.activeLayer._namedChildren['test'].length == 1; - }, true); + return paper.project.activeLayer._namedChildren['test'].length; + }, 1); equals(function() { return group.children['test'] == path2; }, true); - + equals(function() { return group._namedChildren['test'].length == 1; }, true); - - paper.project.activeLayer.addChild(path2); + + equals(function() { + return paper.project.activeLayer._namedChildren['test'][0] == path; + }, true); + + paper.project.activeLayer.appendTop(path2); + + equals(function() { + return group.children['test'] == null; + }, true); equals(function() { return group._namedChildren['test'] === undefined; }, true); - + equals(function() { - return paper.project.activeLayer._namedChildren['test'].length == 2; + return paper.project.activeLayer.children['test'] == path2; }, true); + + equals(function() { + return paper.project.activeLayer._namedChildren['test'].length; + }, 2); }); test('Setting name of child back to null', function() {