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

Manage layer name and label seems to be a missing feature #696

Closed
ThomasG77 opened this issue May 8, 2013 · 8 comments
Closed

Manage layer name and label seems to be a missing feature #696

ThomasG77 opened this issue May 8, 2013 · 8 comments

Comments

@ThomasG77
Copy link
Contributor

Because there is no layer switcher at the moment (so not really a need for layer name and a label), layer name and label management are not implemented.
If this feature is really missing, getters/setters for optional name (system) and label (display title) at layer level have to be added.

@fredj
Copy link
Member

fredj commented May 8, 2013

Thanks to ol.Object this is already possible:

var layer = new ol.layer.TileLayer({
  source: new ol.source.OSM()
});

layer.set('name', 'osm layer');
layer.set('foobar', 42);

layer.get('name');
layer.get('foobar');

But it's not possible to add the key/value pair inside the constructor. eg:

var layer = new ol.layer.TileLayer({
  name: 'osm layer',
  foobar: 42,
  source: new ol.source.OSM()
});

see #690

@ThomasG77
Copy link
Contributor Author

Do you think it's better to close the issue and let the #690 deal with this case?

I also think that the layer id or name have to be mandatory.
It's because of the use case to find the right layers after map initialisation and without direct reference to a layer.
For example, I want to customize a layer after with map.getLayers().getArray(), I can't really find the right one and an array index is not the right way.

@twpayne
Copy link
Contributor

twpayne commented May 8, 2013

I think parts of #690 are sufficient here.

If/when we add a layer switcher control we can add the name and id properties to ol.layer.Layer with getters and setters. However, they should definitely not be mandatory because they are only needed by the layer switcher and making them mandatory would force everybody to add extra lines of code all the time.

If you need to customise a layer after adding it to the map, I suggest keeping a reference to the layer in an appropriate data structure for your application.

@elemoine
Copy link
Member

elemoine commented May 9, 2013

From experience I know it's very convenient for application or 3rd party lib developers to use the map as a place holder. More specifically it's very convenient to add layers, controls, interactions, whatever to a map and be able to retrieve references to these objects from the map. That makes it possible to pass one object only to modules / components: the map.

For example I think I'd like to be able to do this:

var map = new ol.map({
  layers: [
    new ol.source.osm({name: 'osm'})
  ]
});

And then later (in some other place in the application code):

var osmLayer = map.getLayers().getBy('name', {first: true});
osmLayer.setOpacity(0.5);

I think expect to be able to do this, and it makes sense to me because it enables good application architectures.

Thoughts?

@twpayne
Copy link
Contributor

twpayne commented May 9, 2013

OpenLayers 3 will rarely be the only JavaScript library loaded by the developer. Even some of our examples use jQuery.

There are hundreds of implementations of code to find an element in an array out there according to some predicate (jQuery's filter, Underscore's findWhere, Closure's goog.array.find, and so on and so on). I just do not see the advantage of adding a hundred-and-oneth implementation to the OpenLayers 3 API.

@elemoine
Copy link
Member

elemoine commented May 9, 2013

But we did add our own Array implementation, namely ol.Collection. I know why we did, but it can also be seen as a hundred-and-oneth Array wrapper.

For the record, with jQuery's grep function:

var osmLayer = jQuery.grep(map.getLayers().getArray(), function(layer) {
  return layer.get('name') == 'osm';
})[0];
osmLayer.setOpacity(0.5);

Don't know yet if I agree or disagree with you. One thing I know is that it's very useful to use the map as a placeholder, from which you can retrieve references to objects you passed to the Map constructor. You don't seem to disagree with that.

@twpayne
Copy link
Contributor

twpayne commented May 9, 2013

ol.Collection fires events when items are added and removed, JavaScript's native Array does not.

One thing I know is that it's very useful to use the map as a placeholder, from which you can retrieve references to objects you passed to the Map constructor. You don't seem to disagree with that.

I don't disagree. The underlying array of layers is available by calling getArray on the collection of layers. Instead of copy and pasting the tens of common functions that already exist that operate on arrays into methods on ol.Collection, I think we should just expose the low-level Array and let the user use whatever fits best with their application.

@elemoine
Copy link
Member

elemoine commented May 9, 2013

I can feel some good sense in what you're seeing.

I'm tempted to (re-)add a controls ol.Collection within the map, for the reason I mentioned above. But this would require managing the collection, i.e. remove/add controls from the controls collection when setMap is called.

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