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

Show load status of layers in tree panel #358

Closed
wants to merge 2 commits into from

Conversation

bentrm
Copy link
Member

@bentrm bentrm commented May 12, 2015

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.

@bentrm bentrm changed the title Loadingstate Show load status of layers in tree panel May 12, 2015
@marcjansen
Copy link
Member

Does this PR replace #357?

@bentrm
Copy link
Member Author

bentrm commented May 18, 2015

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
Copy link
Member

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

@weskamm
Copy link
Member

weskamm commented May 18, 2015

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
Copy link
Member Author

bentrm commented May 19, 2015

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
Copy link
Member

weskamm commented May 20, 2015

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
Copy link
Member Author

bentrm commented May 20, 2015

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
Copy link
Member Author

bentrm commented Jan 12, 2016

I revisited this and made some changes:

  • by default a loading layer has a spinner instead of its layer icon in the tree panel
  • one may override this on a layer per layer basis by setting the metadata.hideSpinnerInLayerTree property of the layer
  • the tree example has been adapted to show a map region that better demonstrates the features of the layer panel (out-of-range layers, loading layers)

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)?

@bentrm bentrm closed this Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants