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

Map.remove() fails when MapboxDraw control is present #820

Closed
vlad-shevchenko opened this issue Sep 17, 2018 · 3 comments
Closed

Map.remove() fails when MapboxDraw control is present #820

vlad-shevchenko opened this issue Sep 17, 2018 · 3 comments

Comments

@vlad-shevchenko
Copy link

vlad-shevchenko commented Sep 17, 2018

mapbox-gl-js version: 0.49.0
mapbox-gl-draw version: 1.0.9

Steps to Trigger Behavior

When trying to call Map.remove method on a map with a MapboxDraw control present, the call fails with the following stacktrace:

Uncaught TypeError: Cannot read property 'getLayer' of undefined
    at o.getLayer (map.js:1215)
    at mapbox-gl-draw.js:1
    at Array.forEach (<anonymous>)
    at Object.removeLayers (mapbox-gl-draw.js:1)
    at t.exports.onRemove (mapbox-gl-draw.js:1)
    at o.remove (map.js:1667)
    at <anonymous>:1:5

It reproduces on a clean example at https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-draw/ with only map.remove() line added in the end. JSBin: http://jsbin.com/zivobafume/1/edit?html,js,console,output

Consider the implementation of Map.remove method: https://github.com/mapbox/mapbox-gl-js/blob/master/src/ui/map.js#L1666

...
this.setStyle(null);
...
for (const control of this._controls) control.onRemove(this);
...

It clears style before calling controls' onRemove. And MapboxDraw's onRemove method depends on styles field. I suppose, it could be fixed by adding following line to the beginning of setup.removeLayers method:

if (!ctx.map.styles) return;

I'm not sure whether it's a MapboxDraw's bug, maybe I should've reported it to Mapbox. But I haven't faced the problem with any other plugin, so here we are.

For those who are also facing the issue, it can be workarounded by manually removing MapboxDraw control before calling map.remove():

map.removeControl(draw);
map.remove();

Expected Behavior

Map successfully removed

Actual Behavior

Unhandled TypeError

@nicoten
Copy link

nicoten commented Sep 17, 2018

Having the same issue - this was not listed as a breaking change but unfortunately it did break things. The workaround works but it would be nice to not need it.

@ansarikhurshid786
Copy link

// create control
var draw = mapboxgl.Draw({
    drawing: true,
    displayControlsDefault: false,
    controls: {
        polygon: true,
        trash: true
    }
});
// add control to map
map.addControl(draw);

// remove control from map
draw.remove()

@ansis
Copy link
Contributor

ansis commented Dec 18, 2018

This was an issue in mapbox-gl-js that has been fixed (mapbox/mapbox-gl-js#7479) and is released in the latest version of mapbox-gl-js.

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

No branches or pull requests

4 participants