emilhe / dash-leaflet

MIT License
213 stars 37 forks source link

ImageOverlay load event #182

Closed prl900 closed 1 year ago

prl900 commented 1 year ago

This is somehow related to issue #83 and the need to notify users when Leaflet is waiting for data to be loaded. In the case of ImageOverlay, an image is requested using a GET request to a defined URL with unknown wait times.

The original Leaflet.js API describes a load event in ImageOverlay which seems to suit this need: https://leafletjs.com/reference.html#imageoverlay-load

I can see events are mentioned in the dash-leaflet wrapper: https://github.com/thedirtyfew/dash-leaflet/blob/95d9f868cde0443e8093db356bd84886b96e642a/src/lib/components/ImageOverlay.react.js#L96

But I don't know enough about the internals of dash-leaflet to understand if this load event is available or how it can be used in Dash.

Thanks for your help and happy to turn this into a stackoverflow question if it's better suited.

emilhe commented 1 year ago

The event is currently not available from Dash. I guess it could be exposed as a load prop, possibly in combination with an n_load prop.

emilhe commented 1 year ago

I have just drafted a very simple implementation. I have added a new property loaded which is initialised to null. Once the image finishes loading, it it set to true. Would this implementation fit your use case? Here is a small example,

import json
import dash_leaflet as dl
from dash import Dash, html, Output, Input

lena_url = "https://upload.wikimedia.org/wikipedia/en/7/7d/Lenna_%28test_image%29.png?20111103160805"

app = Dash()
app.layout = html.Div([
    dl.Map([
        dl.TileLayer(),
        dl.ImageOverlay(url=lena_url, bounds=[(56, 10), (55, 13)], id="lena")
    ], style={'width': '100%', 'height': '50vh', 'margin': "auto", "display": "block"}),
    html.Div(id="log")
])

@app.callback(Output("log", "children"), Input("lena", "loaded"))
def on_load(loaded):
    print(loaded)
    return str(loaded)

if __name__ == '__main__':
    app.run_server(port=7777)

You should be able to run it if your install dash-leaflet==0.1.26rc1.

prl900 commented 1 year ago

This is exactly what we needed!. Thank you so much @emilhe

prl900 commented 1 year ago

Hi @emilhe, I think there is some kind of problem with this last release 0.1.26

Browsers report an error when loading your example above that wasn't happening before:

DevTools failed to load source map: Could not load content for http://127.0.0.1:7777/_dash-component-suites/dash_leaflet/leaflet.js.map: HTTP error: status code 500, net::ERR_HTTP_RESPONSE_CODE_FAILURE

Looks like one of the Javascript libraries is missing?

prl900 commented 1 year ago

I think this prop is not working as expected. Here is a sample code modified from your example. This updates the overlay to a random image with a button but the "loaded" callback is only firing once at the beginning:

import json
import dash_leaflet as dl
from dash import Dash, html, Output, Input
import random

tile = "https://a.tile.openstreetmap.org/6/38/19.png"

app = Dash()
app.layout = html.Div([
    dl.Map([
        dl.TileLayer(),
        dl.ImageOverlay(url=tile, bounds=[(56, 10), (55, 13)], id="overlay")
    ], style={'width': '100%', 'height': '50vh', 'margin': "auto", "display": "block"}),
    html.Div(html.Button("Update", id='update')),
    html.Div(id="log")
])

@app.callback(Output("overlay", "url"), Input("update", "n_clicks"))
def update(_):
    return f"https://a.tile.openstreetmap.org/6/{random.randint(0,24)}/{random.randint(0,24)}.png"

@app.callback(Output("log", "children"), Input("overlay", "loaded"))
def on_load(loaded):
    print(loaded)
    return str(loaded)

if __name__ == '__main__':
    app.run_server(port=7777, debug=True) 

I think the on_load() callback should fire each time a new url is loaded.

emilhe commented 1 year ago

@prl900 This is actually expected behaviour (in Dash). With the current design (a bool), the value only changes the first time (from null to bool). And since Dash callbacks fire only on property changes, you'll only see the callback fire the first time.

If you want the callback to fire every time, the property value should either be reset, when a new image url is set - or the loaded property should hold more information (e.g. what image was loaded, how long time it took, ...). I believe the latter approach is the most Dash-like (this is also the idea behind props like n_clicks etc.).

I have pushed a version 0.1.27rc1 that changes the property to include the event time stamp and url; with this information present (in fact, I guess the timestamp would be enough), the value will set will be unique, and the callback is Dash should thus fire every time. Does it work as intended for your use case?

prl900 commented 1 year ago

Thank you very much @emilhe for explaining the conventions for Dash callbacks firing. It all makes sense now.

The new approach you propose is good. As you suggest, the timestamp should be enough to make this property fire with each update.

I believe this new method should be enough to cover our needs. The new release candidate hasn't been published yet though, but I will test it and confirm once you push it.

emilhe commented 1 year ago

Ah yes, sorry, I can see that the upload failed. I have re-uploaded it now :)

prl900 commented 1 year ago

Perfect. this works perfectly and solves our problem. I have just tested and works very well.

In our case, we just need the callback to fire and can ignore the contents of the variable as it's usually the case with button clicks. From a design perspective, I agree it makes sense to use just the timestamp though.

Thanks again for your help with this!