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
[Doc] Update Mapbox docs regarding deck v9 and react-map-gl v7 #8146
Conversation
7cd9f85
to
3a1f983
Compare
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.
Thanks for taking time to update the docs. I think the "three options" section in "Using with Mapbox" is getting quite confusing, and I would like to offer some further clarifications.
MapboxLayer
is no longer recommended. Its functionality can be fully replaced byMapboxOverlay
withinterleaved: true
. So the three options are:- Using
<DeckGL>
as the parent component of<Map>
- Using
<Map>
as the parent ofMapboxOverlay
, withinterleaved: false
- Using
<Map>
as the parent ofMapboxOverlay
, withinterleaved: true
- Using
- You should use option 2|3 if you want to use more mapbox-gl features, such as controls and terrain.
- You should use option 1 if you want to use more Deck features, such as multi-view and effects.
Thanks for looking into this ! Do you want me to try and make a new version taking your remarks into account ? Or do you prefer to take it from here ? Even if I can do it if you want, Feel free to use everything I wrote if you prefer, I don't need to be the author or anything. I was puzzled about MapboxLayer as well, and was actually going to open a separate issue once this one was finished to ask about it; good to know you already have the information. Am i correct to understand that you do not want to keep two separate examples of navigationcontrol : deck as root + react-map-gl 5 + NavigationControl, as well as Map as root + react-map-gl 7 + deck through MapboxOverlay ? And only keep the second one? Finally, one last question but maybe we can talk about this one in a separate issue (I was holding back like for the question of MapboxLayer): I think the following code is a bit weird, can it be simplified by offering a different API for mapboxOverlay ? Or does every project really need the few lines calling useControl and seemingly passing the props twice ? import {useControl} from 'react-map-gl';
function DeckGLOverlay(props: MapboxOverlayProps & {
interleaved?: boolean;
}) {
const overlay = useControl<MapboxOverlay>(() => new MapboxOverlay(props));
overlay.setProps(props);
return null;
}
// in jsx
<Map ... >
<DeckGLOverlay layers={[scatterplotLayer]} />
...
</Map> Cheers, |
If you don't mind, your effort is much appreciated.
We don't need to keep the react-map-gl v5 example.
You are hyper focused on your own use case here. |
Hi, I'm not sure but it feels like it's worth it to find a way to host this code somewhere, because using a simple deckgl data display on top of a basic map (and users expect basic maps to have navigation controls..) feels like a very common use case, not just my hyper focused use case (which is why you feel like I'm insisting I guess). I think it would be worth it to make Also, maybe a better implementation for DeckGLOverlay is function DeckGLOverlay(props) {
const overlay = useControl(() => new MapboxOverlay(props));
useEffect(() => {
overlay.setProps(props);
}, [props])
return null;
} Using useEffect makes it easier to see what's happening and avoids calling setProps (even if it is meant to be cheap I guess) when nothing has changed and components are just rendered because the parent has rendered. Looking at https://github.com/visgl/react-map-gl/blob/e11b41eefe73ff773bbec1db73277e4aca1f1bef/src/components/attribution-control.ts#L32, they also add in a memo() call (even though for this kind of component it's not very useful..), maybe add it too so that the DeckGLOverlay behaves more like other mapbox controls ? as a note for myself, if you decide to modify anything regarding DeckGLOverlay, your example https://github.com/visgl/react-map-gl/blob/master/examples/deckgl-overlay/src/app.tsx should be updated as well as the examples in this repo. Cheers, |
Make v7 control compatibility use cases more prominent
Which example with a react-map-gl basemap would you use to illustrate "You should use option 1 if you want to use more Deck features, such as multi-view and effects.". Or should we link to an example not using any of Deck's advanced features but still using DeckGL as the root and Map as the child ? Should it be in get-started or can we like to website examples ? Maybe this one ? examples/website/radio/app.jsx For the full list, I see
Should some of them be converted to deckOverlay if they are not using advanced deck features ? |
Remove all mentions of react-map-gl v5/v6, assume v7 use relative links for ../../api-reference no longer recommend MapboxLayer unless specific constraints require it
I've pushed a version taking your remarks into account, feel free to add your own ideas to it |
Why is this PR not merged yet? the current version of docs is very confusing since it's out of date... |
|
||
Mapbox provides an example of [finding the first label layer](https://www.mapbox.com/mapbox-gl-js/example/geojson-layer-in-stack/). For more sophisticated injection point lookups, refer to Mapbox' documentation on the format of Mapbox style layers, see [Mapbox Style Spec](https://www.mapbox.com/mapbox-gl-js/style-spec/#layers). | ||
|
||
|
||
In some cases, the application wants to add a deck.gl 3D layer (e.g. ArcLayer, HexagonLayer, GeoJsonLayer) on top of a Mapbox basemap, while seamlessly blend into the z-buffer. This will interleave the useful visualization layers from both the deck.gl and Mapbox layer catalogs. In this case, a `beforeId` is not needed. | ||
|
||
## Compatibility |
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 looked for the first release the WebGL2 compatibility commit appeared to make this table:
- Mapbox
useWebGl
flag: mapbox/mapbox-gl-js@d7652ba - Mapbox by default: mapbox/mapbox-gl-js@27aa250
- Maplibre by default: maplibre/maplibre-gl-js@7a1d5b5
I like the examples you've included in the documentation. As for the rest of the React website examples, they should be written without |
- If you don't use the most advanced features of Deck, such as multi-view and effects, and want to use all the features of mapbox-gl, such as controls like `NavigationControl` or plugins, then you have to use Mapbox as the root element. The recommended approach is then to use the Deck canvas as an overlay on top of the basemap, inserted into the map container using [MapboxOverlay](../../api-reference/mapbox/mapbox-overlay#using-with-react-map-gl) from the [@deck.gl/mapbox](../../api-reference/mapbox/overview.md) module in its default mode (overlaid, which corresponds to `interleaved: false`). The [react get-started example](https://github.com/visgl/deck.gl/tree/master/examples/get-started/react/mapbox/) illustrates this pattern. | ||
- Otherwise, if you need the more advanced features of Deck, then the recommended approach is to use Deck as the root element with its canvas as an overlay on top of the child Mapbox map. The [Minimap example](https://deck.gl/examples/multi-view) illustrates the basic pattern. This is the most tested and robust use case with respect to Deck's functionality, but you can't use all the features of mapbox-gl like controls (e.g. `NavigationControl`) and plugins. | ||
- Finally, if you need to mix deck.gl layers with base map layers, e.g. having deck.gl surfaces below text labels or objects occluding each other correctly in 3D, then you have to use deck.gl layers interleaved with Mapbox layers in the same WebGL context. In addition to using Mapbox as the root element (option 1), you have to use [MapboxOverlay](../../api-reference/mapbox/mapbox-overlay#using-with-react-map-gl) in interleaved mode (`interleaved: true`). Be cautious that this feature subjects to bugs and limitations of mapbox-gl's custom layer interface, and is only compatible with WebGL2 (See [compatibility](../../api-reference/mapbox/overview#compatibility)). Here's an [interactive example](https://deck.gl/examples/mapbox), and in the following image, notice that the yellow circles (first deck.gl layer) are between the ground (first mapbox layer) and the labels (second mapbox layer) and also below the buildings (third mapbox layer) which correctly occlude the arcs (second deck.gl layer) |
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.
Summary of Current Examples:
Option 1: Mapbox/Maplibre as root, Deck as overlay
- React: Using react-map-gl
<Map>
as the parent ofMapboxOverlay
, withinterleaved: false
- Mapbox: Getting Started with React + Mapbox
- Maplibre: Getting Started with React + Maplibre
- Pure-JS: Using vanilla
mapbox-gl
as the root element forMapboxOverlay
, withinterleaved: false
- Mapbox: Getting Started with Pure JS + Mapbox
- Maplibre: Getting Started with Pure JS + Maplibre
- Scripting: See option 2.
Option 2: Deck as root, Mapbox/Maplibre as child
- React: Using
<DeckGL>
as the parent component of<Map>
- Shown in most website examples... the Minimap example (aka radio) illustrates the pattern.
- Scripting: Using vanilla
DeckGL
as the parent component ofmapbox-gl
- Pure JS: See option 1.
Option 3: Mapbox/Maplibre as root, Deck layers interleaved in shared WebGL2 context.
- React: Using react-map-gl
<Map>
as the parent ofMapboxOverlay
, withinterleaved: true
- Mapbox: Mapbox Website example.
- Maplibre: Set
interleave: true
in the React getting-started examples in Option 1.
- Pure JS: Using vanilla
mapbox-gl
as the root element forMapboxOverlay
, withinterleaved: true
- Mapbox: mapbox-overlay in Gallery
- Maplibre: maplibre-overlay in Gallery
- Also, set
interleaved: true
in the Pure JS getting-started examples in Option 1.
- Scripting: Interleaved rendering not supported.
Co-authored-by: Ib Green <ib@foursquare.com>
Signed-off-by: Chris Gervang <chris@gervang.com>
Just pushed up a bunch of changes taking feedback and the deprecation of |
Signed-off-by: Chris Gervang <chris@gervang.com>
Signed-off-by: Chris Gervang <chris@gervang.com>
For #8541
Background
Hi, this started as a very light pull request but I've now included many changes which I think are improvement over the current docs. Let me know what you think. Please see #8147 for some background