From 52c95eef8490738dd2c237323917499740b1bd87 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Wed, 16 Nov 2016 20:20:13 -0800 Subject: [PATCH] Remove paint class support --- js/style/style.js | 29 ++++--- js/style/style_layer.js | 84 +++++++------------- js/ui/map.js | 101 ++---------------------- test/js/style/style.test.js | 2 +- test/js/style/style_layer.test.js | 123 ------------------------------ test/js/ui/map.test.js | 33 -------- 6 files changed, 48 insertions(+), 324 deletions(-) diff --git a/js/style/style.js b/js/style/style.js index bf96870c360..745a5d9eb4a 100644 --- a/js/style/style.js +++ b/js/style/style.js @@ -63,7 +63,7 @@ class Style extends Evented { this._loaded = true; this.stylesheet = stylesheet; - this.updateClasses(); + this.updatePaintProperties(); for (const id in stylesheet.sources) { this.addSource(id, stylesheet.sources[id], options); @@ -156,10 +156,9 @@ class Style extends Evented { return ids.map((id) => this._layers[id].serialize()); } - _applyClasses(classes, options) { + _applyPaintPropertyUpdates(options) { if (!this._loaded) return; - classes = classes || []; options = options || {transition: true}; const transition = this.stylesheet.transition || {}; @@ -170,10 +169,10 @@ class Style extends Evented { const props = this._updatedPaintProps[id]; if (this._updatedAllPaintProps || props.all) { - layer.updatePaintTransitions(classes, options, transition, this.animationLoop, this.zoomHistory); + layer.updatePaintTransitions(options, transition, this.animationLoop, this.zoomHistory); } else { for (const paintName in props) { - this._layers[id].updatePaintTransition(paintName, classes, options, transition, this.animationLoop, this.zoomHistory); + this._layers[id].updatePaintTransition(paintName, options, transition, this.animationLoop, this.zoomHistory); } } } @@ -242,7 +241,7 @@ class Style extends Evented { /** * Apply queued style updates in a batch */ - update(classes, options) { + update(options) { if (!this._changed) return; const updatedIds = Object.keys(this._updatedLayers); @@ -255,7 +254,7 @@ class Style extends Evented { this._reloadSource(id); } - this._applyClasses(classes, options); + this._applyPaintPropertyUpdates(options); this._resetUpdates(); this.fire('data', {dataType: 'style'}); @@ -369,7 +368,7 @@ class Style extends Evented { this._updatedSymbolOrder = true; } - this.updateClasses(id); + this.updatePaintProperties(id); } /** @@ -495,15 +494,15 @@ class Style extends Evented { return this.getLayer(layer).getLayoutProperty(name); } - setPaintProperty(layerId, name, value, klass) { + setPaintProperty(layerId, name, value) { this._checkLoaded(); const layer = this.getLayer(layerId); - if (util.deepEqual(layer.getPaintProperty(name, klass), value)) return; + if (util.deepEqual(layer.getPaintProperty(name), value)) return; const wasFeatureConstant = layer.isPaintValueFeatureConstant(name); - layer.setPaintProperty(name, value, klass); + layer.setPaintProperty(name, value); const isFeatureConstant = !( value && @@ -516,14 +515,14 @@ class Style extends Evented { this._updateLayer(layer); } - this.updateClasses(layerId, name); + this.updatePaintProperties(layerId, name); } - getPaintProperty(layer, name, klass) { - return this.getLayer(layer).getPaintProperty(name, klass); + getPaintProperty(layer, name) { + return this.getLayer(layer).getPaintProperty(name); } - updateClasses(layerId, paintName) { + updatePaintProperties(layerId, paintName) { this._changed = true; if (!layerId) { this._updatedAllPaintProps = true; diff --git a/js/style/style_layer.js b/js/style/style_layer.js index d6181cfabe5..f8ba8769e45 100644 --- a/js/style/style_layer.js +++ b/js/style/style_layer.js @@ -30,8 +30,8 @@ class StyleLayer extends Evented { this._layoutSpecifications = styleSpec[`layout_${this.type}`]; this._paintTransitions = {}; // {[propertyName]: StyleTransition} - this._paintTransitionOptions = {}; // {[className]: {[propertyName]: { duration:Number, delay:Number }}} - this._paintDeclarations = {}; // {[className]: {[propertyName]: StyleDeclaration}} + this._paintTransitionOptions = {}; // {[propertyName]: { duration:Number, delay:Number }} + this._paintDeclarations = {}; // {[propertyName]: StyleDeclaration} this._layoutDeclarations = {}; // {[propertyName]: StyleDeclaration} this._layoutFunctions = {}; // {[propertyName]: Boolean} @@ -39,14 +39,8 @@ class StyleLayer extends Evented { const options = {validate: false}; // Resolve paint declarations - for (const key in layer) { - const match = key.match(/^paint(?:\.(.*))?$/); - if (match) { - const klass = match[1] || ''; - for (paintName in layer[key]) { - this.setPaintProperty(paintName, layer[key][paintName], klass, options); - } - } + for (paintName in layer.paint) { + this.setPaintProperty(paintName, layer.paint[paintName], options); } // Resolve layout declarations @@ -93,44 +87,33 @@ class StyleLayer extends Evented { } } - setPaintProperty(name, value, klass, options) { - const validateStyleKey = `layers.${this.id}${klass ? `["paint.${klass}"].` : '.paint.'}${name}`; + setPaintProperty(name, value, options) { + const validateStyleKey = `layers.${this.id}.paint.${name}`; if (util.endsWith(name, TRANSITION_SUFFIX)) { - if (!this._paintTransitionOptions[klass || '']) { - this._paintTransitionOptions[klass || ''] = {}; - } if (value === null || value === undefined) { - delete this._paintTransitionOptions[klass || ''][name]; + delete this._paintTransitionOptions[name]; } else { if (this._validate(validateStyle.paintProperty, validateStyleKey, name, value, options)) return; - this._paintTransitionOptions[klass || ''][name] = value; + this._paintTransitionOptions[name] = value; } + } else if (value === null || value === undefined) { + delete this._paintDeclarations[name]; } else { - if (!this._paintDeclarations[klass || '']) { - this._paintDeclarations[klass || ''] = {}; - } - if (value === null || value === undefined) { - delete this._paintDeclarations[klass || ''][name]; - } else { - if (this._validate(validateStyle.paintProperty, validateStyleKey, name, value, options)) return; - this._paintDeclarations[klass || ''][name] = new StyleDeclaration(this._paintSpecifications[name], value); - } + if (this._validate(validateStyle.paintProperty, validateStyleKey, name, value, options)) return; + this._paintDeclarations[name] = new StyleDeclaration(this._paintSpecifications[name], value); } } - getPaintProperty(name, klass) { - klass = klass || ''; + getPaintProperty(name) { if (util.endsWith(name, TRANSITION_SUFFIX)) { return ( - this._paintTransitionOptions[klass] && - this._paintTransitionOptions[klass][name] + this._paintTransitionOptions[name] ); } else { return ( - this._paintDeclarations[klass] && - this._paintDeclarations[klass][name] && - this._paintDeclarations[klass][name].value + this._paintDeclarations[name] && + this._paintDeclarations[name].value ); } } @@ -200,30 +183,19 @@ class StyleLayer extends Evented { return false; } - updatePaintTransitions(classes, options, globalOptions, animationLoop, zoomHistory) { - const declarations = util.extend({}, this._paintDeclarations['']); - for (let i = 0; i < classes.length; i++) { - util.extend(declarations, this._paintDeclarations[classes[i]]); - } - + updatePaintTransitions(options, globalOptions, animationLoop, zoomHistory) { let name; - for (name in declarations) { // apply new declarations - this._applyPaintDeclaration(name, declarations[name], options, globalOptions, animationLoop, zoomHistory); + for (name in this._paintDeclarations) { // apply new declarations + this._applyPaintDeclaration(name, this._paintDeclarations[name], options, globalOptions, animationLoop, zoomHistory); } for (name in this._paintTransitions) { - if (!(name in declarations)) // apply removed declarations + if (!(name in this._paintDeclarations)) // apply removed declarations this._applyPaintDeclaration(name, null, options, globalOptions, animationLoop, zoomHistory); } } - updatePaintTransition(name, classes, options, globalOptions, animationLoop, zoomHistory) { - let declaration = this._paintDeclarations[''][name]; - for (let i = 0; i < classes.length; i++) { - const classPaintDeclarations = this._paintDeclarations[classes[i]]; - if (classPaintDeclarations && classPaintDeclarations[name]) { - declaration = classPaintDeclarations[name]; - } - } + updatePaintTransition(name, options, globalOptions, animationLoop, zoomHistory) { + const declaration = this._paintDeclarations[name]; this._applyPaintDeclaration(name, declaration, options, globalOptions, animationLoop, zoomHistory); } @@ -247,16 +219,14 @@ class StyleLayer extends Evented { 'minzoom': this.minzoom, 'maxzoom': this.maxzoom, 'filter': this.filter, - 'layout': util.mapObject(this._layoutDeclarations, getDeclarationValue) + 'layout': util.mapObject(this._layoutDeclarations, getDeclarationValue), + 'paint': util.mapObject(this._paintDeclarations, getDeclarationValue) }; - for (const klass in this._paintDeclarations) { - const key = klass === '' ? 'paint' : `paint.${klass}`; - output[key] = util.mapObject(this._paintDeclarations[klass], getDeclarationValue); - } - return util.filterObject(output, (value, key) => { - return value !== undefined && !(key === 'layout' && !Object.keys(value).length); + return value !== undefined && + !(key === 'layout' && !Object.keys(value).length) && + !(key === 'paint' && !Object.keys(value).length); }); } diff --git a/js/ui/map.js b/js/ui/map.js index 327a2e17b3f..30f3db2a66c 100755 --- a/js/ui/map.js +++ b/js/ui/map.js @@ -92,10 +92,6 @@ const defaultOptions = { * @param {number} [options.bearingSnap=7] The threshold, measured in degrees, that determines when the map's * bearing (rotation) will snap to north. For example, with a `bearingSnap` of 7, if the user rotates * the map within 7 degrees of north, the map will automatically snap to exact north. - * @param {Array} [options.classes] Mapbox style class names with which to initialize the map. - * Keep in mind that these classes are used for controlling a style layer's paint properties, so are *not* reflected - * in an HTML element's `class` attribute. To learn more about Mapbox style classes, read about - * [Layers](https://www.mapbox.com/mapbox-gl-style-spec/#layers) in the style specification. * @param {boolean} [options.attributionControl=true] If `true`, an [AttributionControl](#AttributionControl) will be added to the map. * @param {boolean} [options.failIfMajorPerformanceCaveat=false] If `true`, map creation will fail if the performance of Mapbox * GL JS would be dramatically worse than expected (i.e. a software renderer would be used). @@ -186,11 +182,8 @@ class Map extends Camera { }); } - this._classes = []; - this.resize(); - if (options.classes) this.setClasses(options.classes); if (options.style) this.setStyle(options.style); if (options.attributionControl) this.addControl(new AttributionControl()); @@ -199,7 +192,7 @@ class Map extends Camera { if (this.transform.unmodified) { this.jumpTo(this.style.stylesheet); } - this.style.update(this._classes, {transition: false}); + this.style.update({transition: false}); }); this.on('data', function(event) { @@ -248,85 +241,6 @@ class Map extends Camera { return this; } - /** - * Adds a Mapbox style class to the map. - * - * Keep in mind that these classes are used for controlling a style layer's paint properties, so are *not* reflected - * in an HTML element's `class` attribute. To learn more about Mapbox style classes, read about - * [Layers](https://www.mapbox.com/mapbox-gl-style-spec/#layers) in the style specification. - * - * @param {string} klass The style class to add. - * @param {StyleOptions} [options] - * @fires change - * @returns {Map} `this` - */ - addClass(klass, options) { - if (this._classes.indexOf(klass) >= 0 || klass === '') return this; - this._classes.push(klass); - this._classOptions = options; - - if (this.style) this.style.updateClasses(); - return this._update(true); - } - - /** - * Removes a Mapbox style class from the map. - * - * @param {string} klass The style class to remove. - * @param {StyleOptions} [options] - * @fires change - * @returns {Map} `this` - */ - removeClass(klass, options) { - const i = this._classes.indexOf(klass); - if (i < 0 || klass === '') return this; - this._classes.splice(i, 1); - this._classOptions = options; - - if (this.style) this.style.updateClasses(); - return this._update(true); - } - - /** - * Replaces the map's existing Mapbox style classes with a new array of classes. - * - * @param {Array} klasses The style classes to set. - * @param {StyleOptions} [options] - * @fires change - * @returns {Map} `this` - */ - setClasses(klasses, options) { - const uniqueClasses = {}; - for (let i = 0; i < klasses.length; i++) { - if (klasses[i] !== '') uniqueClasses[klasses[i]] = true; - } - this._classes = Object.keys(uniqueClasses); - this._classOptions = options; - - if (this.style) this.style.updateClasses(); - return this._update(true); - } - - /** - * Returns a Boolean indicating whether the map has the - * specified Mapbox style class. - * - * @param {string} klass The style class to test. - * @returns {boolean} `true` if the map has the specified style class. - */ - hasClass(klass) { - return this._classes.indexOf(klass) >= 0; - } - - /** - * Returns the map's Mapbox style classes. - * - * @returns {Array} The map's style classes. - */ - getClasses() { - return this._classes; - } - /** * Resizes the map according to the dimensions of its * `container` element. @@ -859,7 +773,6 @@ class Map extends Camera { * @param {string} name The name of the paint property to set. * @param {*} value The value of the paint propery to set. * Must be of a type appropriate for the property, as defined in the [Mapbox Style Specification](https://www.mapbox.com/mapbox-gl-style-spec/). - * @param {string=} klass A style class specifier for the paint property. * @returns {Map} `this` * @example * map.setPaintProperty('my-layer', 'fill-color', '#faafee'); @@ -867,8 +780,8 @@ class Map extends Camera { * @see [Adjust a layer's opacity](https://www.mapbox.com/mapbox-gl-js/example/adjust-layer-opacity/) * @see [Create a draggable point](https://www.mapbox.com/mapbox-gl-js/example/drag-a-point/) */ - setPaintProperty(layer, name, value, klass) { - this.style.setPaintProperty(layer, name, value, klass); + setPaintProperty(layer, name, value) { + this.style.setPaintProperty(layer, name, value); this._update(true); return this; } @@ -878,11 +791,10 @@ class Map extends Camera { * * @param {string} layer The ID of the layer to get the paint property from. * @param {string} name The name of a paint property to get. - * @param {string=} klass A class specifier for the paint property. * @returns {*} The value of the specified paint property. */ - getPaintProperty(layer, name, klass) { - return this.style.getPaintProperty(layer, name, klass); + getPaintProperty(layer, name) { + return this.style.getPaintProperty(layer, name); } /** @@ -1115,8 +1027,7 @@ class Map extends Camera { _render() { if (this.style && this._styleDirty) { this._styleDirty = false; - this.style.update(this._classes, this._classOptions); - this._classOptions = null; + this.style.update(); this.style._recalculate(this.transform.zoom); } diff --git a/test/js/style/style.test.js b/test/js/style/style.test.js index 5ad706e86ec..993c160cbca 100644 --- a/test/js/style/style.test.js +++ b/test/js/style/style.test.js @@ -1029,7 +1029,7 @@ test('Style#queryRenderedFeatures', (t) => { }); style.on('style.load', () => { - style._applyClasses([]); + style._applyPaintPropertyUpdates(); style._recalculate(0); t.test('returns feature type', (t) => { diff --git a/test/js/style/style_layer.test.js b/test/js/style/style_layer.test.js index 5bce3ea710a..3ec5ef72ffc 100644 --- a/test/js/style/style_layer.test.js +++ b/test/js/style/style_layer.test.js @@ -31,61 +31,6 @@ test('StyleLayer#updatePaintTransition', (t) => { t.end(); }); - t.test('updates paint transition with class', (t) => { - const layer = StyleLayer.create({ - "id": "background", - "type": "background", - "paint": { - "background-color": "red" - }, - "paint.mapbox": { - "background-color": "blue" - } - }); - layer.updatePaintTransition('background-color', ['mapbox'], {}); - t.deepEqual(layer.getPaintValue('background-color'), [0, 0, 1, 1]); - t.end(); - }); - - t.test('updates paint transition with extraneous class', (t) => { - const layer = StyleLayer.create({ - "id": "background", - "type": "background", - "paint": { - "background-color": "red" - } - }); - layer.updatePaintTransition('background-color', ['mapbox'], {}); - t.deepEqual(layer.getPaintValue('background-color'), [1, 0, 0, 1]); - t.end(); - }); - - t.end(); -}); - -test('StyleLayer#updatePaintTransitions', (t) => { - t.test('respects classes regardless of layer properties order', (t) => { - const layer = StyleLayer.create({ - "id": "background", - "type": "fill", - "paint.blue": { - "fill-color": "#8ccbf7", - "fill-opacity": 1 - }, - "paint": { - "fill-opacity": 0 - } - }); - - layer.updatePaintTransitions([], {transition: false}, null, createAnimationLoop()); - t.equal(layer.getPaintValue('fill-opacity'), 0); - - layer.updatePaintTransitions(['blue'], {transition: false}, null, createAnimationLoop()); - t.equal(layer.getPaintValue('fill-opacity'), 1); - - t.end(); - }); - t.end(); }); @@ -138,46 +83,6 @@ test('StyleLayer#setPaintProperty', (t) => { t.end(); }); - t.test('sets classed paint value', (t) => { - const layer = StyleLayer.create({ - "id": "background", - "type": "background", - "paint.night": { - "background-color": "red" - } - }); - - layer.setPaintProperty('background-color', 'blue', 'night'); - - t.deepEqual(layer.getPaintProperty('background-color', 'night'), 'blue'); - t.end(); - }); - - t.test('unsets classed paint value', (t) => { - const layer = StyleLayer.create({ - "id": "background", - "type": "background", - "paint": { - "background-color": "red", - "background-opacity": 1 - }, - "paint.night": { - "background-color": "blue", - "background-opacity": 0.1 - } - }); - layer.updatePaintTransitions(['night'], {transition: false}, null, createAnimationLoop()); - t.deepEqual(layer.getPaintProperty('background-color', 'night'), 'blue'); - t.deepEqual(layer.getPaintValue('background-color'), [0, 0, 1, 1]); - - layer.setPaintProperty('background-color', null, 'night'); - layer.updatePaintTransitions(['night'], {transition: false}, null, createAnimationLoop()); - t.deepEqual(layer.getPaintValue('background-color'), [1, 0, 0, 1]); - t.equal(layer.getPaintProperty('background-color', 'night'), undefined); - - t.end(); - }); - t.test('preserves existing transition', (t) => { const layer = StyleLayer.create({ "id": "background", @@ -211,21 +116,6 @@ test('StyleLayer#setPaintProperty', (t) => { t.end(); }); - t.test('sets transition with a class name equal to the property name', (t) => { - const layer = StyleLayer.create({ - "id": "background", - "type": "background", - "paint": { - "background-color": "red" - } - }); - - layer.setPaintProperty('background-color-transition', {duration: 400}, 'background-color'); - layer.updatePaintTransitions([], {transition: false}, null, createAnimationLoop()); - t.deepEqual(layer.getPaintProperty('background-color-transition', 'background-color'), {duration: 400}); - t.end(); - }); - t.test('emits on an invalid property value', (t) => { const layer = StyleLayer.create({ "id": "background", @@ -362,19 +252,6 @@ test('StyleLayer#serialize', (t) => { t.end(); }); - t.test('serializes layers with paint classes', (t) => { - const layer = createSymbolLayer({ - 'paint.night': { - 'text-color': 'orange' - } - }); - t.deepEqual( - StyleLayer.create(layer).serialize(), - layer - ); - t.end(); - }); - t.test('serializes functions', (t) => { const layerPaint = { 'text-color': { diff --git a/test/js/ui/map.test.js b/test/js/ui/map.test.js index b6d205885e6..eae23f610f6 100755 --- a/test/js/ui/map.test.js +++ b/test/js/ui/map.test.js @@ -546,39 +546,6 @@ test('Map', (t) => { map.removeControl(control); }); - t.test('#addClass', (t) => { - const map = createMap(); - map.addClass('night'); - t.ok(map.hasClass('night')); - t.end(); - }); - - t.test('#removeClass', (t) => { - const map = createMap(); - map.addClass('night'); - map.removeClass('night'); - t.ok(!map.hasClass('night')); - t.end(); - }); - - t.test('#setClasses', (t) => { - const map = createMap(); - map.addClass('night'); - map.setClasses([]); - t.ok(!map.hasClass('night')); - - map.setClasses(['night']); - t.ok(map.hasClass('night')); - t.end(); - }); - - t.test('#getClasses', (t) => { - const map = createMap(); - map.addClass('night'); - t.deepEqual(map.getClasses(), ['night']); - t.end(); - }); - t.test('#project', (t) => { const map = createMap(); t.deepEqual(map.project([0, 0]), { x: 100, y: 100 });