geoext / geoext2

GeoExt 2 — JavaScript Toolkit for Rich Web Mapping Applications
http://geoext.github.io/geoext2/
Other
142 stars 106 forks source link

Show load status of layers in tree panel #358

Closed bentrm closed 3 years ago

bentrm commented 9 years ago

This is a follow up to PR #357.

The 'loading' attribute is implicitly inherited by the NodeInterface that will be mixed into LayerTreeModel when added to a tree panel. Setting it to true will cause the node to show it's actual status in the tree panel temporarily replacing the layer icon with the Ext provided load spinner. It can be observed in the tree examples.

marcjansen commented 9 years ago

Does this PR replace #357?

bentrm commented 9 years ago

I thought Github would handle dependent PRs transparently. This builds on top of #357 removing the event handlers but actually adds a "feature". It's probably not that much of a deal so maybe reviewing only this branch is enough.

marcjansen commented 9 years ago

Once #357 is in, this simply would need a rebase.

weskamm commented 9 years ago

Setting it to true will cause the node to show it's actual status

Could you point out the direction at which point the parameter has to be set? I just had a short look, but could not figure out why the tree example works, but the tree-legend not or how to enable it there. it would also be nice to have an option to toggle the behaviour on and off, where off should be the default.

Besides, this is a very nice addition in my opinion, and the current code looks ok so far. Having a short description in the code for the new handlers would be nice

bentrm commented 9 years ago

Could you point out the direction at which point the parameter has to be set?

You mean when the loading property is available? In the docs it's described the following:

This class is used as a set of methods that are applied to the prototype of a Model to decorate it with a Node API. This means that models used in conjunction with a tree will have all of the tree related methods available on the model.

So among others loading is available as soon as the nodes are added to an Ext.tree.Panel.

I just had a short look, but could not figure out why the tree example works, but the tree-legend not or how to enable it there.

It's working but it's not visible (:trollface:) as tree-legend has the following CSS.

.gx-tree-layer-icon {
    display: none !important;
}

Sorry for the misleading description that wasn't my intention, it's really just visible in the plain tree example.

it would also be nice to have an option to toggle the behaviour on and off, where off should be the default.

I wonder where to put the option. LayerNode is really just used indirectly by LayerLoader but it may also be nice to centralize such things as properties of the LayerTreeModel.

weskamm commented 9 years ago

Thanks for clarification! I still think that we need a property to enable this behaviour. I even think that this should be configurable per layer, as i can imagine that i.e. local (and fast) vectorlayers would suffer from weird flicker effects because of their quick loadingtimes. So maybe even the LayerModel might be the right starting point. Are you willing to enhance this PR? Any other opinions @marcjansen @chrismayer ?

bentrm commented 9 years ago

Sure, I see the benefit in making it configurable. I tried the following:

    onLayerLoadStart: function() {
        var me = this;
        me._timeoutID = window.setTimeout(function() {
          me.target.set('loading', true);
        }, 200);
    },

    onLayerLoadEnd: function() {
        this.clearLoadingTimeout();
        this.target.set('loading', false);
    },

    clearLoadingTimeout: function() {
        if (this._timeoutID) {
            window.clearTimeout(this._timeoutID);
            this._timeoutID = null;
        }
    },

It works quite well as it is kind of dynamic, but of course there is the edge case of a loading process taking 201ms. It would be possible to have something like loadingTimeout as a numeric attribute in LayerTreeModel disabling listeners all together if the value is undefined/negativ. Anyway, I don't want to overcomplicate things for upstream, so a boolean loadingSpinner would probably be sufficient.

bentrm commented 8 years ago

I revisited this and made some changes:

Unfortunately, the pre selected base layer of the demo won't load anymore (doesn't load in the online examples as well). Don't know if you have any alternative services for that (maybe related: #372)?