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

[docs] Overlaid example on v9.1 #203

Draft
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

charlieforward9
Copy link

@charlieforward9 charlieforward9 commented Feb 2, 2025

Uses changes from #162 to create an example of how to use EditableGeoJsonLayer using the overlaid approach, demonstrating a bug explained in #201, where the drawing modes do not work.

I was encountering the same problems in overlaid mode in v9.0.x as well.

Please review and advise.

@charlieforward9
Copy link
Author

After many hours attempting to learn and resolve the issue, I must step back and absorb the defeat of the day.

I tried bumping mjolnir to v3 and messing with it in this method:

_addEventHandlers() {
// @ts-expect-error accessing protected props
const {eventManager} = this.context.deck;
const {eventHandler} = this.state._editableLayerState;
for (const eventType of EVENT_TYPES) {
eventManager.on(eventType as any, eventHandler, {
// give nebula a higher priority so that it can stop propagation to deck.gl's map panning handlers
priority: 100
});
}
}

No matter my approach, the events would not fire... please enlighten me 😩

@charlieforward9 charlieforward9 changed the title [docs] Example overlaid on v9.1 [docs] Overlaid example on v9.1 Feb 2, 2025
@charlieforward9 charlieforward9 marked this pull request as draft February 2, 2025 22:55
@ondave
Copy link

ondave commented Feb 3, 2025

Having exactly the same problem.

I think the root cause is that the anyclick event does not get recognised. This gets it working (for me anyway, no gauarntee it works in the generic case on all devices):


_addEventHandlers() { 
   // @ts-expect-error accessing protected props 
   const {eventManager} = this.context.deck; 
   const {eventHandler} = this.state._editableLayerState; 
  
   const _EVENT_TYPES=EVENT_TYPES.concat(['click'])
   for (const eventType of _EVENT_TYPES) { 
     eventManager.on(eventType as any, eventHandler, { 
       // give nebula a higher priority so that it can stop propagation to deck.gl's map panning handlers 
       priority: 100 
     }); 
   } 
 }

_onclick(eventSrc){
   this._onanyclick(eventSrc);
}

@charlieforward9
Copy link
Author

charlieforward9 commented Feb 3, 2025

@ondave this snippet is not working for me on my PC using Chrome, can you add it to the example and contribute it to my PR so I can review it?

@ondave
Copy link

ondave commented Feb 4, 2025

Ah, yes, sorry, didn't read your exact use case close enough. I was having issues with a vanilla deck.gl setup, and that fix patches that case OK, where anyclick just doesn't get fired.

For the MapboxOverlay case, I don't think the click events ever get to the deck instance eventManager. The MapboxOverlay captures the events and forwards the events directly to the deck.ts _onEvent function, but the EditableLayers events are registered in the deck instance eventManager.

Just out of interest did the EditableLayers (formerly nebula.gl, and pre deck v9) ever work with the MapboxOverlay?

I think one of the core dev team would have a better idea about all this.

@charlieforward9
Copy link
Author

charlieforward9 commented Feb 4, 2025

Just out of interest did the EditableLayers (formerly nebula.gl, and pre deck v9) ever work with the MapboxOverlay?

I am not sure of this, but this is what I would like to get working in v9.1+.

I think one of the core dev team would have a better idea about all this.

Patiently waiting 🧘 (though the blame shows most was just forked by @ibgreen from the disbanded nebula team)

ibgreen and others added 18 commits February 8, 2025 18:02
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
 - @deck.gl-community/[email protected]
@laurence-hudson-mindfoundry

@charlieforward9 Have you had any progress finding a solution to this? I appear to be getting bitten by the same issue (EditableGeoJsonLayer not working if following the overlay approach); as speculated above I dont think this related to the v9.1/mjolnir.js change.

@charlieforward9
Copy link
Author

@laurence-hudson-mindfoundry i have worked to keep my progress up to date on this branch, but still no success yet.

If you care to debug it with me, this branch is a good place to start

@laurence-hudson-mindfoundry

@charlieforward9 Have you had any progress finding a solution to this? I appear to be getting bitten by the same issue (EditableGeoJsonLayer not working if following the overlay approach); as speculated above I dont think this related to the v9.1/mjolnir.js change.

@charlieforward9 I've had a breakthrough for my own map. Using the interleaved overlay rendering variant (interleaved prop on MapboxOverlay) avoids the event swallowing. While interleaved has some consequences, at least for my app there bearable. So functioning EditableGeoJsonLayer while also retaining the react-map-gl as the outer component (for maplibre's NavigationControl / map api / etc).

@charlieforward9
Copy link
Author

Thanks for the update. Are you able to reproduce it in my example repo that this PR uses so I can build from that?

@laurence-hudson-mindfoundry
Copy link

laurence-hudson-mindfoundry commented Feb 28, 2025

Here's a minimal example using MapOverlay (interleaved true works, false doesn't). I struggled to get the dev env for the branch up and running, but here a codesandbox (it uses 9.0, which probably confirms that this sleeper bug has been around for a while).

https://codesandbox.io/p/sandbox/hz9vn9

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.

8 participants