update default tile/style endpoints for mapbox styles #1317
Conversation
riastrad
force-pushed
the
riastrad-change-defaults
branch
from
c50a68c to
a82b112
Compare
map.js tests appear to have been broken since a40d000
riastrad
changed the title
riastrad
changed the title
|
👋 @mapbox/gl-js @mapbox/static-apis this branch changes some pretty core functionality, so would appreciate a close review from one or both of you. |
|
👋 @asheemmamoowala @mourner any chance you'll be able to review this before the end of the week? |
|
@riastrad yes, will do! |
src/tile_layer.js
Outdated
| return templated; | ||
| var tileURL; | ||
| if (L.Browser.retina && url.indexOf('/styles/v1') !== -1) { | ||
| tileURL = [templated.slice(0, templated.indexOf('?')), '@2x', templated.slice(templated.indexOf('?'))].join(''); |
There was a problem hiding this comment.
Nit: this could probably be written in a simpler way, maybe templated.replace('?', '@2x?')?
There was a problem hiding this comment.
Wow, yep - would make this much more readable. Switched to .replace in bd3b7bc
| if (typeof console === 'object' && | ||
| typeof console.warn === 'function') { | ||
| console.warn(_); | ||
| } |
There was a problem hiding this comment.
The context this is being used may cause hundreds of duplicate warnings to be printed — maybe worth adopting something like warnOnce from GL JS?
There was a problem hiding this comment.
Great point, implemented in 5a25f62
Resolves #1316
Up until now, Mapbox.js defaults have relied heavily on the Mapbox Legacy Static Tiles endpoints.
I'll mark this PR as ready for review when the following are implemented:
mapIDwill automatically load the equivalent style from the modern Static Tiles API.riastrad.{class style id}) is passed tomapobjects andtileLayerobjects.Logs a console warning about dropping support for the outdatedGiven that the url pattern for legacy & modern APIs is different, this appears to be unnecessary.tileLayer.setFormat()method with defaultmapIDs.If I identify any other usage of legacy APIs within the defaults, I will flag it and incorporate it within this PR before the review.
cc @mapbox/static-apis