Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add ol.View2D#setCenterLatLng #1860

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
7 participants
Owner

elemoine commented Mar 14, 2014

This PR adds a setCenterLatLng and a centerLatLng option to ol.View2D as discussed in openlayers#457.

Owner

ahocevar commented Mar 14, 2014

+1 from me. If others also like this, and it gets merged eventually, I'll volunteer to update the other examples in a separate pull request.

Member

pagameba commented Mar 14, 2014

+1 this simplifies a very common use case.

Owner

elemoine commented Mar 14, 2014

I just added a commit that adds a getCenterLatLng function.

Owner

elemoine commented Mar 14, 2014

Thanks for the feedback. Other opinions? Note that I don't mind throwing this away.

Contributor

twpayne commented Mar 14, 2014

This one gets my spidey senses tingling. Although it covers a common beginner use case, I fear that it merely defers the problem. As soon as the user tries to position a popup on Washington DC at [38.5, -77.0], he will be be surprised when his overlay appears in Atlantic Ocean off the west coast of Africa. When the users handles a click event on Tokyo he will be surprised that event.coordinate contains [15440013.373027043, 4397372.7444622405] nothing like the [36.7, 138.7] he expects.

Member

marcjansen commented Mar 14, 2014

A valid concern, @twpayne. I totally agree. Yet I want stuff to be as easy accomplishable as possible. Could we somehow have a coordinate interceptor with some sensible defaults? Just thinking out loud here.

Owner

tschaub commented Mar 14, 2014

Redundant/repeated defaults are also a recipe for trouble.

I'm mixed on this. I agree it adds convenience that will be appreciated. But it also suggests we add calculateLatLngExtent, fitLatLngExtent, and other similar.

Contributor

twpayne commented Mar 14, 2014

Well said @tschaub.

One option is add LatLng versions of principal functions, and double the size and complexity of the ol3 API.

Another option is that, instead of giving a man a fish, we teach him how to fish.

Contributor

twpayne commented Mar 14, 2014

@marcjansen, #463 was an attempt to add a coordinate interceptor with sensible defaults.

Member

pagameba commented Mar 14, 2014

Good points against. On the other hand, not providing convenient APIs increases the complexity and size of application code that uses ol3.

colllin commented Mar 17, 2014

What about adding an optional second parameter to setCenter to specify the source projection?

map.getView().setCenter([38.5, -77.0], 'EPSG:4326')

Could make sense elsewhere, too:

map.getView().getCenter('EPSG:4326')
Contributor

twpayne commented Mar 17, 2014

center is an ol.Object-style observable property, which means we cannot add extra arguments to the getter and setter.

colllin commented Mar 17, 2014

Right, but it's wrapped in a setter function that could perform the necessary transformations?

Contributor

twpayne commented Mar 17, 2014

The setter functions are an integral part of the ol.Object behaviour. See src/ol/object.js.

colllin commented Mar 17, 2014

I'm sorry, maybe I'm lost... I'm talking about adding a second (optional) parameter to ol.View2D.prototype.setCenter. Is that what you're talking about?

ol.View2D.prototype.setCenter = function(center) {
    this.set(ol.View2DProperty.CENTER, center);
};

could become (hastily written):

ol.View2D.prototype.setCenter = function(center, opt_projection) {
    var newCenter = center;
    if (opt_projection) {
        newCenter = ol.proj.transform(newCenter, opt_projection, this.getProjection());
    }
    this.set(ol.View2DProperty.CENTER, newCenter);
};
Contributor

twpayne commented Mar 17, 2014

How would this work under property binding? Consider:

var view1 = new ol.View2D({
  projection: 'EPSG:1234',
  center: [56, 78]
});
var view2 = new ol.View2D();
view2.bindTo('center', view1);
view2.getCenter('EPSG:9876');

getCenter would somehow have to find out that the stored center values ([56, 78]) were in the EPSG:1234 projection and transform them to EPSG:9876. How could getCenter do this?

Owner

tschaub commented Mar 17, 2014

To be fair, it is only sensible to bind center between two views if they also have the same projection.

colllin commented Mar 17, 2014

@twpayne I hadn't come across property binding yet in my port from OL2 to OL3, but that looks very nice.

It's just an idea.

Maybe the reason it feels a little wrong to do all of your own transformations is that you already told the map what projection it's in. But before you give it some coordinates, you need to ask the map what projection it's in and transform your coordinates accordingly. If you give a map an [x, y] and a projection, it should be able to do the rest, no?

Then again, I'm so naïve when it comes to mapping that I don't even know if what I'm suggesting is logical.

Owner

elemoine commented Apr 18, 2014

I'm closing this for now, as discussed during some previous hangout meeting.

@elemoine elemoine closed this Apr 18, 2014

@elemoine elemoine deleted the elemoine:latlng branch Apr 27, 2015

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