Skip to content
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

[mapbox] add MapboxOverlay #6738

Merged
merged 2 commits into from
Mar 24, 2022
Merged

[mapbox] add MapboxOverlay #6738

merged 2 commits into from
Mar 24, 2022

Conversation

Pessimistress
Copy link
Collaborator

This enables deck.gl to be used with react-map-gl v7.0 along with native controls. It also greatly simplifies the pure-js use case, whereas before the users were required to implement camera synchronization manually.

The MapboxOverlay implements Mapbox's IControl interface. When used, a deck canvas is inserted into Mapbox's controls container, and Mapbox handles all the user interaction. This works almost identically to the GoogleMapsOverlay class.

Change List

  • add MapboxOverlay
  • get-started example
  • docs

@coveralls
Copy link

coveralls commented Mar 7, 2022

Coverage Status

Coverage decreased (-0.4%) to 82.063% when pulling f85938a on x/mapbox-overlay into 70d55e1 on master.

@felixpalmer
Copy link
Collaborator

Great to see that the interface is now the same as the GoogleMapsOverlay, will simplify integrations where both can be supported, or swapped out.

One thing I wonder (as this is for future consideration, not this PR) is how we deal with the interleaved layers vs separate canvas overlay. Right now MapboxOverlay will always render into a separate overlaid canvas, while GoogleMapsOverlay will share a canvas (ignoring the legacy raster maps code path). I wonder if this could be made more consistent, either by:

  • Switching MapboxOverlay to share the canvas by using a MapboxLayer under the hood
  • Providing a prop in both overlays, e.g. interleaved to let the user decide

modules/mapbox/src/mapbox-overlay.js Outdated Show resolved Hide resolved
docs/api-reference/mapbox/mapbox-overlay.md Outdated Show resolved Hide resolved
@dmitov
Copy link

dmitov commented Mar 9, 2022

@kylebarron @Pessimistress do you guys need a hand with this MR? I'd be glad to help with getting this merged! ✌️

@Pessimistress
Copy link
Collaborator Author

Thanks @felixpalmer. I created issue #6773 to track follow up works.

@Pessimistress Pessimistress merged commit 94ade66 into master Mar 24, 2022
@Pessimistress Pessimistress deleted the x/mapbox-overlay branch March 24, 2022 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants