[WIP] Replace leaflet with mapbox-gl #122

gnestor · 2018-04-04T14:09:16Z

Closes #116

gnestor · 2018-04-04T14:12:49Z

I ran into a Typescript block:

In the @types/mapbox-gl, we have:

declare namespace mapboxgl {
    var accessToken: string;
    ...
}

In the extension, we have:

mapboxgl.accessToken = ACCESS_TOKEN;

And the Typescript error is: Cannot assign to 'accessToken' because it is a constant or a read-only property.

@ian-r-rose @blink1073 Any ideas?

ian-r-rose · 2018-04-04T14:30:58Z

According to this issue, ES6 module imports are immutable, and so you can't change the access token (even though it is declared using let in the typings). The fix they suggest is to change the import statement to an ES5 style:

import mapboxgl = require('mapbox-gl');

I tried this, and it seemed to fix the compilation error.

ryanbaumann · 2018-04-04T14:35:26Z

Let me know if you want any 👀 on this when ready for review! I'm the maintainer of mapboxgl-jupyter.

gnestor · 2018-04-04T15:11:49Z

Thanks @ian-r-rose!

@ryanbaumann Excellent! Please take a look now. It's rendering but all of the maps are blank:

You can probably debug must quicker than I 👍

ryanbaumann

@gnestor added comments to the PR - let me know if you have any questions. This is the example I'd recommend referencing https://www.mapbox.com/mapbox-gl-js/example/multiple-geometries/ as you build.

ryanbaumann · 2018-04-05T15:11:16Z

packages/geojson-extension/src/index.tsx

+      if (this._map) {
+        this._map.addLayer({
+          id: 'layer',
+          type: 'symbol',


I would recommend using a circle layer here for points, line for linestrings, and fill for polygons. You can create all three of these layers from the same geojson source, using filters to select only a certain geometry type to render in the layer. See https://www.mapbox.com/mapbox-gl-js/example/multiple-geometries/

ryanbaumann · 2018-04-05T15:11:54Z

packages/geojson-extension/src/index.tsx

+          type: 'symbol',
+          source: {
+            type: 'geojson',
+            data


Two items here:

I recommend creating all of your sources and layers you'll need for the user's map at the same time you create the base map object itself.

First create a geojson source (map.addSource) and a geojson layer (map.addLayer) from that source. Here's a super basic example that uses the same approach - https://gist.github.com/ryanbaumann/cbad147abda4fbff76bd95b386cea45b#file-index-html-L77-L98

Then in the renderModay loop, you'll just run map.setData(myGeojsonData) like this https://www.mapbox.com/mapbox-gl-js/api#geojsonsource#setdata

You'll need to pass some data to the data option in geojson format. See https://www.mapbox.com/mapbox-gl-js/example/multiple-geometries/

ryanbaumann · 2018-04-05T15:14:11Z

packages/geojson-extension/src/index.tsx

@@ -125,16 +87,22 @@ class RenderedGeoJSON extends Widget implements IRenderMime.IRenderer {
   * A message handler invoked on an `'after-attach'` message.
   */
  protected onAfterAttach(msg: Message): void {
+    this._map = new mapboxgl.Map({
+      container: this.node,
+      style: 'mapbox://styles/mapbox/light-v9',


Use mapbox://styles/mapbox/light-v9 ==> mapbox://styles/mapbox/light-v9?optimize=true for a slightly faster map to load. This removes any data that isn't used in the style.

ryanbaumann · 2018-04-05T15:15:07Z

packages/geojson-extension/src/index.tsx

+      container: this.node,
+      style: 'mapbox://styles/mapbox/light-v9',
+      minZoom: 0,
+      maxZoom: 18


I'd remove the minZoom and maxZoom setting here. The defaults will work great.

ryanbaumann · 2018-04-05T15:16:16Z

packages/geojson-extension/src/index.tsx

      // Enable scroll zoom on map focus
-      this._map.on('blur', (event) => {
-        this._map.scrollWheelZoom.disable();
+      this._map.on('blur', (event: Event) => {


There isn't a map blur event in GL JS. https://www.mapbox.com/mapbox-gl-js/api#events. By map focus do you mean full screen? https://www.mapbox.com/mapbox-gl-js/api#fullscreencontrol

The purpose here is to not allow the map to intercept scroll events when a user is scrolling the notebook. If the user wants to zoom the map, he/she can click on the map (focus) and then use the scroll wheel/trackpad. When the user clicks outside of the map (blur), scroll events will once again scroll the notebook.

ryanbaumann · 2018-04-05T15:16:32Z

packages/geojson-extension/src/index.tsx

      });
      // Disable scroll zoom on blur
-      this._map.on('focus', (event) => {
-        this._map.scrollWheelZoom.enable();
+      this._map.on('focus', (event: Event) => {


Likewise for line 100 comment above

ryanbaumann · 2018-04-05T15:17:26Z

packages/geojson-extension/src/index.tsx

-    if (this.isVisible) this._map.invalidateSize();
-    // Update map size after panel/window is resized
-    this._map.fitBounds(this._geoJSONLayer.getBounds());
+    if (this._map && this.isVisible) {


This will happen automatically in GL JS - the map container will resize based on the current div size of the notebook container. It can be removed

I tested this and it appears that it doesn't work automatically. One unique thing about phosphorjs (which is what provides JupyterLab it's Widgets among many other things) is that it can detect resize events on a node-level, so that when a panel is resized within lab, the affected widgets can handle those resizes themselves.

ryanbaumann · 2018-04-05T15:23:29Z

packages/geojson-extension/src/index.tsx

-      // Create GeoJSON layer from data and add to map
-      this._geoJSONLayer = leaflet.geoJSON(data).addTo(this._map);
-      this.update();
+      // Add GeoJSON layer to map


Before you add any layers or sources, you'll want to make sure the map style is loaded. See the basic example in the comment below for how to wait for the map.on('style.load') event.

gnestor · 2018-04-05T19:11:38Z

@ryanbaumann I've made the changes that you suggested AFAIK, but the map still appears the same as before. It would be great if you could install this repo locally and make the remaining necessary changes to get it working. To do this:

Follow the "Install" and "Linking" instructions in the README: https://github.com/jupyterlab/jupyter-renderers#contributing
Run jlpm watch in the jupyter-renderers root directory
In another tab, run JupyterLab with the following: jupyter lab --watch

This will allow you to test a locally installed geojson-extension, make changes, and automatically rebuild the extension and JupyterLab in response to those changes.

ryanbaumann

Added some comments - I won't have time to fork and check for several days, hope the comment helps!

ryanbaumann · 2018-04-06T01:46:46Z

packages/geojson-extension/src/index.tsx

@@ -69,14 +69,8 @@ class RenderedGeoJSON extends Widget implements IRenderMime.IRenderer {
    return new Promise<void>((resolve, reject) => {
      // Add GeoJSON layer to map
      if (this._map) {


I'd check to make sure the map exists here at any time when this function runs. Otherwise, this setData() function may never run, which is what adds the geojson data to the map.

gnestor · 2018-04-06T16:26:59Z

Ok, made some progress:

Is there a convenient way to fit the bounds of the GeoJSON data?

ryanbaumann · 2018-04-06T19:05:13Z

Amazing @gnestor this is going to be great!

To fit the bounds of your map to a geojson feature, I recommend using a subpackage of turf.js to calculate the bounds. Then use map.fitBounds() to set the map camera viewport.

Install:
npm install --save @turf/bbox
npm install --save mapboxgl-js

Use:

import turfBbox = require('@turf/bbox');
import mapboxgl = require('mapboxgl-js');
...
// Run this in the same spot where you add the geojson source data to the map, right after map.getSource('my-soruce').setData(myGeojsonData);

var bbox = turfBbox(myGeojsonData);
map.fitBounds( bbox, { padding: 100 } );

gnestor · 2018-04-06T23:21:05Z

Some progress but this flyover animation is very unnecessary...

ryanbaumann · 2018-04-07T00:38:32Z

@gnestor nice, glad that worked! I'd recommend setting the map center and zoom to fit the bounding box of the data. Here's how I'd recommend initializing the map to the bounds instead of flying to the bounds:

mapbox/mapbox-gl-js#1970 (comment)

gnestor · 2018-04-07T01:27:42Z

Ok, looking much better:

Now, what if I want to map a point on Mars? This was possible using leaflet by providing a different tileset via the url_template argument. Does mapbox-gl support raster tilesets?

ryanbaumann · 2018-04-07T01:42:57Z

@gnestor loving that the primary use case is a map on mars ;)

Yes, Mapbox GL JS supports a ton of layers, everything from Raster Tiles, to Vector Tiles, to Geojson, to Images, Video, and WebGL Canvas layers. Here's an example using a Raster tilemap, which you can simply drop in the URL to your raster tile layer and go (while getting all of the rendering benefits of GL JS for adding GeoJSON data to the map for data visualization) -

https://www.mapbox.com/mapbox-gl-js/example/map-tiles/

As for how to handle accepting different base maps, I'd recommend allowing the user to set a style, which can be a link to a Mapbox GL Style Sheet, or a locally created style sheet. You're already doing this with the mapbox://styles/mapbox/light-v9?optimize=true URL specified today.

A user could swap the style out for any mapbox:// URL, or a custom style sheet that has - for example - raster tiles of mars.

See https://github.com/mapbox/mapboxgl-jupyter/blob/master/mapboxgl/viz.py#L27 for an example of what that looks like in Mapboxgl-Jupyter

gnestor · 2018-05-01T21:32:54Z

@ryanbaumann Do you have any cycles available to polish this? I'm pretty busy at the moment but I'd like to get this merged 👍

ryanbaumann · 2018-05-02T01:04:25Z

@gnestor, not this week, unfortunately - I'll revisit next week.

davidbrochart · 2022-06-23T16:01:49Z

I guess since Mapbox switched to a proprietary license, mapbox-gl is not an option anymore.
I opened #276 which is based on MapLibre GL.

ryanbaumann suggested changes Apr 5, 2018

View reviewed changes

ryanbaumann suggested changes Apr 6, 2018

View reviewed changes

gnestor force-pushed the master branch from c4b76bd to ae9ad4c Compare April 6, 2018 16:04

gnestor added 4 commits April 6, 2018 09:04

Replace leaflet with mapbox-gl

7450f14

Fix Typescript error

7024c72

Update mapbox-gl implementation

987d92b

More updates

5895d09

gnestor force-pushed the mapbox-gl branch from de73bf3 to 5895d09 Compare April 6, 2018 16:26

Use turf.js to compute bounding box from geojson

3bda478

gnestor force-pushed the master branch from 60b72f4 to e155ac1 Compare April 17, 2018 21:43

astrojuanlu mentioned this pull request May 15, 2018

Improve plotting satellogic/telluric#7

Open

snyk-bot mentioned this pull request May 19, 2020

[Snyk] Upgrade vega-embed from 3.9.2 to 3.30.0 nottyboi513/jupyter-renderers#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Replace leaflet with mapbox-gl #122

[WIP] Replace leaflet with mapbox-gl #122

gnestor commented Apr 4, 2018

gnestor commented Apr 4, 2018

ian-r-rose commented Apr 4, 2018 •

edited

ryanbaumann commented Apr 4, 2018

gnestor commented Apr 4, 2018

ryanbaumann left a comment

ryanbaumann Apr 5, 2018

ryanbaumann Apr 5, 2018

ryanbaumann Apr 5, 2018

ryanbaumann Apr 5, 2018

ryanbaumann Apr 5, 2018

gnestor Apr 5, 2018

ryanbaumann Apr 5, 2018

ryanbaumann Apr 5, 2018

gnestor Apr 5, 2018

ryanbaumann Apr 5, 2018

gnestor commented Apr 5, 2018

ryanbaumann left a comment

ryanbaumann Apr 6, 2018

gnestor commented Apr 6, 2018

ryanbaumann commented Apr 6, 2018 •

edited

gnestor commented Apr 6, 2018

ryanbaumann commented Apr 7, 2018

gnestor commented Apr 7, 2018

ryanbaumann commented Apr 7, 2018

gnestor commented May 1, 2018

ryanbaumann commented May 2, 2018

davidbrochart commented Jun 23, 2022

[WIP] Replace leaflet with mapbox-gl #122

Are you sure you want to change the base?

[WIP] Replace leaflet with mapbox-gl #122

Conversation

gnestor commented Apr 4, 2018

gnestor commented Apr 4, 2018

ian-r-rose commented Apr 4, 2018 • edited

ryanbaumann commented Apr 4, 2018

gnestor commented Apr 4, 2018

ryanbaumann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnestor commented Apr 5, 2018

ryanbaumann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnestor commented Apr 6, 2018

ryanbaumann commented Apr 6, 2018 • edited

gnestor commented Apr 6, 2018

ryanbaumann commented Apr 7, 2018

gnestor commented Apr 7, 2018

ryanbaumann commented Apr 7, 2018

gnestor commented May 1, 2018

ryanbaumann commented May 2, 2018

davidbrochart commented Jun 23, 2022

ian-r-rose commented Apr 4, 2018 •

edited

ryanbaumann commented Apr 6, 2018 •

edited