Ticket #102 (closed feature: fixed)

Opened 8 years ago

Last modified 6 years ago

"loading..." indicator, so that users don't wonder what is happening

Reported by: anonymous Owned by: sderle
Priority: minor Milestone: Addins
Component: Events Version:
Keywords: patch Cc: penyaskito@…
State:

Description (last modified by sderle) (diff)

can be implemented as a map control.

Attachments

loading-panel.patch Download (8.7 KB) - added by openlayers 8 years ago.
Loading panel patch
loading.gif Download (2.6 KB) - added by openlayers 8 years ago.
loading animated GIF
loadingpanel.patch Download (7.6 KB) - added by crschmidt 7 years ago.
ol-ajax-loader.gif Download (7.8 KB) - added by penyaskito 7 years ago.
Added an alternative loading progress indicator with the OL logo
loadingpanel_map.patch Download (10.0 KB) - added by bartvde 7 years ago.
new patch which also implements having a loading indicator for the whole map

Change History

Changed 8 years ago by euzuro

  • milestone Farallon Islands deleted

Changed 8 years ago by sderle

  • description modified (diff)
  • milestone set to 2.5 Release

Changed 8 years ago by openlayers

Loading panel patch

Changed 8 years ago by openlayers

The patch attached is a little change to the OpenLayers.Layer.Grid class in order to let it trigger the "loadstart" and "loadend" events and a loadingPanel.html example showing how to implement a custom control to handle them showing up an nice animated loading panel GIF when loading tiles.

Changed 8 years ago by openlayers

loading animated GIF

Changed 7 years ago by euzuro

If #831 is approved and applied, we should revisit this ticket to see how we can get the two working together. I havent looked at the example, but there is an interesting bit in this library code regardin listening for "error" and "abort" on the image, which I think might be worth getting in.

Changed 7 years ago by crschmidt

This depends on #842, which is the portion of the grid changes with error/abort handlers added.

Changed 7 years ago by crschmidt

Changed 7 years ago by crschmidt

  • keywords patch added

Attached patch replicates functionality of existing patch, but does it in a way that is built into OpenLayers.

  • Control is split out into OpenLayers.Control.LoadingPanel class.
  • Layer is passed to control constructor, which registers events
  • No more need for the grid changes, since that is now built into the library
  • Includes example, docs and tests

However, before this goes in, I think we need to rethink the draw() function -- how do we want to allow people to control the look and feel of this thing? I think that the display should be moved, as much as possible, into CSS, so this patch is a big start, but is not complete.

Changed 7 years ago by crschmidt

  • milestone changed from 2.5 Release to 2.6 Release

Changed 7 years ago by penyaskito

Added an alternative loading progress indicator with the OL logo

Changed 7 years ago by penyaskito

  • cc penyaskito@… added

Changed 7 years ago by bartvde

A question, this creates a loading indicator per layer, whereas I would need one per map. Is this not implemented, or is it by design that the loading indicator is per layer? I can imagine an option where if you put in layer:null that it would work for all layers in the map.

The patch contains at least one problem causing it not to work for me in Firefox, it misses the "px" suffix for setting width and height:

this.div.style.width = msgW+"px";
this.div.style.height = msgH+"px";

Also, it might be better to this with css, so:

this.div2.className = 'loading';

And:

.loading{
  background-image:url(lib/openlayers/img/loading.gif);
  background-repeat: no-repeat;
  background-position: center;
}

Finally, I did not get why it needs 2 divs, I get the same effect using 1 div, also using 1 div there is no positioning problem when the map updates its size (there is a problem when using the 2 div's in the patch).

When I finish implementing this for all layers in a map, I'll post an updated patch.

Changed 7 years ago by bartvde

new patch which also implements having a loading indicator for the whole map

Changed 7 years ago by crschmidt

  • state set to Review

Changed 7 years ago by crschmidt

  • state changed from Review to Needs Discussion

I think this may be a candidate for 'addins' -- not really necessary for trunk, but could help out a fair number of users, and is more 'application level' code that it makes sense to have seperately. I've sent an email to the mailing list asking for opinions: I'll mark this needs discussion until I hear more back on that front.

Changed 7 years ago by tschaub

Some random comments:

1) We now use a simpler class syntax (no need for mentioning the prototype or using inherit) - see any of the other OL classes for an example. 2) Other elements associated with controls also use the control.displayClass property as a class name. This is generated by the Control superclass. I think it is better to use this convention than to use the "loading" class name - it also gives app designers the ability to override the name. 3) 200 and 300 look like magic values. Does this stop me from having a partially opaque element over the whole map if my map is bigger than 200 x 300? I think these values are better as css declarations. 4) Super trivial, but we also now need to separate the @requires comment block from other natural doc comments.

I also think this would make a nice addin. Would be good to start with something anyway.

Changed 7 years ago by crschmidt

  • milestone changed from 2.6 Release to Addins

Changed 7 years ago by bartvde

I want to have a look at this again in the near future, just not to forget, this ticket has a dependency on:

ticket:1220 conflict between addlayer and loadstart event, we will need to listen to preaddlayer instead of addlayer ticket:1343, we can now get the layer from evt.layer. Also we need to listen to removelayer events.

Changed 6 years ago by euzuro

Hi I have looked over this code as well and I agree with all of the sentiments that tim and chris expressed above in the comments.

Also, I would like to see this thing in action, but the link on:  http://trac.openlayers.org/wiki/Addins/LoadingPanel

is broken.

Is the patch included here in the ticket up-to-date with what is in your sandbox, bart?

Basically I think we are all in agreement that this is a very good thing to have. If the issues that tim makes above are settled, then I think this is ready to be a bonafide addin.

Changed 6 years ago by euzuro

  • status changed from new to closed
  • state Needs Discussion deleted
  • resolution set to fixed
Note: See TracTickets for help on using tickets.