Skip to content

Commit

Permalink
Merge pull request #5269 from plotly/mapbox_layer_order
Browse files Browse the repository at this point in the history
Fix reordering of mapbox raster/image layers on update
  • Loading branch information
archmoj authored Nov 13, 2020
2 parents a51b91f + 4054735 commit a382098
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 15 deletions.
36 changes: 24 additions & 12 deletions src/plots/mapbox/layers.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ proto.needsNewSource = function(opts) {
// stay safe and make new source on type changes
return (
this.sourceType !== opts.sourcetype ||
this.source !== opts.source ||
JSON.stringify(this.source) !== JSON.stringify(opts.source) ||
this.layerType !== opts.type
);
};
Expand All @@ -85,11 +85,23 @@ proto.needsNewLayer = function(opts) {
);
};

proto.lookupBelow = function() {
return this.subplot.belowLookup['layout-' + this.index];
};

proto.updateImage = function(opts) {
var map = this.subplot.map;
map.getSource(this.idSource).updateImage({
url: opts.source, coordinates: opts.coordinates
});

// Since the `updateImage` control flow doesn't call updateLayer,
// We need to take care of moving the image layer to match the location
// where updateLayer would have placed it.
var _below = this.findFollowingMapboxLayerId(this.lookupBelow());
if(_below !== null) {
this.subplot.map.moveLayer(this.idLayer, _below);
}
};

proto.updateSource = function(opts) {
Expand All @@ -107,29 +119,29 @@ proto.updateSource = function(opts) {
map.addSource(this.idSource, sourceOpts);
};

proto.updateLayer = function(opts) {
var subplot = this.subplot;
var convertedOpts = convertOpts(opts);

var below = this.subplot.belowLookup['layout-' + this.index];
var _below;

proto.findFollowingMapboxLayerId = function(below) {
if(below === 'traces') {
var mapLayers = subplot.getMapLayers();
var mapLayers = this.subplot.getMapLayers();

// find id of first plotly trace layer
for(var i = 0; i < mapLayers.length; i++) {
var layerId = mapLayers[i].id;
if(typeof layerId === 'string' &&
layerId.indexOf(constants.traceLayerPrefix) === 0
) {
_below = layerId;
below = layerId;
break;
}
}
} else {
_below = below;
}
return below;
};

proto.updateLayer = function(opts) {
var subplot = this.subplot;
var convertedOpts = convertOpts(opts);
var below = this.lookupBelow();
var _below = this.findFollowingMapboxLayerId(below);

this.removeLayer();

Expand Down
28 changes: 25 additions & 3 deletions test/jasmine/tests/mapbox_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -948,10 +948,15 @@ describe('@noCI, mapbox plots', function() {
layout: {
mapbox: {
layers: [{
'sourcetype': 'raster',
'source': ['https://a.tile.openstreetmap.org/{z}/{x}/{y}.png'],
'below': 'traces',
}, {
'sourcetype': 'image',
'coordinates': coords,
'source': source
}]
'source': source,
'below': 'traces',
}],
}
}
};
Expand All @@ -970,7 +975,7 @@ describe('@noCI, mapbox plots', function() {
Plotly.react(gd, makeFigure(redImage)).then(function() {
var mapbox = gd._fullLayout.mapbox._subplot;
map = mapbox.map;
layerSource = map.getSource(mapbox.layerList[0].idSource);
layerSource = map.getSource(mapbox.layerList[1].idSource);

spyOn(layerSource, 'updateImage').and.callThrough();
spyOn(map, 'removeSource').and.callThrough();
Expand All @@ -981,6 +986,23 @@ describe('@noCI, mapbox plots', function() {
{url: greenImage, coordinates: coords}
);
expect(map.removeSource).not.toHaveBeenCalled();

// Check order of layers
var mapbox = gd._fullLayout.mapbox._subplot;
var mapboxLayers = mapbox.getMapLayers();
var plotlyjsLayers = mapbox.layerList;

var indexLower = mapboxLayers.findIndex(function(layer) {
return layer.id === 'plotly-layout-layer-' + plotlyjsLayers[0].uid;
});

var indexUpper = mapboxLayers.findIndex(function(layer) {
return layer.id === 'plotly-layout-layer-' + plotlyjsLayers[1].uid;
});

expect(indexLower).toBeGreaterThan(-1);
expect(indexUpper).toBeGreaterThan(0);
expect(indexUpper).toBe(indexLower + 1);
})
.catch(failTest)
.then(done);
Expand Down

0 comments on commit a382098

Please sign in to comment.