diff --git a/src/dom_components/model/Component.js b/src/dom_components/model/Component.js index 76f0f137f..bcbedb1bc 100644 --- a/src/dom_components/model/Component.js +++ b/src/dom_components/model/Component.js @@ -688,6 +688,19 @@ const Component = Backbone.Model.extend(Styleable).extend( ); }, + __isSymbolNested() { + if (!this.__isSymbolOrInst() || this.__isSymbolTop()) return false; + const symbTopSelf = (this.__isSymbol() + ? this + : this.__getSymbol() + ).__getSymbTop(); + const symbTop = this.__getSymbTop(); + const symbTopMain = symbTop.__isSymbol() + ? symbTop + : symbTop.__getSymbol(); + return symbTopMain !== symbTopSelf; + }, + __getAllById() { const { em } = this; return em ? em.get('DomComponents').allById() : {}; @@ -823,31 +836,53 @@ const Component = Backbone.Model.extend(Styleable).extend( symb.append(toAppend, { fromInstance: this, ...o }); }); } else { - // Allow removing single instances + // Propagate remove only if the component is an inner symbol if (!m.__isSymbolTop()) { - const toUp = m.__getSymbToUp(toUpOpts); + const { index } = o; + const opts = { fromInstance: m, ...o }; + const isSymbNested = m.__isSymbolNested(); + let toUpFn = symb => symb.remove(opts); + let toUp = m.__getSymbToUp(toUpOpts); + + if (isSymbNested) { + const parent = m.parent(); + toUp = parent.__getSymbToUp(toUpOpts); + toUpFn = symb => { + const toRemove = symb.components().at(index); + toRemove && toRemove.remove({ fromInstance: parent, ...opts }); + }; + } !isTemp && - this.__logSymbol('remove', toUp, { opts: o, removed: m.cid }); - toUp.forEach(symb => { - const opts = { fromInstance: m, ...o }; - - // In case of nested symbols, I only need to propagate changes to its instances - if (symb.__isSymbolTop() && symb.__getSymbols()) { - const toUpInst = symb.__getSymbToUp({ - fromInstance: m, - ...toUpOpts - }); - this.__logSymbol('remove-inst', toUpInst, { - opts: o, - symbol: symb - }); - toUpInst.forEach(inst => { - inst.remove(opts); - }); - } else { - symb.remove(opts); - } - }); + this.__logSymbol('remove', toUp, { + opts: o, + removed: m.cid, + isSymbNested + }); + toUp.forEach(toUpFn); + + // toUp.forEach(symb => { + // if (isSymbNested) { + // const toRemove = symb.parent().components().at(index); + // toRemove && toRemove.remove(opts); + // // } + + // // // In case of nested symbols, I only need to propagate changes to its instances + // // if (symb.__isSymbolTop() && symb.__getSymbols()) { + // // const toUpInst = symb.__getSymbToUp({ + // // fromInstance: m, + // // ...toUpOpts + // // }); + // // this.__logSymbol('remove-inst', toUpInst, { + // // opts: o, + // // symbol: symb + // // }); + // // toUpInst.forEach(inst => { + // // inst.remove(opts); + // // }); + // } else { + // symb.remove(opts); + // } + // }); } // Remove instance reference from the symbol diff --git a/test/specs/dom_components/model/Symbols.js b/test/specs/dom_components/model/Symbols.js index fac39cd40..f36bb6181 100644 --- a/test/specs/dom_components/model/Symbols.js +++ b/test/specs/dom_components/model/Symbols.js @@ -384,6 +384,7 @@ describe('Symbols', () => { test('Adding the instance, of the second symbol, inside the first symbol, propagates correctly to all first instances', () => { const added = symbol.append(secComp)[0]; + expect(added.__isSymbolNested()).toBe(true); // The added component is still the second instance expect(added).toBe(secComp); // The added component still has the reference to the second symbol @@ -412,6 +413,18 @@ describe('Symbols', () => { ); }); + test('Adding the instance, of the second symbol, inside one of the first instances, and then removing it, will not affect second instances outside', () => { + const secComp2 = createSymbol(secComp); + const added = comp.append(secComp)[0]; + expect(secComp2.__isSymbolNested()).toBe(false); + const secInstans = secSymbol.__getSymbols(); + expect(secInstans.length).toBe(all.length + 1); // + 1 is secComp2 + // Remove the second instance, added in one of the first instances + added.remove(); + // All first symbols will remove their copy and only the secComp2 will remain + expect(secSymbol.__getSymbols().length).toBe(1); + }); + test('Moving the second instance inside first instances, propagates correctly to all other first symbols', () => { const added = comp.append(secComp)[0]; expect(added.parent()).toBe(comp); // extra checks