Improve mapbox.style description + use mapbox-gl v1.1.1

[etpinard]

@plotly/plotly_js @nicolaskruchten

[antoinerg]

This description is much clearer 👍

[antoinerg]

💃 from me

[etpinard]

More from @nicolaskruchten on slack:

what does this line mean? https://github.com/plotly/plotly.js/blob/master/src/plots/mapbox/layout_attributes.js#L141

Is this line up to date wrt to raster and image source types? https://github.com/plotly/plotly.js/blame/master/src/plots/mapbox/layout_attributes.js#L120

would be good to clarify this line if it only applies when sourcetype is geojson: https://github.com/plotly/plotly.js/blob/master/src/plots/mapbox/layout_attributes.js#L143

---

these will be addressed in this PR.

[nicolaskruchten]

Would be good to point out that this is only needed for certain values of layout.mapbox.style: https://github.com/plotly/plotly.js/blob/master/src/plots/mapbox/layout_attributes.js#L40

[nicolaskruchten]

The relationship between layers.type and layers.sourcetype is pretty nebulous to me at the moment... Would be good to clarify.

[nicolaskruchten]

The attr for choroplethmapbox.below here https://github.com/plotly/plotly.js/blob/master/src/traces/choroplethmapbox/attributes.js#L63 should be expanded a bit to say something like "the default value is 'water', which is the ID of a layer that is often present in Mapbox styles. If no layer with this ID is implicitly or explicitly defined via layout.mapbox.style or explicitly present in layout.mapbox.layers then this trace will be positioned as usual."

... or something. that's basically a clearer description of the current behaviour and assumptions IMO.

[etpinard]

The attr for choroplethmapbox.below here https://github.com/plotly/plotly.js/blob/master/src/traces/choroplethmapbox/attributes.js#L63 should be expanded a bit to say something like "the default value is 'water', which is the ID of a layer that is often present in Mapbox styles.

That's not exactly true, here's the logic:

plotly.js/src/traces/choroplethmapbox/plot.js

Lines 106 to 132 in 14133af

	proto.getBelow = function(optsAll) {
	if(optsAll.below !== undefined) {
	return optsAll.below;
	}
	var mapLayers = this.subplot.map.getStyle().layers;
	var out = '';
	// find layer just above top-most "water" layer
	for(var i = 0; i < mapLayers.length; i++) {
	var layerId = mapLayers[i].id;
	if(typeof layerId === 'string') {
	var isWaterLayer = layerId.indexOf('water') === 0;
	if(out && !isWaterLayer) {
	out = layerId;
	break;
	}
	if(isWaterLayer) {
	out = layerId;
	}
	}
	}
	return out;
	};

The default below value is the layer just above the last water* layer. It's unfortunate that mapbox-gl doesn't have an above attribute, it would have been easier to describe.

[etpinard]

Based off this branch's latest commit

https://github.com/plotly/plotly.js/blob/ca762e96d9f8ea77966cfe4918c292e48661f3b0/src/plots/mapbox/layout_attributes.js

here are some TODOs that will done on this branch:

• Improve "mapbox token error message" mapbox token error message #4059 - done in a460be0

• rewrite mapbox.style description as:

Defines the map layers that are rendered by default below the trace layers defined in `data`, 
which are themselves by default rendered below the layers defined in `layout.mapbox.layers`.

These layers can be defined either explicitly as a Mapbox Style object which can contain multiple 
layer definitions that load data from any public or private Web Map Service (WMS), or implicitly 
by using one of the builtin plotly.js style objects which use WMSes which do not require any 
access tokens, or by using a  default Mapbox style or custom Mapbox style URL, both of 
which require a Mapbox access token.

Mapbox Style objects are of the form described in the Mapbox GL JS documentation available at
https://docs.mapbox.com/mapbox-gl-js/style-spec

The built-in plotly.js styles objects are... (provide name and the tile source)

The built-in Mapbox styles are...

Mapbox style URLs are of the form...

done in b933528

• mention new non-mapbox-server style shortcuts (once @antoinerg 's work is merged)

done in b933528

• Clarify relationship between layers.type and layers.sourcetype - done in d81f28c

• anything else?

[nicolaskruchten]

This is already a big set of improvements, thank you very much :)

One small adjustment to my initial proposal: the phrase public or private Web Map Service (WMS) should be expanded to say public or private Tile Map Service (TMS or XYZ) or Web Map Service (WMS)

[etpinard]

@plotly/plotly_js @nicolaskruchten this PR is now reviewable again.

Note that I merged master (with @antoinerg 's attributions and non-mapbox styles work) and I bumped mapbox-gl to v1.1.1 - so this is essentially on-track to be the "last" mapbox PR for 1.49.0 😏

[nicolaskruchten]

what does sourcetype have to be if source is a URL?

[nicolaskruchten]

And actually I would leave the word "tile" in there, as it's not super clear what the URL should point to otherwise. We should try to say what the non-array URL should point to also: an image? a GeoJSON file?

[etpinard]

Now this is:

plotly.js/src/plots/mapbox/layout_attributes.js

Lines 137 to 142 in 29c2bf9

	'Sets the source data for this layer (mapbox.layer.source).',
	'When `sourcetype` is set to *geojson*, `source` can be a URL to a GeoJSON',
	'or a GeoJSON object.',
	'When `sourcetype` is set to *vector* or *raster*, `source` can be a URL or',
	'an array of tile URLs.',
	'When `sourcetype` is set to *image*, `source` can be a URL to an image.'

[nicolaskruchten]

Suggested change

	'With `sourcetype` set to *vector*, *circle*, *line*, *fill* and *symbol* are available.',
	'With `sourcetype` set to *vector*, the following values are allowed: *circle*, *line*, *fill* and *symbol*.',

[etpinard]

Now this is:

plotly.js/src/plots/mapbox/layout_attributes.js

Lines 170 to 178 in 29c2bf9

	'Sets the layer type,',
	'that is the how the layer data set in `source` will be rendered',
	'With `sourcetype` set to *geojson*, the following values are allowed:',
	'*circle*, *line*, *fill* and *symbol*.',
	'but note that *line* and *fill* are not compatible with Point',
	'GeoJSON geometries.',
	'With `sourcetype` set to *vector*, the following values are allowed:',
	' *circle*, *line*, *fill* and *symbol*.',
	'With `sourcetype` set to *raster* or `*image*`, only the *raster* value is allowed.'

[nicolaskruchten]

💃 thanks very much, this is a huge step up in terms of users' ability to bootstrap their understanding of how to do things :)