From c1bffbdef9140fe07daa18dcf58308f6092ee791 Mon Sep 17 00:00:00 2001
From: Artur Arseniev <artur.catch@hotmail.it>
Date: Sat, 14 Apr 2018 13:24:42 +0200
Subject: [PATCH] Fix CSS rule rendering with custom media rules

---
 src/css_composer/view/CssRulesView.js        | 44 +++++++++++++++-----
 test/specs/css_composer/view/CssRulesView.js | 27 ++++++++++--
 2 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/src/css_composer/view/CssRulesView.js b/src/css_composer/view/CssRulesView.js
index 892265a4f..01c4b102d 100644
--- a/src/css_composer/view/CssRulesView.js
+++ b/src/css_composer/view/CssRulesView.js
@@ -4,7 +4,7 @@ const $ = Backbone.$;
 
 // % is not a valid character for classes
 const getBlockId = (pfx, widthMedia) =>
-  `${pfx}rules-${widthMedia.replace('%', 'pc')}`;
+  `${pfx}${widthMedia ? `-${widthMedia.replace('%', 'pc')}` : ''}`;
 
 module.exports = require('backbone').View.extend({
   initialize(o) {
@@ -36,6 +36,11 @@ module.exports = require('backbone').View.extend({
    * @private
    * */
   addToCollection(model, fragmentEl) {
+    // If the render is not yet started
+    if (!this.renderStarted) {
+      return;
+    }
+
     var fragment = fragmentEl || null;
     var viewObject = CssRuleView;
     var config = this.config;
@@ -66,16 +71,30 @@ module.exports = require('backbone').View.extend({
       rendered = view.render().el;
     }
 
-    const mediaWidth = this.getMediaWidth(model.get('mediaText'));
-    const styleBlockId = getBlockId(this.pfx, mediaWidth);
+    const clsName = this.className;
+    const mediaText = model.get('mediaText');
+    const defaultBlockId = getBlockId(clsName);
+    let blockId = defaultBlockId;
+
+    // If the rule contains a media query it might have a different container
+    // for it (eg. rules created with Device Manager)
+    if (mediaText) {
+      blockId = getBlockId(clsName, this.getMediaWidth(mediaText));
+    }
 
     if (rendered) {
-      if (fragment) {
-        fragment.querySelector(`#${styleBlockId}`).appendChild(rendered);
-      } else {
-        let $stylesContainer = this.$el.find(`#${styleBlockId}`);
-        $stylesContainer.append(rendered);
+      const container = fragment || this.el;
+      let contRules;
+
+      // Try to find a specific container for the rule, if it
+      // containes a media query
+      try {
+        contRules = container.querySelector(`#${blockId}`);
+      } catch (e) {
+        contRules = container.querySelector(`#${defaultBlockId}`);
       }
+
+      contRules.appendChild(rendered);
     }
 
     return rendered;
@@ -91,13 +110,14 @@ module.exports = require('backbone').View.extend({
   },
 
   render() {
+    this.renderStarted = 1;
     this.atRules = {};
     const $el = this.$el;
     const frag = document.createDocumentFragment();
+    const className = this.className;
     $el.empty();
 
     // Create devices related DOM structure
-    const pfx = this.pfx;
     this.em
       .get('DeviceManager')
       .getAll()
@@ -108,12 +128,14 @@ module.exports = require('backbone').View.extend({
           ((left && left.replace('px', '')) || Number.MAX_VALUE)
       )
       .forEach(widthMedia => {
-        $(`<div id="${getBlockId(pfx, widthMedia)}"></div>`).appendTo(frag);
+        $(`<div id="${getBlockId(className, widthMedia)}"></div>`).appendTo(
+          frag
+        );
       });
 
     this.collection.each(model => this.addToCollection(model, frag));
     $el.append(frag);
-    $el.attr('class', this.className);
+    $el.attr('class', className);
     return this;
   }
 });
diff --git a/test/specs/css_composer/view/CssRulesView.js b/test/specs/css_composer/view/CssRulesView.js
index 5feeb93eb..a6f81bc49 100644
--- a/test/specs/css_composer/view/CssRulesView.js
+++ b/test/specs/css_composer/view/CssRulesView.js
@@ -6,7 +6,7 @@ module.exports = {
   run() {
     describe('CssRulesView', () => {
       let obj;
-      const prefix = 'rules-';
+      const prefix = 'rules';
       const devices = [
         {
           name: 'Mobile portrait',
@@ -62,8 +62,9 @@ module.exports = {
               ((left && left.replace('px', '')) || Number.MAX_VALUE)
             );
           });
-        foundStylesContainers.each(function($styleC, idx) {
-          expect($styleC.id).toEqual(prefix + sortedDevicesWidthMedia[idx]);
+        foundStylesContainers.each(($styleC, idx) => {
+          const width = sortedDevicesWidthMedia[idx];
+          expect($styleC.id).toEqual(`${prefix}${width ? `-${width}` : ''}`);
         });
       });
 
@@ -73,6 +74,26 @@ module.exports = {
         expect(obj.addToCollection.calledOnce).toExist(true);
       });
 
+      it('Add correctly rules with different media queries', () => {
+        const foundStylesContainers = obj.$el.find('div');
+        const rules = [
+          {
+            selectorsAdd: '#testid'
+          },
+          {
+            selectorsAdd: '#testid2',
+            mediaText: '(max-width: 1000px)'
+          },
+          {
+            selectorsAdd: '#testid2',
+            mediaText: 'screen and (max-width: 900px) and (min-width: 600px)'
+          }
+        ];
+        obj.collection.add(rules);
+        const stylesCont = obj.el.querySelector(`#${obj.className}`);
+        expect(stylesCont.children.length).toEqual(rules.length);
+      });
+
       it('Render new rule', () => {
         obj.collection.add({});
         expect(obj.$el.find(`#${prefix}`).html()).toExist();