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

Keyboard events are not handled when using maplibre-gl-js #1135

Closed
neodescis opened this issue Dec 28, 2022 · 3 comments · Fixed by #1165
Closed

Keyboard events are not handled when using maplibre-gl-js #1135

neodescis opened this issue Dec 28, 2022 · 3 comments · Fixed by #1165

Comments

@neodescis
Copy link
Contributor

Currently the keydown event handler is not working for maplibre. I realize maplibre may not be officially supported, but MapboxDraw mostly works with it. The problem is that the keydown handler currently looks for a CSS class on the canvas, in a specific order in classList:

if ((event.srcElement || event.target).classList[0] !== 'mapboxgl-canvas') return; // we only handle events on the map

taken from here: https://github.com/mapbox/mapbox-gl-draw/blob/main/src/events.js#L130

In maplibre, they've added an additional class in front of that one, so that the canvas class is: "maplibregl-canvas mapboxgl-canvas", which causes the check above to bail out of handling the events.

So, my question is, would you take a pull request that changes the above line of code to something along the lines of !classList.contains('mapboxgl-canvas') instead? Or it could even look for either of the two above classes. I'm not sure how long maplibre is planning to keep around the mapbox CSS classes.

maplibre/maplibre-gl-js#2006

maplibre-gl-js version: 2.4.0
mapbox-gl-draw version: 1.3.0

Steps to Trigger Behavior

  1. Select existing drawn shape
  2. Hit delete on the keyboard

Expected Behavior

Polygon is deleted

Actual Behavior

Polygon is not deleted

@benesg
Copy link

benesg commented Mar 1, 2023

I'm also using MapLibre and it would be really nice if this would be fixed so hotkeys are working.

@stepankuzmin
Copy link
Contributor

Hi @neodescis,

Thanks for opening the issue. I think we can use classList.contains here. Want to make a PR for that?

@neodescis
Copy link
Contributor Author

Hi @neodescis,

Thanks for opening the issue. I think we can use classList.contains here. Want to make a PR for that?

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants