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

Unexpected behaviour when calling "Map#addLayer" with invalid "before" argument #3868

Closed
iskandari opened this issue Dec 28, 2016 · 2 comments · Fixed by #5401
Closed

Unexpected behaviour when calling "Map#addLayer" with invalid "before" argument #3868

iskandari opened this issue Dec 28, 2016 · 2 comments · Fixed by #5401

Comments

@iskandari
Copy link

I am using the Mapbox GL example change a map's style.

When the style is switched to dark-v9, basic-v9, or streets-v9, existing layers reload with the map.on('style.load', function () {} This is expected.

However, this is not the case with satellite-v9 - when the satellite style is toggled, the existing layers load behind the style. Please see my example code

This is unexpected behavior. Why does this happen and Is there a workaround?

@iskandari iskandari reopened this Dec 28, 2016
@averas
Copy link
Contributor

averas commented Dec 28, 2016

The problem is that you are providing a before parameter to addLayer referencing a layer ("road-primary") that does exist in some of the styles but not in satellite.

One might assume that this would cause an exception, or at the very least fall back to the default behaviour of adding the layer at the end of the layer stack, however, the current implementation:

   const index = before ? this._order.indexOf(before) : this._order.length;
   this._order.splice(index, 0, id);

.. will actually place the the layer second to last when there is no hit (index of -1) on the before parameter. In your case that means that your layers are added before the last layer in the satellite style, which is the raster layer. Simply removing the "road-primary" parameter will make your code work: http://codepen.io/anon/pen/mONPJm

Although you did not use the API as documented I would argue that the current behaviour is not logical and can be improved (thrown exception or fallback to last as mentioned above).

@stevage
Copy link
Contributor

stevage commented Jan 19, 2018

FWIW (and I realise this may not be helpful feedback), I'm not enjoying this change. I'm often running into situations where an addLayer is failing because some other layer isn't present. This happens:

  • when I'm temporarily switching my base style out for something that loads faster (now my overlay layers don't get added at all)
  • when I'm trying out different base styles
  • when for some reason my layers didn't load in the order I anticipated
  • when my layers, their names, and ordering are all in a bit of flux

Basically, this change is slowing me down and feels pedantic. I would prefer the "before" param to be taken as a hint ("stick the new layer before layer X if you can, otherwise stick it at the end"). That is, the "...or..." option suggested by the OP here:

Either a hard fail, or add the layer to the end of the stack with a warning that the specified before doesn't exist.

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

Successfully merging a pull request may close this issue.

4 participants