PolymerElements / iron-list

Element for a virtual, "infinite" list
https://www.webcomponents.org/element/PolymerElements/iron-list
219 stars 131 forks source link

Setting display property of element prevents hiding of element #119

Open indolering opened 9 years ago

indolering commented 9 years ago

Oddly, commenting out the display value in the child element's :host selector "resolves" the problem. Does not occur on Firefox.

Screenshot from Chrome Version 45.0.2454.101 running on OS X. image

I'm unable to produce this in a fresh element but I've sent my codebase to @blasten.

blasten commented 9 years ago

@indolering I still need a small file that can reproduce this issue. There could potentially be hundreds of things happening here at the same time.

indolering commented 9 years ago

I was finally able to replicate the issue in seed-element and it occurs whenever you specify the display property of the child. I'm not sure why I couldn't replicate the issue previously.

blasten commented 8 years ago

@indolering

  1. Can you try this again? the list uses the hidden attribute which is standard and paper-styles has a polyfill for IE10 https://github.com/PolymerElements/paper-styles/blob/master/classes/shadow-layout.html#L241
  2. In your code: https://gist.github.com/indolering/2a14ed5726569ad3862a#file-seed-element-html-L76 you need to use Polymer's array mutation API to notify the changes. e.g. this.push('matching', record);
indolering commented 8 years ago

@blasten I wiped that seed-element from my machine a while ago.

In my production code I replace the entire array. I tried adding this.notifyPath('_matching', this._matching); after the array replacement but it didn't help.

blasten commented 8 years ago

@indolering is this still an issue?

indolering commented 8 years ago

is this still an issue?

Yes, here's an updated Gist that uses proper data binding.

Can you try this again? the list uses the hidden attribute which is standard and paper-styles has a polyfill for IE10

Ahh, yes, I just ran into this while working on another issue: the hidden attribute is overridden by the display attribute. You could hide the element via styles (maybe a selector for direct children with a given class combined with an !important attribute) or physically re/move items when the list is too small. If you want to use the hidden attribute you will either need to wrap each list item in a container or forbid list items from setting a non-default display property.

indolering commented 8 years ago

@blasten Are there serious performance or architectural barriers to simply creating/deleting elements when there aren't enough to fill the list? The other three options I outlined all have serious drawbacks.

Forbiding list items from altering their display type would mean that they can't use flex or other layouts.

Wrapping each item in a container would break existing styles and cause problems with :nth-child() selectors. Wrapping items would also make it difficult for the child to detect if it is at the top of the list, something I rely on to solve the issues I've had with paper-tooltip being covered up by parent objects.

Hiding items with a class is my second favorite option after destroying child elements. You could use an unlikely class name to avoid collisions. However, it would break if users aren't careful when programmatically setting classes.

indolering commented 8 years ago

FWIW, the current system of recycling also appears to screw up :nth-child() selectors. Is the performance penalty for creating/destroying elements really that high?

indolering commented 8 years ago

I fixed the last Gist link above.

blasten commented 8 years ago

@indolering yes, the cost of creating/destroying the DOM is high enough for cases when those nodes are needed in the future. e.g. imagine a filter or search feature. Also, I won't recommend using the nth-child selector for this element. If your list is relatively small and/or you don't have critical perf requirements I'd recommend using dom-repeat.

indolering commented 8 years ago

My list is tens of thousands of entries long. Does the hiding via a class sound like a doable solution?

blasten commented 8 years ago

This PR: https://github.com/PolymerElements/iron-list/pull/328 is trying to solve this issue.