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

"loading" as an attribute of TileLayer? #455

Closed
fitoprincipe opened this issue Jan 7, 2020 · 22 comments
Closed

"loading" as an attribute of TileLayer? #455

fitoprincipe opened this issue Jan 7, 2020 · 22 comments

Comments

@fitoprincipe
Copy link
Contributor

Hi all,

the new show_loading attribute added recently to the TileLayer class shows an animation each time any tile is loading, it is fine, but wouldn't it be better if a loading property (read only attribute) is set to True each time any tile is loading? Therefore we could make use of it our own way.

@fitoprincipe fitoprincipe changed the title "loading" as an attribute of TileLayer? "loading" as an attribute of TileLayer? Jan 7, 2020
@martinRenou
Copy link
Member

Hi! There is an on_load event that is triggered when the tiles finished loading. Something like:

def on_load(**kwargs):
    # Do something
    pass

my_tile_layer.on_load(on_load)

Maybe that could suit your needs?

@fitoprincipe
Copy link
Contributor Author

fitoprincipe commented Jan 7, 2020

Hi @martinRenou, I tried to make a separate widget that shows a red bar when loading tiles and a green bar when all tiles are loaded, but when you move, new tiles load but I need to "catch" this so I can make the bar red again. I though of using the map's events, but then I though it would be better to just have a loading attribute.

@martinRenou
Copy link
Member

I totally agree. It should not be too difficult to add. Would you like to take a shot? :)

@fitoprincipe
Copy link
Contributor Author

Yes, sure =) I will try

@fitoprincipe
Copy link
Contributor Author

Ok, I need some help @martinRenou ..

in leaflet.py:

class TileLayer(RasterLayer):
    ...
    loading = Bool(False).tag(sync=True, o=True)

then in TileLayer.js:

var LeafletTileLayerView = LeafletRasterLayerView.extend({
    leaflet_events: function () {
        LeafletTileLayerView.__super__.leaflet_events.apply(this, arguments);
        var that = this;
        this.obj.on('loading', function (e) {
            that.model.set('loading', true);  // this is how the attr center of Map is updated
            if (that.model.get('show_loading'))
                that.spinner = new Spinner().spin(that.map_view.el);
        });
        this.obj.on('load', function() {
           that.model.set('loading', false); // set it to false when finished loading..
            that.send({
                event: 'load'
            });
            if (that.model.get('show_loading'))
                that.spinner.stop();
        });
    },
})

Then I tested it with a listener:

m = ipyleaflet.Map()
lay = TileLayer(url='https://{s}.tile.opentopomap.org/{z}/{x}/{y}.png')
m.add_layer(lay)
def ob(change):
    print('loading')
lay.observe(ob, names='loading')

But it does not work. When logging in the console the value of loading in the JS side it updates properly, but does not link to the python side. If you think this approach is quite close, I can make a PR.

Thanks!

@martinRenou
Copy link
Member

Did you try doing

that.model.save_changes();

After that.model.set('loading', ...) ?

@fitoprincipe
Copy link
Contributor Author

I tried that indeed, but I will do it again just to be sure.

@martinRenou
Copy link
Member

BTW your Python code should look like:

class TileLayer(RasterLayer):
    ...
    loading = Bool(False).tag(sync=True)

Without the o=True because it is not a LeafletJS option

@martinRenou
Copy link
Member

And if you are using JupyterLab, you won't see the print from your ob function

@fitoprincipe
Copy link
Contributor Author

fitoprincipe commented Jan 9, 2020

Ok, I'll take out the o=True. I am on a classical notebook (not JupyterLab)

@fitoprincipe
Copy link
Contributor Author

Well, non of this changes make it work 🤔

@martinRenou
Copy link
Member

Would you mind opening a PR anyway? So that I can give it a try :)

@fitoprincipe
Copy link
Contributor Author

Another option, which is different but will archive what I am looking for, is to make an on_loading method for TileLayer. This is easier to make (I already did).

@martinRenou
Copy link
Member

Yes. I don't understand why the boolean does not update though... I guess if we go for the boolean we would get rid of the on_load method?

IMO the loading flag provides better homogeneity with other layers.

@fitoprincipe
Copy link
Contributor Author

With "loading flag" you mean the on_loading method or the loading attribute?

@martinRenou
Copy link
Member

I mean loading attribute :)

@fitoprincipe
Copy link
Contributor Author

fitoprincipe commented Jan 9, 2020

Ok, yes, I though it was a more consistent approach. But, I am not sure if removing on_load method is a good idea, it could be a breaking change for dependents 🤔

@martinRenou
Copy link
Member

So, it seems like the 'loading' boolean is correctly updated (I simulated a slow internet connection in order to have time to print the boolean before the tiles finished loading :P). It's the observation that does not seem to work 😕

@martinRenou
Copy link
Member

No! the observation works too! It's the print statement that is not put on the page 😕

If you try this code with your PR it works like a charm!

from ipyleaflet import *
from ipywidgets import Output

m = Map(center=(52, 10), zoom=8, basemap=basemaps.Hydda.Full)

x = m.layers[0]

out = Output()

@out.capture()
def on_load_change(change):
    print('loading? ', x.loading)

x.observe(on_load_change, 'loading')

display(m)
display(out)

@fitoprincipe
Copy link
Contributor Author

You are right! It does work 😄 👏

@martinRenou
Copy link
Member

Closed by #460 Thanks a lot!

This print statement that does not show must be unrelated.

@fitoprincipe
Copy link
Contributor Author

Thank you @martinRenou! I hope I can keep contributing =)

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

No branches or pull requests

2 participants