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

feat: Support for custom mapbox layer #1851

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ptbrowne
Copy link

@ptbrowne ptbrowne commented May 2, 2022

Sometimes it can be handy to insert a visualisation layer under another layer from mapbox (for example under the labels).

To do that, we can use DeckGL's custom mapbox layer that we insert between mapbox own layers.

Since DeckGL's custom layer need methods on the prototype (onAdd, onRemove, render), we cannot copy the layer props with spreading, otherwise we lose the methods. This is why the layer prop is added on the Layer so that we can pass the custom layer without any copying. This is also why there is some rework around beforeId so that it is external to the layer and only part of the Layer component props, not of the layer options props.

I'd like to have your opinion on this way of doing before continuing the work on this PR (maybe improving the documentation or the example).

Instead of using DeckGl as the container for Mapbox, we use Mapbox
as the container for DeckGl layer. This lets us insert the deckgl layer
between the mapbox layers, which is very handy when dealing with labels.

We use DeckGL's custom Mapbox layer here. Since DeckGL's custom layer
need method on the prototype, we can copy the layer props with spreading,
otherwise we lose the methods. This is why the `layer` prop is added
on the Layer so that we can pass the custom layer without any copying.
@Pessimistress
Copy link
Collaborator

Thank you for the proposal. To simplify things, I suggest we don't change the prop types for the existing use cases, and just add

type CustomLayer = {
  type: 'custom',
  layer: CustomLayerInterface
}

What do you think?

@sven-meyer-wetter-com
Copy link

Any news on that?

@Reichenberg
Copy link

I'm very much interested in seeing this merged. is there a way to help this along?

@Reichenberg
Copy link

Reichenberg commented Apr 17, 2023

@Pessimistress @ptbrowne

@stampycode
Copy link

Was trying to recreate in react-map-gl the example here to display a 3d model, but it looks like this isn't currently possible.

Is there a known workaround for this?

Many thanks!

// @ts-ignore
map.addLayer(options, props.beforeId);
map.addLayer(layerProps, beforeId);
}
}

/* eslint-enable complexity, max-statements */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove extra lines

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.

6 participants