From d2198ca6340a33ba9f6faa88f527e17eb575a97b Mon Sep 17 00:00:00 2001 From: Artur Arseniev Date: Thu, 21 Dec 2017 21:22:28 +0100 Subject: [PATCH] Fix class removing from Components. Fixes #661 --- src/commands/index.js | 4 ++-- src/dom_components/model/Component.js | 13 +++++++++++-- src/dom_components/view/ComponentView.js | 11 ++++------- src/selector_manager/view/ClassTagView.js | 2 -- src/selector_manager/view/ClassTagsView.js | 9 +-------- src/style_manager/view/SectorsView.js | 4 ++-- test/specs/dom_components/model/Component.js | 7 +++++++ test/specs/dom_components/view/ComponentV.js | 13 ++++++++++++- test/specs/selector_manager/view/ClassTagView.js | 10 +--------- 9 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/commands/index.js b/src/commands/index.js index 9467924bd..aa7ab7b5f 100644 --- a/src/commands/index.js +++ b/src/commands/index.js @@ -139,7 +139,7 @@ module.exports = () => { var collection = sel.collection; var index = collection.indexOf(sel); collection.add(sel.clone(), {at: index + 1}); - ed.trigger('component:update', sel); + sel.emitUpdate() }, }; @@ -164,7 +164,7 @@ module.exports = () => { const onEnd = (e, opts) => { em.runDefault(); em.set('selectedComponent', sel); - ed.trigger('component:update', sel); + sel.emitUpdate() dragger && dragger.blur(); }; diff --git a/src/dom_components/model/Component.js b/src/dom_components/model/Component.js index 5118eb1a2..b23535f8b 100644 --- a/src/dom_components/model/Component.js +++ b/src/dom_components/model/Component.js @@ -168,6 +168,8 @@ module.exports = Backbone.Model.extend(Styleable).extend({ this.initComponents(); this.initToolbar(); this.set('status', ''); + this.listenTo(this.get('classes'), 'add remove change', + () => this.emitUpdate('classes')); this.init(); }, @@ -282,7 +284,7 @@ module.exports = Backbone.Model.extend(Styleable).extend({ */ getAttributes() { const classes = []; - const attributes = this.get('attributes') || {}; + const attributes = { ...this.get('attributes') }; // Add classes this.get('classes').each(cls => classes.push(cls.get('name'))); @@ -723,7 +725,14 @@ module.exports = Backbone.Model.extend(Styleable).extend({ }) return scr; - } + }, + + + emitUpdate(property) { + const em = this.em; + const event = 'component:update' + (property ? `:${property}` : ''); + em && em.trigger(event, this.model); + }, },{ diff --git a/src/dom_components/view/ComponentView.js b/src/dom_components/view/ComponentView.js index 189516685..550bf0ac6 100644 --- a/src/dom_components/view/ComponentView.js +++ b/src/dom_components/view/ComponentView.js @@ -56,14 +56,11 @@ module.exports = Backbone.View.extend({ * @private */ handleChange() { - var em = this.em; - if(em) { - var model = this.model; - em.trigger('component:update', model); + const model = this.model; + model.emitUpdate(); - for(var prop in model.changed) { - em.trigger('component:update:' + prop, model); - } + for (let prop in model.changed) { + model.emitUpdate(prop); } }, diff --git a/src/selector_manager/view/ClassTagView.js b/src/selector_manager/view/ClassTagView.js index 43216ce76..96bd42ef6 100644 --- a/src/selector_manager/view/ClassTagView.js +++ b/src/selector_manager/view/ClassTagView.js @@ -89,7 +89,6 @@ module.exports = require('backbone').View.extend({ */ changeStatus() { this.model.set('active', !this.model.get('active')); - this.target.trigger('targetClassUpdated'); }, @@ -107,7 +106,6 @@ module.exports = require('backbone').View.extend({ sel && sel.get & sel.get('classes').remove(model); coll && coll.remove(model); setTimeout(() => this.remove(), 0); - em && em.trigger('targetClassRemoved'); }, diff --git a/src/selector_manager/view/ClassTagsView.js b/src/selector_manager/view/ClassTagsView.js index ada424c2d..941973ac1 100644 --- a/src/selector_manager/view/ClassTagsView.js +++ b/src/selector_manager/view/ClassTagsView.js @@ -52,7 +52,7 @@ module.exports = Backbone.View.extend({ this.em = this.target; this.listenTo(this.target ,'change:selectedComponent',this.componentChanged); - this.listenTo(this.target, 'targetClassUpdated', this.updateSelector); + this.listenTo(this.target, 'component:update:classes', this.updateSelector); this.listenTo(this.collection, 'add', this.addNew); this.listenTo(this.collection, 'reset', this.renderClasses); @@ -190,8 +190,6 @@ module.exports = Backbone.View.extend({ stateChanged(e) { if(this.compTarget){ this.compTarget.set('state', this.$states.val()); - if(this.target) - this.target.trigger('targetStateUpdated'); this.updateSelector(); } }, @@ -219,11 +217,6 @@ module.exports = Backbone.View.extend({ compCls.add(model); var lenA = compCls.length; this.collection.add(model); - - if (lenA > lenB) { - target.trigger('targetClassAdded'); - } - this.updateStateVis(); } } diff --git a/src/style_manager/view/SectorsView.js b/src/style_manager/view/SectorsView.js index 89f03ade5..339c9bde6 100644 --- a/src/style_manager/view/SectorsView.js +++ b/src/style_manager/view/SectorsView.js @@ -19,10 +19,10 @@ module.exports = Backbone.View.extend({ body.removeChild(dummy); this.propTarget = target; const coll = this.collection; + const events = 'change:selectedComponent component:update:classes change:device'; this.listenTo(coll, 'add', this.addTo); this.listenTo(coll, 'reset', this.render); - this.listenTo(this.target, 'change:selectedComponent targetClassAdded targetClassRemoved targetClassUpdated ' + - 'targetStateUpdated targetStyleUpdated change:device', this.targetUpdated); + this.listenTo(this.target, events, this.targetUpdated); }, diff --git a/test/specs/dom_components/model/Component.js b/test/specs/dom_components/model/Component.js index 5e2ca63d5..fb32d393f 100644 --- a/test/specs/dom_components/model/Component.js +++ b/test/specs/dom_components/model/Component.js @@ -198,6 +198,13 @@ module.exports = { expect(result.length).toEqual(2); }); + it('removeClass actually removes classes from attributes', () => { + obj.addClass('class1'); + obj.removeClass('class1'); + const result = obj.getAttributes(); + expect(result.class).toEqual(undefined); + }); + it('setAttributes', () => { obj.setAttributes({ id: 'test', diff --git a/test/specs/dom_components/view/ComponentV.js b/test/specs/dom_components/view/ComponentV.js index 77a464ae8..6265710cc 100644 --- a/test/specs/dom_components/view/ComponentV.js +++ b/test/specs/dom_components/view/ComponentV.js @@ -1,6 +1,7 @@ const ComponentView = require('dom_components/view/ComponentView'); const Component = require('dom_components/model/Component'); const DomComponents = require('dom_components'); +const Editor = require('editor/model/Editor'); module.exports = { run() { @@ -13,13 +14,16 @@ module.exports = { var hClass = 'hc-state'; var dcomp; var compOpts; + let em; beforeEach(() => { + em = new Editor({}); dcomp = new DomComponents(); compOpts = { + em, componentTypes: dcomp.componentTypes, }; - model = new Component(); + model = new Component({}, compOpts); view = new ComponentView({ model }); @@ -121,6 +125,13 @@ module.exports = { expect(view.$el.html()).toEqual('
'); }); + it('removeClass removes classes from attributes', () => { + model.addClass('class1'); + model.removeClass('class1'); + const result = model.getAttributes(); + expect(result.class).toEqual(undefined); + }); + }); } }; diff --git a/test/specs/selector_manager/view/ClassTagView.js b/test/specs/selector_manager/view/ClassTagView.js index f7179b827..e6afade2a 100644 --- a/test/specs/selector_manager/view/ClassTagView.js +++ b/test/specs/selector_manager/view/ClassTagView.js @@ -69,14 +69,6 @@ module.exports = { setTimeout(() => expect(fixtures.innerHTML).toNotExist(), 0) }); - it('On remove triggers event', () => { - var spy = sinon.spy(); - sinon.stub(obj.target, 'get').returns(0); - obj.target.on("targetClassRemoved", spy); - obj.$el.find('#close').trigger('click'); - expect(spy.called).toEqual(true); - }); - it('Checkbox toggles status', () => { var spy = sinon.spy(); obj.model.on("change:active", spy); @@ -89,7 +81,7 @@ module.exports = { it('On toggle triggers event', () => { var spy = sinon.spy(); sinon.stub(obj.target, 'get').returns(0); - obj.target.on("targetClassUpdated", spy); + obj.target.on("component:update:classes", spy); obj.$el.find('#checkbox').trigger('click'); expect(spy.called).toEqual(true); });