Documented type of 'layer' parameter to listener functions not correct

[monetschemist]

I may be missing something here, but it seems to me that the type of the 'layer' argument to listener functions is not correct.

As far as I can tell from reading the current documentation, the layer parameter to the listener functions is either the layer just added (for bfl:layerloaded events) or the file name of the layer that could not be loaded (for the remaining bfl:* events). However this does not seem to be the case.

By trial and error using Object.getOwnPropertyNames() and reading the source code, I see that the bfl:layerloaded event listener function receives an argument that is a JavaScript object which has a field layer which is the real layer. For example, to write a listener function that registers the new layer with the Leaflet ListenerControl it's necessary to do something like this:

map.on('bfl:layerloaded', function(layer) {
    layerControl.addOverlay(layer.layer, layer.layer.options.name);
});

Similarly, for bfl:layerisempty and other error condition events, the listener function receives an argument that is a JavaScript object which has a field layer which is the real layer name. For example, to write a listener function that informs the user that a new layer is empty, it's necessary to do something like this:

map.on('bfl:layerisempty', function(layer) {
    alert(`Error: layer '${layer.layer}' is empty`);
});

It seems to me that either the documentation should be updated, and probably the argument structure passed to the listener functions should not be referred to as the layer parameter.

[gabriel-russo]

Hey! thanks for the feedback!

In the wiki you may find something like this:

map.on("bfl:layerisempty", ({layer}) => {
    console.log("Your uploaded layer do not have any features inside, please check it. Layer: ", layer)          
});

Now looking at it, it's not very intuitive... because you have to destructure a single key object (?)...

And about the why some events return the filename and others the object:

The way i thought making the function is:

• Any errors: get the user's filename that failed and then make some error alert using it
• Success: get the layer object to collect some metadata or put it in an array of added layers or something like that

I think i can improve both, the documentation and the code.~So, let me add some tasks to future me:~

~- [ ] Change the events responses from object of object to object.~ ~- [ ] Change in docs the parameter "layer" to "filename" in error events that returns only the filename.~ ~- [ ] Update docs after changes.~

thanks again for the feedback, anything more let me know.

Regards,

[monetschemist]

Hi @gabriel-russo I really like the plugin! And now that I understand how to get out the file name (for example to adjust the layer control widget, or to alert something meaningful), I'm 100% satisfied. So thanks a great deal for the excellent work.

Before anything else I have to emphasize that my JavaScript is pretty basic and the notation ({layer}) in the lambda is not something I was familiar with before you pointed it out in your kind response. But now I get it.

Of course it's your plugin, but given that it's already out there and I presume people are using it as-is, I'm not sure I would be inclined to change it - if someone updates their <script> includes to a newer version, their code will stop working. Also, if you use an object of object approach, you can put more metadata into it at a later date.

What seems to me to be useful, whether you make changes or not, is to provide a little more detail / example around just what that thing is coming into the handler. Could be examples with a lambda or with a function and then say logging or alerting with the file name, and using more suggestive names.

For example, if you stick with the current structure, you could call the parameter to bfl:layerloaded something like bflLoadEventData and then explain what its contents are. And with bfl:layerisempty and the other error events, you could use something like bflLoadErrorData, and then explain what its contents are.

Thinking about the specific event bfl:layerloaderror, maybe at some point you'll have more diagnostic info about what went wrong during the parse that you can put in the top-level object with a tag like exception or reason or cause. And of course with bfl:filesizelimit you could put in another tag like maxAllowedSize.

I hope these ideas are clear and offer you some value.

[gabriel-russo]

You're right! i can make these "problems" a feature, so i will update the tasks and make this issue a feature request

• [ ] Add more data in "layerloaded" event - like: user's input file extension, file size... or simply file metadata, timestamp (like onInsertTimeStamp).
• [ ] Add more diagnostic info in "layer load error"
• [ ] Throw the list of supported file types (read the option "formats") in "filenotsupported" event. Same to "filesizelimit", throw the size limit.
• [ ] Improve the docs with more use cases examples

Note: feel free to adding more contents

I will try to implement these in my free time... it will take some time to release

Again, thanks for the feedback!

Regards,

[monetschemist]

@gabriel-russo this all sounds good to me!

I think one of the most obvious use cases for the bfl:layerloaded event is adding the just-loaded layer to the layer control, which is one of the examples I sent you earlier. Thinking about changing the names a bit along the lines we have been discussing...

// create the OpenStreetMap layer, add it to the map

const osm = L.tileLayer('https://tile.openstreetmap.org/{z}/{x}/{y}.png', {
    maxZoom: 19,
    attribution: '&copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>'
});
osm.addTo(map);

// create the baseLayers and overlays structures for the layer control

var baseLayers = {
    'OpenStreetMap': osm
}
var overlays = {
    // empty for now, layers will be added by loading
}

// create the layer control and add it to the map

var layerControl = L.control.layers(baseLayers, overlays).addTo(map);

// add the bfl:layerloaded event listener and use it to update the layer control

map.on('bfl:layerloaded', function(layerLoadedData) {
    layerControl.addOverlay(layerLoadedData.layer, layerLoadedData.layer.options.name);
});