diff --git a/NEWS.md b/NEWS.md index 13e20a1675..1c32416bd8 100644 --- a/NEWS.md +++ b/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 diff --git a/lib/assets/core/javascripts/cartodb3/data/legends/legend-bubble-definition-model.js b/lib/assets/core/javascripts/cartodb3/data/legends/legend-bubble-definition-model.js index 12a007b89c..dec2da2b23 100755 --- a/lib/assets/core/javascripts/cartodb3/data/legends/legend-bubble-definition-model.js +++ b/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; + } } }); diff --git a/lib/assets/core/javascripts/cartodb3/deep-insights-integration/legend-manager.js b/lib/assets/core/javascripts/cartodb3/deep-insights-integration/legend-manager.js index 9263cfe718..1cb8bae579 100755 --- a/lib/assets/core/javascripts/cartodb3/deep-insights-integration/legend-manager.js +++ b/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'); } diff --git a/lib/assets/core/test/spec/cartodb3/data/legends/legend-bubble-definition-model.spec.js b/lib/assets/core/test/spec/cartodb3/data/legends/legend-bubble-definition-model.spec.js index abbea55055..3e92ca8c63 100644 --- a/lib/assets/core/test/spec/cartodb3/data/legends/legend-bubble-definition-model.spec.js +++ b/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'); }); }); diff --git a/lib/assets/core/test/spec/cartodb3/deep-insights-integration/legend-manager.spec.js b/lib/assets/core/test/spec/cartodb3/deep-insights-integration/legend-manager.spec.js index e159e20194..fcac635135 100644 --- a/lib/assets/core/test/spec/cartodb3/deep-insights-integration/legend-manager.spec.js +++ b/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'}); }); });