vpusher / paper-tree

Browsable tree of nodes with expandable/collapsible capabilities and actions menu
MIT License
27 stars 24 forks source link

Issues related to shadow dom vs. shady dom #2

Closed platoscave closed 7 years ago

platoscave commented 7 years ago

Hi, I ran into a couple of issues when applying paper-tree, I think they are related to shadow dom vs. shady dom, but I'm not sure. That might explain why they work in you're demo but not in my application. My application is based on the Polymer Starer Kit. I'm working on Chrome v55.

I went ahead and forked your repository and applied some fixes. In all just 4 lines of code. The Fork/branch can be found here: https://github.com/neuralquest/paper-tree/tree/lazy-load

Issues: I ran in to a problem related to event retargeting. The e.target in paper-tree _selectNode was pointing to the paper-tree node, as opposed to the selected paper-tree-node. I was able to fix this using var normalizedEvent = Polymer.dom(event). See: https://www.polymer-project.org/1.0/docs/devguide/events#retargeting

I also had an issue with node-row highlighting. What I found was that the class selector under :host, needs to be placed in parentheses, like so: :host(.selected). There are examples of this in Rob Dodson's cheat sheet: http://robdodson.me/shadow-dom-css-cheat-sheet/

We may have to add ::content to the statement as wel. I didn't bother with this since its working for me without. https://www.polymer-project.org/1.0/docs/devguide/styling#styling-distributed-children-content

New Features: I propose firing a toggle-children event from toggleChildren so that we can use the tree for lazy loading. I've added a lazyload-demo in the form of a custom element. Still needs to be incorporated in the demo page. See Tree lazy loading in action: https://neuralquest-395e9.firebaseapp.com/ (work in progress, takes a few seconds to warm up :-()

I added src$="[[data.src]]" to the iron-icon so that users can optionally provide a url/svg/base64 string. The docs say that scr takes precedence over icon, but this is not true. To work around this I also changed the parm in _computeIcon to data and added if(data.src) return '';. The app above uses base64 strings read from the database.

I also added white-space: nowrap; to .node-container to prevent wrapping of the tree node. You can see the effect in the example above by sliding the slider to the left.

I also propose adding an animated loading icon. By setting loading:true you are in effect saying "I don't know yet if there are any children". See _computeClass and class selector .node-preicon.loading:before in paper-tree-node. The app above shows this in action.

Hope this helps. I learned a lot from studying your code. Chris

vpusher commented 7 years ago

@platoscave Thx for posting.

About your issues:

Would you try the release v1.0.2 and tell me if it fixes all the things.

Then we could talk about your feature requests.

platoscave commented 7 years ago

I can confirm that v1.0.2 works for me. I also added a fourth enhancement request for an animated 'loading' icon.

platoscave commented 7 years ago

Issues resolved. New features discussed in pull request