COMPASS-618: Update Mapbox with Fixes for Reverse Proxy. #1169
Conversation
There was a problem hiding this comment.
LGTM minus some small comments inline. Thanks!
I did notice though that the maps tiles didn't load occasionally, and required several restarts of Compass before I could see them. Is this a caching issue? Any idea how we can force bust the cache?
(The errors in the console seem unrelated)
| @@ -276,28 +278,6 @@ const minicharts_d3fns_geo = function() { | |||
| return this; | |||
| } | |||
|
|
|||
| function disableMapsFeature() { | |||
There was a problem hiding this comment.
Doesn't seem to have been introduced by this PR but it looks like I can't disabled the maps feature anymore via "Privacy Settings". Even when turned off, I still get maps.
Update: See COMPASS-1431 and #1170. @durran mind reviewing and backporting to 1.8-releases (COMPASS-1432) while you're in that code?
| // options.view.parent.render(); | ||
| } | ||
|
|
||
| function loadMapBoxScript(done) { |
There was a problem hiding this comment.
So glad this is gone. This was ugly.
| @@ -381,20 +348,20 @@ const minicharts_d3fns_geo = function() { | |||
|
|
|||
| // create the map once | |||
| if (!map) { | |||
| mapboxgl.accessToken = TOKEN; | |||
| map = new mapboxgl.Map({ | |||
| container: innerDiv[0][0], | |||
| // not allowed to whitelabel the map without enterprise license | |||
There was a problem hiding this comment.
not allowed to whitelabel ever due to OpenStreetMaps license. Also want to support this open source project. Could you update this comment while you're in that area?
(The attribution is now shown as a little info sprinkle that can be hovered over for more info).
| @@ -1,131 +0,0 @@ | |||
| /* eslint no-unused-expressions: 0 */ | |||
There was a problem hiding this comment.
why did we have to remove these tests? Didn't quite understand that. What Blobs?
There was a problem hiding this comment.
Mapbox has a dependency on the Javascript Blob object now (https://developer.mozilla.org/en/docs/Web/API/Blob) which is not available in Node. You can see Node's reasoning for not implementing here: nodejs/node#164
I'd like to revisit these tests at a later point and make it so Mapbox doesn't get loaded in them.
There was a problem hiding this comment.
I'll investigate today if maybe I can get them running in the renderer process instead.
* COMPASS-618: Bring in latest mapbox * Use mapbox uri * Update mapbox and nav control * Blobs are not supported in Node * Update license comment * Skip travis race condition
Everything loading fine now on mapbox-gl 0.39.1 (api, tiles, styles, sprites, etc)
This is based on my change to mapbox-gl-js: mapbox/mapbox-gl-js#4995
Unfortunately Blobs are not supported in Node and required the removal of 2 schema enzyme tests to get everything passing. We should revisit when finally extracting the schema plugin from Compass.