Lazy loading events and mixins

[platoscave]

This branch contains features that facilitate lazy loading
Including in paper-tree-node:

• a new toggle-children event fired from toggleChildren
• a new class selector for the loading icon: .node-preicon.loading:before
• the addition of src$="[[data.src]]" to the iron-icon (not related to lazy loading)
• the addition of white-space: nowrap; to node-container (not related to lazy loading)

New files:

• loading.svg: animated loading icon
• lazyload-demo: a custom element that demonstrates lazy loading

Chris

[vpusher]

@platoscave In my mind the component should not know about the lazy stuff. It is only dumb UI. All we need to provide is utils and events so it can implemented by the end-developer easily.

Consequently, let's only keep a toggle event on <paper-tree-node>, that will bubble up to the top <paper-tree> for listening purpose. So no CSS selectors specific to lazy loading, no embbeded svg.

Icon could be updated with something like:

<paper-tree on-toggle-children="toggleChildren"</paper-tree>
[...]
function toggleChildren(e) {
    var node = e.detail;
    node.set('data.icon', 'loading');
}

[vpusher]

Could you provide something much more simpler and shorter ? This would act as a sample. It have to be understood in seconds. We only need to listen 'toggle' event and some lazy data fetcher (a few lines)

[platoscave]

I'll have a go at this.

[vpusher]

could you add it as a dom bind (instead of a component) in the demo page. and fix the former demos BTW 😅

[vpusher]

As explained, lazyness is not part of the component. Consider remove this image.

[vpusher]

Same here. Remove that block.

[vpusher]

I would go for a toggle event name.

[vpusher]

Remove this also.

[vpusher]

this.fire('toggle', this);

[platoscave]

@vpusher
The problem with this approach is, it's not the icon we want to (un-)set to loading, it's the preicon.
This would involve updating the class of the node from the 'toggle' event in the host. We would also have to inject the loading class from the host with one of those weird '>' notations. Any thoughts on how to achieve this?

While I agree with your philosophy, I don't necessarily agree that adding a loading state detracts from "dumbness". open can be thought of as having four states: expanded, collapsed, loading, none. This will work in any world.
...So another approach might be to change open from Boolean to something that can hold four states.

Lazy loading is especially important in enterprise applications where we're dealing with large amounts of data which we don't want to download all at once. This is where paper-tree will come into its own. It makes sense to provide minimal support for this out-of-the-box.

[vpusher]

I know how important it is to lazy load data, and how much you need it in some case. The thing is some will want to update the pre-icon, the others will want to update the icon and perhaps some others will want a loading text message (or both, or the three). That is to say the way the loading feedback is rendered is under the end developer responsibility. I don't want to take any opinion on this. This is a view that is bound to a model and it provides some hooks/events to implement you own logic.

To resume, to solve your issue the component needs the followings openings/hooks:

[x] know when children are toggled: a toggle event is needed.
[ ] be able to change pre-icon: a mixin would be a good answer for that.
[x] be able to change icon: you can actually do it editing the node's data.icon
[x] be able to change text: you can actually do it editing the node's data.name

[platoscave]

@vpusher
I've adopted your recommendations in the latest push.
In order to enable changing the pre-icon I've add the --loading-icon-theme that can serve as a mixin to paper-tree-node.

The lazyload-demo is now much simpler and works fine, all except for applying the 'loading' class to the pre-icon span. The 'loading' class needs to be applied shortly after the node is created due to a data update. These spans are really hard to find. I thought about searching the dom tree but this is generally discouraged because "It requires the consumer to be aware about element's internals. Web component should only be used with properties/attributes and events".

What would be a good way to get access the pre-icon span so that we can modify it's class?

[vpusher]

@platoscave what about my last commit ? does it fit your needs ?

[platoscave]

We have to add target.loaded = true; to the demo's, Apart from that, that it works like a charm.

I'm also testing --paper-tree-icon-theme. It turns out that iron-icon doen't provide much in the way of mixins to do something with the icon, other than size and color.
I found, what I think might be a better way to modify the icon:

setNodeIcon: function (node, icon) {
     var ironIcon = node.$$('iron-icon');
     ironIcon.set('src', icon);
 },

The only problem here is the issue which I mentioned earlier: iron-icon icon takes precedence over src contrary to docs #78. I'd have to hack my way around this as long as it hasn't been resolved. If this is an acceptable way of working we might not need --paper-tree-icon-theme.

Note: I had to add src/ to background:url(\'src/loading.svg\');\ in setLoadingState. I'm not sure if something similar is needed in your demo's, as I'm not running the code directly, but rather from my custom element.

How does one go about running the demo's, given that there is no bower-components context?

[vpusher]

@platoscave do you run it with polymer serve ?

[benbenbenbenbenben]

In my opinion the solution here is overly complicated. You can lazy render simply by using dom-if in a template on children. If a node isn't visible it isn't rendered (yet).

#4

[vpusher]

@benbenbenbenbenben here we are mostly dealing with customizing the UI through openings/hooks such as events and mixins (to fit the use case of @platoscave). Data lazy loading is totally in charged of the end developer. That's the first point. The second point is about lazy render the tree, and that is the purpose of your PR. Tank you for that, I will consider it, merge it and then release a new version of the component with those 2 enhancements (PRs).

[vpusher]

@platoscave I am merging this one. Most of your use cases are covered now, and the current implementation seems generic enough to me. Please address a new PR for the icon src issue if you really need it.

[benbenbenbenbenben]

@vpusher okay thanks for this

[vpusher]

@platoscave Released in v1.0.3