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

Changes to enable float zoom levels #608

Merged
merged 22 commits into from
Jun 1, 2020

Conversation

omanges
Copy link
Contributor

@omanges omanges commented May 31, 2020

These changes fix the issue raised in #591

omanges and others added 22 commits March 16, 2020 19:09
@martinRenou
Copy link
Member

LGTM! I can squash and merge.

@martinRenou
Copy link
Member

The CI failure seems unrelated, but I am not sure

@martinRenou
Copy link
Member

Yes it's unrelated. It might be a new flake8 rule. Let's fix it in another PR.

@martinRenou martinRenou merged commit 3fb4459 into jupyter-widgets:master Jun 1, 2020
@davidbrochart
Copy link
Member

davidbrochart commented Jun 1, 2020

zoom_delta and zoom_snap are not used on the JS side?

@martinRenou
Copy link
Member

martinRenou commented Jun 1, 2020

It is :) It will be auto-magically handled by ipyleaflet, being an option (o=True).

@davidbrochart
Copy link
Member

Did you check that it works fine? I tried on a vector tile layer and it doesn't seem to work.

@omanges
Copy link
Contributor Author

omanges commented Jun 1, 2020 via email

@martinRenou
Copy link
Member

martinRenou commented Jun 1, 2020

Did you check that it works fine? I tried on a vector tile layer and it doesn't seem to work.

Actually you're right. Handling options changes is not implemented on the Map. This should be fixed.

We should have something like:

model_events() {
  let key;
  let options = this.model.get('options');
  for (var i = 0; i < o.length; i++) {
    key = options[i];
    this.listenTo(this.model, 'change:' + key, () => {
      L.setOptions(this.obj, this.get_options());
    }, this);
  }
}

In all base classes, we only have it in ControlView, not even in LayerView, which surprises me.

@davidbrochart
Copy link
Member

@omanges it looks like it works for everything but a vector tile layer.

@davidbrochart
Copy link
Member

My bad, I didn't pass the options on the Map but on the layer!
Everything's good, sorry!

@omanges
Copy link
Contributor Author

omanges commented Jun 1, 2020 via email

@omanges omanges deleted the float_zoom branch October 17, 2020 11:47
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.

3 participants