Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Update layer immediately when changing its max/min zoom level #11399

Merged
merged 4 commits into from
Apr 12, 2018

Conversation

LukasPaczos
Copy link
Contributor

Closes #11386.

@LukasPaczos LukasPaczos added Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl labels Mar 6, 2018
@LukasPaczos LukasPaczos added this to the android-v6.0.0 milestone Mar 6, 2018
@tobrun tobrun removed the Android Mapbox Maps SDK for Android label Mar 9, 2018
Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on @LukasPaczos! Changes look good. It would be nice to have tests that verify this behavior. There are a few prerequisites to be able to do that though. How do you feel about learning:

  • How to extend the node bindings so that they can exercise setMin/MaxZoom
  • How to add tests to our render test suite

I or someone else from the GL team would be happy to help walk you through these.

@LukasPaczos
Copy link
Contributor Author

Sounds great @jfirebaugh! Please let me know whenever you or anybody else has a moment to hit those bullet points with me.

Copy link
Contributor

@ivovandongen ivovandongen left a comment

Choose a reason for hiding this comment

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

👍

return Nan::ThrowTypeError("Second and third arguments must be numbers");
}

mbgl::style::Layer* layer = nodeMap->map->getStyle().getLayer(*Nan::Utf8String(info[0]));
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to check if the layer actually exists here.


void NodeMap::SetLayerZoomRange(const Nan::FunctionCallbackInfo<v8::Value>& info) {
using namespace mbgl::style;
using namespace mbgl::style::conversion;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think you need the conversion namespace here right?

@LukasPaczos LukasPaczos force-pushed the 11386-update-layer-with-zoom-change branch from 0404f2d to b9bffc2 Compare April 5, 2018 10:55
@LukasPaczos LukasPaczos merged commit 22b4ef1 into release-boba Apr 12, 2018
@LukasPaczos LukasPaczos deleted the 11386-update-layer-with-zoom-change branch April 12, 2018 08:42
LukasPaczos added a commit that referenced this pull request Apr 16, 2018
* [android][core] update layer immediately when changing it's max/min zoom

* [core] node bindings for layer zoom range

(cherry picked from commit 22b4ef1)
LukasPaczos pushed a commit that referenced this pull request Apr 16, 2018
) (#11687)

* [android][core] update layer immediately when changing it's max/min zoom

* [core] node bindings for layer zoom range

(cherry picked from commit 22b4ef1)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants