diff --git a/src/dom_components/model/Component.js b/src/dom_components/model/Component.js index 03cb3b082..41bfdc84c 100644 --- a/src/dom_components/model/Component.js +++ b/src/dom_components/model/Component.js @@ -169,11 +169,11 @@ const Component = Backbone.Model.extend(Styleable).extend( this.opt = opt; this.em = em; this.config = opt.config || {}; - this.ccid = Component.createId(this); this.set('attributes', { ...(this.defaults.attributes || {}), ...(this.get('attributes') || {}) }); + this.ccid = Component.createId(this); this.initClasses(); this.initTraits(); this.initComponents(); @@ -1037,17 +1037,73 @@ const Component = Backbone.Model.extend(Styleable).extend( * @private */ createId(model) { + if (window.stoop) debugger; + let { id } = model.get('attributes'); + let nextId; + + if (id) { + nextId = Component.getIncrementId(id); + model.setId(nextId); + } else { + nextId = Component.getNewId(); + } + + componentList[nextId] = model; + return nextId; + }, + + getNewId() { componentIndex++; // Testing 1000000 components with `+ 2` returns 0 collisions const ilen = componentIndex.toString().length + 2; const uid = (Math.random() + 1.1).toString(36).slice(-ilen); - const nextId = 'i' + uid; - componentList[nextId] = model; - return nextId; + let newId = `i${uid}`; + while (componentList[newId]) newId = Component.getNewId(); + + return newId; + }, + + getIncrementId(id) { + let counter = 1; + let newId = id; + + while (componentList[newId]) { + counter++; + newId = `${id}-${counter}`; + } + + return newId; }, getList() { return componentList; + }, + + /** + * This method checks, for each parsed component and style object + * (are not Components/CSSRules yet), for duplicated id and fixes them + * + */ + checkId(components, styles = []) { + const comps = isArray(components) ? components : [components]; + comps.forEach(comp => { + const { attributes = {} } = comp; + const { id } = attributes; + + // Check if we have collisions with current components + if (id && componentList[id]) { + const newId = Component.getIncrementId(id); + attributes.id = newId; + // Update passed styles + styles && + styles.forEach(style => { + const { selectors } = style; + selectors.forEach((sel, idx) => { + if (sel === `#${id}`) selectors[idx] = `#${newId}`; + }); + }); + } + }); } } ); diff --git a/src/dom_components/model/Components.js b/src/dom_components/model/Components.js index 92943fb23..d1d732fac 100644 --- a/src/dom_components/model/Components.js +++ b/src/dom_components/model/Components.js @@ -1,6 +1,7 @@ import { isEmpty, isArray, isString } from 'underscore'; const Backbone = require('backbone'); +let Component; module.exports = Backbone.Collection.extend({ initialize(models, opt = {}) { @@ -36,6 +37,8 @@ module.exports = Backbone.Collection.extend({ const { em } = this; const cssc = em.get('CssComposer'); const parsed = em.get('Parser').parseHtml(value); + if (!Component) Component = require('./Component'); + Component.checkId(parsed.html, parsed.css); if (parsed.css && cssc && !opt.temporary) { cssc.addCollection(parsed.css, { diff --git a/test/specs/dom_components/model/Component.js b/test/specs/dom_components/model/Component.js index 7bb5331fb..cae379676 100644 --- a/test/specs/dom_components/model/Component.js +++ b/test/specs/dom_components/model/Component.js @@ -526,6 +526,7 @@ module.exports = { describe('Components', () => { beforeEach(() => { + em = new Editor({}); dcomp = new DomComponents(); compOpts = { componentTypes: dcomp.componentTypes @@ -549,6 +550,58 @@ module.exports = { var m = c.add({ type: 'text' }); expect(m instanceof ComponentText).toEqual(true); }); + + test('Avoid conflicting components with the same ID', () => { + const em = new Editor({}); + dcomp = new DomComponents(); + dcomp.init({ em }); + const id = 'myid'; + const block = ` +
Test
+ + `; + const added = dcomp.addComponent(block); + expect(dcomp.getComponents().length).toBe(1); + expect(added.getId()).toBe(id); + const cc = em.get('CssComposer'); + expect(cc.getAll().length).toBe(2); + expect( + cc + .getAll() + .at(0) + .selectorsToString() + ).toBe(`#${id}`); + expect( + cc + .getAll() + .at(1) + .selectorsToString() + ).toBe(`#${id}:hover`); + const added2 = dcomp.addComponent(block); + const id2 = added2.getId(); + const newId = `${id}-2`; + expect(id2).toBe(newId); + expect(cc.getAll().length).toBe(4); + expect( + cc + .getAll() + .at(2) + .selectorsToString() + ).toBe(`#${newId}`); + expect( + cc + .getAll() + .at(3) + .selectorsToString() + ).toBe(`#${newId}:hover`); + }); }); } }; diff --git a/test/specs/grapesjs/index.js b/test/specs/grapesjs/index.js index 48825694e..7517fe690 100644 --- a/test/specs/grapesjs/index.js +++ b/test/specs/grapesjs/index.js @@ -450,7 +450,7 @@ describe('GrapesJS', () => { expect(css).toEqual(`${protCss}.test2{color:red;}.test3{color:blue;}`); }); - describe('Component selection', () => { + describe.only('Component selection', () => { let editor, wrapper, el1, el2, el3; beforeEach(() => { @@ -465,6 +465,7 @@ describe('GrapesJS', () => { el1 = wrapper.find('#el1')[0]; el2 = wrapper.find('#el2')[0]; el3 = wrapper.find('#el3')[0]; + // console.log('wrapper', wrapper.getEl().innerHTML); }); test('Select a single component', () => {