farmOS / farmOS-map

farmOS-map is an OpenLayers wrapper library designed for agricultural mapping needs. It can be used in any project that has similar requirements.
https://farmOS.github.io/farmOS-map
MIT License
39 stars 22 forks source link

Add loading indicator as default behavior #187

Closed paul121 closed 1 year ago

paul121 commented 1 year ago

This is an initial simple implementation for #97

I've largely just taken this OL example and adapted into our code a default named behavior loading. It's really quite simple. I think we could expand on this with more complex features described in #97 by using behavior settings eg: mode: simple/complex, progess: true/false, etc.

paul121 commented 1 year ago

One thing to note here. The example uses the map's wrapper that is provided by implementing code. When using the map's wrapper it must be position: relative, otherwise the spinner ends up in the middle of the browser window, not the center of the map.

I chose to add the loading spinner to the map's viewport container because the viewport is already position: relative. The benefit is that this does not require that implementing code to set the position correctly. But I'm not familiar enough with the internals to know if using the viewport is a bad idea. I've opened a PR to openlayers to suggest & ask about this: https://github.com/openlayers/openlayers/pull/14233

paul121 commented 1 year ago

But I'm not familiar enough with the internals to know if using the viewport is a bad idea. I've opened a PR to openlayers to suggest & ask about this:

So OL doesn't recommend this (see my PR referenced above). They state that the application "owns" the target element, and the library "owns" the viewport element. We are in an interesting situation because farmOS-map is the library... IMO using the viewport should work fine, I don't think it will disappear from us ;-)

mstenta commented 1 year ago

Wow this is a pretty simple change! Haven't tested it but LGTM!