-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
docs(mapbox) discoverability for integration examples #7734
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,8 +13,9 @@ This module makes it easy to use deck.gl as native layers and controls in the Ma | |
### Include the Standalone Bundle | ||
|
||
```html | ||
<script src="https://unpkg.com/deck.gl@^8.1.0/dist.min.js"></script> | ||
<script src='https://api.tiles.mapbox.com/mapbox-gl-js/v1.10.0/mapbox-gl.js'></script> | ||
<script src="https://unpkg.com/deck.gl@^8.9.0/dist.min.js"></script> | ||
<script src='https://unpkg.com/maplibre-gl@latest/dist/maplibre-gl.js'></script> | ||
<link href='https://unpkg.com/maplibre-gl@latest/dist/maplibre-gl.css' rel='stylesheet' /> | ||
<script type="text/javascript"> | ||
const {MapboxOverlay} = deck; | ||
</script> | ||
|
@@ -30,6 +31,17 @@ npm install @deck.gl/mapbox | |
import {MapboxOverlay} from '@deck.gl/mapbox'; | ||
``` | ||
|
||
## Examples | ||
|
||
[test/apps/mapbox-integration](https://github.com/visgl/deck.gl/tree/8.9-release/test/apps/mapbox-integration) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Link to master or release? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not link to test/apps from docs. These are not maintained with the mindset that they will be used by external users. If you think our existing examples are insufficient you can add more to examples/get-started There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, that makes sense. I'd like to maintain two public examples for the mapbox integration using interleaved MapboxOverlay with maplibre in react and pure-js since it's a common usage when someone reaches for @deck.gl/mapbox. |
||
|
||
| Source | API | Flavor | Base Map Library | | ||
| --- | --- | --- | --- | | ||
| `/mapbox-overlay` | [`MapboxOverlay`](https://github.com/visgl/deck.gl/blob/master/docs/api-reference/mapbox/mapbox-overlay.md) | pure-js | `mapbox-gl-js` | | ||
| `/mapbox-overlay-react` | [`MapboxOverlay`](https://github.com/visgl/deck.gl/blob/master/docs/api-reference/mapbox/mapbox-overlay.md) | react | `maplibre-gl-js`, `react-map-gl` | | ||
| `/mapbox-layers` | [`MapboxLayer`](https://github.com/visgl/deck.gl/blob/master/docs/api-reference/mapbox/mapbox-layer.md) | pure-js | `maplibre-gl-js` | | ||
| `/mapbox-layers-react` | [`MapboxLayer`](https://github.com/visgl/deck.gl/blob/master/docs/api-reference/mapbox/mapbox-layer.md) | react | `mapbox-gl-js`, `react-map-gl` | | ||
|
||
|
||
## Use Cases | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it's a good idea to modify the mapbox docs for maplibre (particularly keeping it in the "mapbox" folder, but then using maplibre).
While I prefer the FOSS nature of maplibre, the deck.gl integration is still called "mapbox" and probably focused on that.
mapbox and maplibre are currently drifting apart (already):
There might be some divergence in
MapboxOverlay
in the future, too, and it might eventually only support one of the projects (withMaplibreOverlay
as additional wrapper?).While many of the integrations should work similarly, I think it would be a good idea to explain usage of both (maplibre / mapbox) and to list what features are supported in each base-map, and which aren't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You bring up some good points to consider. We're currently focused on supporting both libraries with the same integration module since they're effectively drop-in replacements for most use cases.
Doing a hard split would add maintenance overhead to consider as well, since there are many things to test and most of the code would remain the same between the modules.
I agree showing both library options is probably more useful at this stage, as well as tracking the differences and limitations common to both, and unique to each.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's still too early to split and guess what kind of code should or could be factored out.
For now, I think there should just be a note that "Maplibre provides the Mapbox 1.x API and can be used with the MapboxOverlay with the following limitations / additional features:"
If MapLibre is used directly in the samples, there should probably be a comment that it (currently) provides the mapbox API so users see that this is in fact a mapbox example.