Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Jul 23, 2019
+226 −164
Merged

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

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
14ace10
improve `mapbox.style` description
etpinard Jul 11, 2019
ca762e9
improve `mapbox.*` descriptions (take 2)
etpinard Jul 12, 2019
1b15616
Merge branch 'master' into mapbox-style-better-docs
etpinard Jul 22, 2019
d4a2308
bump mapbox-gl to 1.1.1
etpinard Jul 22, 2019
a460be0
resolves #4059 - throw error msg about "missing style"
etpinard Jul 22, 2019
81f9d53
use styleValuesNonMapbox in mapbox.style attr declaration
etpinard Jul 22, 2019
b933528
mprove `mapbox.style` description (take 3)
etpinard Jul 22, 2019
d81f28c
improve mapbox.layers[i] sourcetype and type attr descriptions
etpinard Jul 22, 2019
29c2bf9
improve mapbox layer `source` and `type` descriptions (again)
etpinard Jul 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions src/plots/mapbox/layout_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ var attrs = module.exports = overrideAll({
description: [
'Sets the mapbox access token to be used for this mapbox map.',
'Alternatively, the mapbox access token can be set in the',
'configuration options under `mapboxAccessToken`.'
'configuration options under `mapboxAccessToken`.',
'Note that accessToken are only required when `style`',
nicolaskruchten marked this conversation as resolved.
Show resolved Hide resolved
'(e.g with values :', constants.styleValuesMapbox.join(', '), ')',
'and/or a layout layer references the Mapbox server.'
].join(' ')
},
style: {
Expand All @@ -48,10 +51,17 @@ var attrs = module.exports = overrideAll({
dflt: constants.styleValueDflt,
role: 'style',
description: [
'Sets the Mapbox map style.',
'Either input one of the default Mapbox style names or the URL to a custom style',
'or a valid Mapbox style JSON.',
'From OpenStreetMap raster tiles, use *open-street-map*.'
'Sets the base map style.',
'Base map styles are rendered below all traces and layout layers.',
etpinard marked this conversation as resolved.
Show resolved Hide resolved
'Base map styles can include multiple layers.',
'Either input one of the default Mapbox style names: ', constants.styleValuesMapbox.join(', '), '.',
'Note that to use these, a Mapbox access token must be set either in the `accesstoken` attribute',
'or in the `mapboxAccessToken` config option.',
'For OpenStreetMap raster tiles, use: ', constants.styleValueOSM, '.',
'No access token is required to render the', constants.styleValueOSM, 'style.',
'One can also set `style` as a URL to a Mapbox custom style, e.g. created in Mapbox Studio.',
'Finally, one can set `style` as a Mapbox style JSON, see',
etpinard marked this conversation as resolved.
Show resolved Hide resolved
'https://docs.mapbox.com/mapbox-gl-js/style-spec for more info.'
].join(' ')
},

Expand Down Expand Up @@ -117,7 +127,7 @@ var attrs = module.exports = overrideAll({
'Sets the source data for this layer (mapbox.layer.source).',
'Source can be either a URL,',
'a geojson object (with `sourcetype` set to *geojson*)',
'or an array of tile URLS (with `sourcetype` set to *vector*).'
'or an array of URLs (with `sourcetype` set to *vector* or *raster*).'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.'

].join(' ')
},

Expand All @@ -138,9 +148,11 @@ var attrs = module.exports = overrideAll({
role: 'info',
description: [
'Sets the layer type (mapbox.layer.type).',
'Support for *raster*, *background* types is coming soon.',
'Note that *line* and *fill* are not compatible with Point',
'GeoJSON geometries.'
'With `sourcetype` set to *geojson*, *circle*, *line*, *fill* and *symbol* are available',
etpinard marked this conversation as resolved.
Show resolved Hide resolved
'but note that *line* and *fill* are not compatible with Point',
'GeoJSON geometries.',
'With `sourcetype` set to *vector*, *circle*, *line*, *fill* and *symbol* are available.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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*.',

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.'

'With `sourcetype` set to *raster* or `*image*`, only the *raster* value is available.'
etpinard marked this conversation as resolved.
Show resolved Hide resolved
].join(' ')
},

Expand Down