Merge pull request #11667 from CartoDB/bubbles-legend-issue

Fixed problem with Bubbles legend when a new analysis is applied

NEWS.md

@@ -56,7 +56,8 @@ Development
 * Migrate to use GNIP v2 for twitter search connector (#10051, #11595)
 
 ### Bug fixes
-* Fixed missing metadata option in header when dataset is sync
+* Fixed problem with Bubbles legend when a new analysis is applied (#11666)
+* Fixed missing metadata option in header when dataset is sync (#11458)
 * Fixed problem with dates when filtering time series widget
 * Fixed problem switching between qualitative and quantitative attributes (#10654)
 * Fixed problem found in Surfaces related with map panning and widgets filtering

lib/assets/core/javascripts/cartodb3/data/legends/legend-bubble-definition-model.js

@@ -1,22 +1,18 @@
 var _ = require('underscore');
 var LegendBaseDefModel = require('./legend-base-definition-model');
 var LegendColorHelper = require('../../editor/layers/layer-content-views/legend/form/legend-color-helper');
+var DEFAULT_BUBBLES_COLOR = '#999999';
 
 module.exports = LegendBaseDefModel.extend({
   defaults: _.extend({}, LegendBaseDefModel.prototype.defaults,
     {
       type: 'bubble',
-      fillColor: null,
+      fillColor: DEFAULT_BUBBLES_COLOR,
       topLabel: '',
       bottomLabel: ''
     }
   ),
 
-  initialize: function (attrs, opts) {
-    LegendBaseDefModel.prototype.initialize.call(this, attrs, opts);
-    this._initBinds();
-  },
-
   parse: function (r, opts) {
     var attrs = LegendBaseDefModel.prototype.parse.call(this, r);
 
@@ -32,17 +28,6 @@ module.exports = LegendBaseDefModel.extend({
     return attrs;
   },
 
-  _initBinds: function () {
-    this.listenTo(this.layerDefinitionModel, 'change:cartocss', this._onChangeStyle.bind(this));
-    this.listenTo(this.layerDefinitionModel.styleModel, 'style:update', this._onChangeStyle.bind(this));
-  },
-
-  _onChangeStyle: function () {
-    if (!this.layerDefinitionModel.get('autoStyle')) {
-      this.set('fillColor', this._inheritStyleColor(this.layerDefinitionModel.styleModel));
-    }
-  },
-
   toJSON: function () {
     return _.extend(
       {},
@@ -69,7 +54,12 @@ module.exports = LegendBaseDefModel.extend({
   _inheritStyleColor: function (styleModel) {
     var fill = styleModel.get('fill');
     var stroke = styleModel.get('stroke');
-    var color = fill.color || stroke.color;
-    return LegendColorHelper.getBubbles(color).color.fixed;
+    var color = (fill && fill.color) || (stroke && stroke.color);
+
+    if (color && (color.fixed || color.range)) {
+      return LegendColorHelper.getBubbles(color).color.fixed;
+    } else {
+      return DEFAULT_BUBBLES_COLOR;
+    }
   }
 });

lib/assets/core/javascripts/cartodb3/deep-insights-integration/legend-manager.js

@@ -1,5 +1,7 @@
 var _ = require('underscore');
 var LegendFactory = require('../editor/layers/layer-content-views/legend/legend-factory');
+var LegendColorHelper = require('../editor/layers/layer-content-views/legend/form/legend-color-helper');
+var DEFAULT_BUBBLES_COLOR = '#999999';
 
 var onChange = function (layerDefModel) {
   var styleModel = layerDefModel.styleModel;
@@ -62,7 +64,16 @@ var onChange = function (layerDefModel) {
 
   if (LegendFactory.isEnabledType('size')) {
     if (size && size.attribute !== undefined) {
-      LegendFactory.createLegend(layerDefModel, 'bubble', {title: size.attribute});
+      var attrs = { title: size.attribute };
+
+      if (!isAutoStyleApplied) {
+        // Do not apply the fill color of an autostyle ramp
+        attrs.fillColor = color && (color.fixed || color.range)
+          ? LegendColorHelper.getBubbles(color).color.fixed
+          : DEFAULT_BUBBLES_COLOR;
+      }
+
+      LegendFactory.createLegend(layerDefModel, 'bubble', attrs);
     } else {
       LegendFactory.removeLegend(layerDefModel, 'bubble');
     }

lib/assets/core/test/spec/cartodb3/data/legends/legend-bubble-definition-model.spec.js

@@ -18,6 +18,9 @@ describe('data/legends/legend-bubble-defintion-model', function () {
     });
 
     style = new Backbone.Model();
+    style.hasNoneStyles = function () {
+      return false;
+    };
     layerDef1.styleModel = style;
 
     this.model = new LegendDefinitionModel(null, {
@@ -27,23 +30,6 @@ describe('data/legends/legend-bubble-defintion-model', function () {
     });
 
     spyOn(this.model, '_inheritStyleColor');
-  });
-
-  describe('without autostyle', function () {
-    it('should _inheritStyleColor on custom autostyle', function () {
-      style.trigger('style:update');
-      expect(this.model._inheritStyleColor).toHaveBeenCalled();
-    });
-  });
-
-  describe('with autostyle', function () {
-    beforeEach(function () {
-      layerDef1.set('autoStyle', 'wadus');
-    });
-
-    it('should _inheritStyleColor on custom autostyle', function () {
-      style.trigger('style:update');
-      expect(this.model._inheritStyleColor).not.toHaveBeenCalled();
-    });
+    spyOn(this.model, 'save');
   });
 });

lib/assets/core/test/spec/cartodb3/deep-insights-integration/legend-manager.spec.js

@@ -100,7 +100,7 @@ describe('deep-insights-integrations/legend-manager', function () {
           cartocss: 'wadus'
         });
 
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#045275'});
       });
 
       it('styles color', function () {
@@ -124,7 +124,7 @@ describe('deep-insights-integrations/legend-manager', function () {
         });
 
         expect(LegendFactory.createLegend).toHaveBeenCalledTimes(2);
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#ffc6c4'});
         expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'choropleth', {title: 'number'});
       });
 
@@ -186,7 +186,7 @@ describe('deep-insights-integrations/legend-manager', function () {
         });
 
         expect(LegendFactory.removeLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'custom_choropleth');
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#045275'});
       });
 
       it('styles color', function () {
@@ -223,7 +223,7 @@ describe('deep-insights-integrations/legend-manager', function () {
 
         expect(LegendFactory.removeLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'custom_choropleth', jasmine.any(Function));
         expect(LegendFactory.createLegend).toHaveBeenCalledTimes(2);
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#ffc6c4'});
         expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'choropleth', {title: 'number'});
       });
 
@@ -295,7 +295,7 @@ describe('deep-insights-integrations/legend-manager', function () {
           cartocss: 'wadus'
         });
 
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#045275'});
       });
 
       it('styles color', function () {
@@ -319,7 +319,7 @@ describe('deep-insights-integrations/legend-manager', function () {
         });
 
         expect(LegendFactory.createLegend).toHaveBeenCalledTimes(2);
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#ffc6c4'});
         expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'choropleth', {title: 'number'});
       });
 
@@ -379,7 +379,7 @@ describe('deep-insights-integrations/legend-manager', function () {
           cartocss: 'sudaw'
         });
 
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#045275'});
       });
 
       it('styles color', function () {
@@ -416,7 +416,7 @@ describe('deep-insights-integrations/legend-manager', function () {
 
         expect(LegendFactory.removeLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'custom_choropleth', jasmine.any(Function));
         expect(LegendFactory.createLegend).toHaveBeenCalledTimes(2);
-        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+        expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#ffc6c4'});
         expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'choropleth', {title: 'number'});
       });
 
@@ -650,7 +650,7 @@ describe('deep-insights-integrations/legend-manager', function () {
       // only once to create a bubble legend, but no choropleth legend
 
       expect(LegendFactory.createLegend).toHaveBeenCalledTimes(1);
-      expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number'});
+      expect(LegendFactory.createLegend).toHaveBeenCalledWith(this.layerDefinitionModel, 'bubble', {title: 'number', fillColor: '#ffc6c4'});
       expect(LegendFactory.createLegend).not.toHaveBeenCalledWith(this.layerDefinitionModel, 'choropleth', {title: 'number'});
     });
   });