Fix the import of components and styles with the same ids. Fixes Grap…

…esJS#932
momu-2016 · Mar 17, 2018 · 7621704 · 7621704
1 parent e6593e7
commit 7621704
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 15 deletions.
diff --git a/src/dom_components/model/Component.js b/src/dom_components/model/Component.js
@@ -323,12 +323,13 @@ const Component = Backbone.Model.extend(Styleable).extend(
 
       if (em && em.getConfig('avoidInlineStyle')) {
         prop = isString(prop) ? this.parseStyle(prop) : prop;
+        prop = { ...prop, ...this.get('style') };
         const state = this.get('state');
         const cc = em.get('CssComposer');
         const propOrig = this.getStyle();
         this.rule = cc.setIdRule(this.getId(), prop, { ...opts, state });
         const diff = shallowDiff(propOrig, prop);
-        this.set('style', {});
+        this.set('style', {}, { silent: 1 });
         keys(diff).forEach(pr => this.trigger(`change:style:${pr}`));
       } else {
         prop = Styleable.setStyle.apply(this, arguments);

diff --git a/src/dom_components/model/Components.js b/src/dom_components/model/Components.js
@@ -34,14 +34,13 @@ module.exports = Backbone.Collection.extend({
 
   add(models, opt = {}) {
     if (typeof models === 'string') {
-      var parsed = this.em.get('Parser').parseHtml(models);
+      const cssc = this.em.get('CssComposer');
+      const parsed = this.em.get('Parser').parseHtml(models);
       models = parsed.html;
 
-      var cssc = this.em.get('CssComposer');
-
       if (parsed.css && cssc) {
-        var { avoidUpdateStyle } = opt;
-        var added = cssc.addCollection(parsed.css, {
+        const { avoidUpdateStyle } = opt;
+        const added = cssc.addCollection(parsed.css, {
           extend: 1,
           avoidUpdateStyle
         });

diff --git a/src/parser/model/ParserCss.js b/src/parser/model/ParserCss.js
@@ -33,20 +33,28 @@ module.exports = config => ({
    * //}
    */
   parseSelector(str = '') {
-    var add = [];
-    var result = [];
-    var sels = str.split(',');
+    const add = [];
+    const result = [];
+    const sels = str.split(',');
+
     for (var i = 0, len = sels.length; i < len; i++) {
       var sel = sels[i].trim();
+
       // Will accept only concatenated classes and last
       // class might be with state (eg. :hover), nothing else.
-      if (/^(\.{1}[\w\-]+)+(:{1,2}[\w\-()]+)?$/gi.test(sel)) {
+      // Can also accept SINGLE ID selectors, eg. `#myid`, `#myid:hover`
+      // Composed are not valid: `#myid.some-class`, `#myid.some-class:hover`
+      if (
+        /^(\.{1}[\w\-]+)+(:{1,2}[\w\-()]+)?$/gi.test(sel) ||
+        /^(#{1}[\w\-]+){1}(:{1,2}[\w\-()]+)?$/gi.test(sel)
+      ) {
         var cls = sel.split('.').filter(Boolean);
         result.push(cls);
       } else {
         add.push(sel);
       }
     }
+
     return {
       result,
       add

diff --git a/src/selector_manager/index.js b/src/selector_manager/index.js
@@ -49,7 +49,10 @@
  * }
  */
 
-import { isString, isElement } from 'underscore';
+import { isString, isElement, isObject } from 'underscore';
+
+const isId = str => isString(str) && str[0] == '#';
+const isClass = str => isString(str) && str[0] == '.';
 
 module.exports = config => {
   var c = config || {},
@@ -121,23 +124,28 @@ module.exports = config => {
      * @param {String} name Selector name
      * @param {Object} opts Selector options
      * @param {String} [opts.label=''] Label for the selector, if it's not provided the label will be the same as the name
-     * @param {String} [opts.type='class'] Type of the selector. At the moment, only 'class' is available
+     * @param {String} [opts.type=1] Type of the selector. At the moment, only 'class' (1) is available
      * @return {Model}
      * @example
      * var selector = selectorManager.add('selectorName');
      * // Same as
      * var selector = selectorManager.add('selectorName', {
-     *   type: 'class',
+     *   type: 1,
      *   label: 'selectorName'
      * });
      * */
     add(name, opts = {}) {
-      if (typeof name == 'object') {
+      if (isObject(name)) {
         opts = name;
       } else {
         opts.name = name;
       }
 
+      if (isId(opts.name)) {
+        opts.name = opts.name.substr(1);
+        opts.type = Selector.TYPE_ID;
+      }
+
       if (opts.label && !opts.name) {
         opts.name = Selector.escapeName(opts.label);
       }
@@ -184,6 +192,10 @@ module.exports = config => {
      * var selector = selectorManager.get('selectorName');
      * */
     get(name, type = Selector.TYPE_CLASS) {
+      if (isId(name)) {
+        name = name.substr(1);
+        type = Selector.TYPE_ID;
+      }
       return selectors.where({ name, type })[0];
     },
 

diff --git a/test/specs/dom_components/index.js b/test/specs/dom_components/index.js
@@ -44,7 +44,9 @@ describe('DOM Components', () => {
     };
 
     beforeEach(() => {
-      em = new Editor();
+      em = new Editor({
+        avoidInlineStyle: 1
+      });
       config = {
         em,
         storeWrapper: 1
@@ -117,6 +119,30 @@ describe('DOM Components', () => {
     it('Render wrapper', () => {
       expect(obj.render()).toExist();
     });
+
+    it('Import propertly components and styles with the same ids', () => {
+      obj = em.get('DomComponents');
+      const cc = em.get('CssComposer');
+      const id = 'idtest';
+      const comp = obj.addComponent(`
+      <div id="${id}" style="color:red; padding: 50px 100px">Text</div>
+      <style>
+        #${id} { background-color: red }
+      </style>`);
+      expect(em.getHtml()).toEqual(`<div id="${id}">Text</div>`);
+      expect(obj.getComponents().length).toEqual(1);
+      obj
+        .getComponents()
+        .first()
+        .addStyle({ margin: '10px' });
+      expect(cc.getAll().length).toEqual(1);
+      expect(cc.getIdRule(id).getStyle()).toEqual({
+        color: 'red',
+        'background-color': 'red',
+        padding: '50px 100px',
+        margin: '10px'
+      });
+    });
   });
 
   ComponentModels.run();

diff --git a/test/specs/parser/model/ParserCss.js b/test/specs/parser/model/ParserCss.js
@@ -1,4 +1,5 @@
 const ParserCss = require('parser/model/ParserCss');
+const Selector = require('selector_manager/model/Selector');
 
 module.exports = {
   run() {
@@ -275,6 +276,35 @@ module.exports = {
         };
         expect(obj.parse(str)).toEqual(result);
       });
+
+      it('Parse ID rule', () => {
+        var str = `#test { color: red }`;
+        var result = {
+          selectors: ['#test'],
+          style: { color: 'red' }
+        };
+        expect(obj.parse(str)).toEqual(result);
+      });
+
+      it('Parse ID rule with state', () => {
+        var str = `#test:hover { color: red }`;
+        var result = {
+          selectors: ['#test'],
+          state: 'hover',
+          style: { color: 'red' }
+        };
+        expect(obj.parse(str)).toEqual(result);
+      });
+
+      it('Avoid composed selectors with ID', () => {
+        var str = `#test.class, #test.class:hover, .class  { color: red }`;
+        var result = {
+          selectors: ['class'],
+          selectorsAdd: '#test.class, #test.class:hover',
+          style: { color: 'red' }
+        };
+        expect(obj.parse(str)).toEqual(result);
+      });
     });
   }
 };
diff --git a/test/specs/selector_manager/index.js b/test/specs/selector_manager/index.js
@@ -45,6 +45,14 @@ describe('SelectorManager', () => {
       expect(obj.get('test').get('type')).toEqual(obj.Selector.TYPE_CLASS);
     });
 
+    it('Add a selector as an id', () => {
+      const name = 'test';
+      var sel = obj.add(`#${name}`);
+      expect(sel.get('name')).toEqual(name);
+      expect(sel.get('label')).toEqual(name);
+      expect(obj.get(`#${name}`).get('type')).toEqual(obj.Selector.TYPE_ID);
+    });
+
     it('Check name property', () => {
       var name = 'test';
       var sel = obj.add(name);