resize notifications are broken

[MajorBreakfast]

My <paper-tabs> element draws it's selection bar incorrectly if I use it as a descendant of a <neon-animated-pages> element on an initially hidden page.

The culprit is this line. It's an optimization, but it's broken and I can't figure out what goes wrong. Can you remove it? (Or even better - fix it :)

[MajorBreakfast]

Got it! Did some codebase digging.
The current approach assumes that all children implement IronResizableBehavior which can then in turn forward the resize notification deeper down the tree.

• Either let NeonAnimatableBehavior inherit from IronResizableBehavior. Thus the behavior is always there and notifications can always be forwarded.
• Or forward the notification to descendants of the selectedPage element as well, not just the element itself.

The former has the advantage that it avoids tree walking as it doesn't require a descendancy check. The latter has the advantage that it doesn't complicate NeonAnimatableBehavior's inheritance chain. I'm not sure which one's better! What do you think?

I hope that this can be resolved. The need to manually implement the IronResizableBehavior if descendants need it is bad for usability. The element should just do "the right thing".

---

Additional problem: _notifyPageResize() is called too late because it's only called after the animation has played. In my case the selection bar only appears after the animation has finished which leads to a flicker.
Edit: Submitted PR #116

[cdata]

Thanks for the suggestion and the contribution. I will take a look!

[MajorBreakfast]

@cdata Thanks for the quick response! I found some old open issues with the term "resize" from June. So, I was a bit worried :) Could you add the bug label as well? (My users won't know which tab is selected => bug)

• My PR only fixes that the selected page gets its resize notification earlier. The old page should probably still receive notifications until it becomes really invisible. That was previously the case.
• Also, a possible optimization would be to just notify a page if and only if <neon-animated-pages> received at least one notification itself while the page in question has been hidden. That requires some bookkeeping though. However, cloud be done by counting notifications for all invisible pages in a WeakMap. (undefined or >=1 would mean the newly selected page needs to be notified)

What do you think?

[cdata]

Sorry, I misread. You are correct that this is a bug. I think the implementation is just weird and needs to be fixed. I believe there is another issue open that talked about a similar problem.

[omarbelkhodja]

I'm facing the same issue, but with an iron-list in the hidden page. See the link below:
http://stackoverflow.com/questions/34494427/polymer-iron-list-display-is-incorrect

[omarbelkhodja]

Maybe the easiest solution would be replacing

return element == selectedPage; 

with

return (typeof selectedPage != "undefined") ? selectedPage.contains(element) : false;

[MajorBreakfast]

Good idea. I didn't know that this method even exists :)

Quick thought: Can we be sure that it will work properly with the shadow DOM? I think that it won't necessarily because the method probably only considers children in the light DOM. BTW Polymer.dom(element).contains() is not a thing. Writing or own should be straightforward though, since it's just a while loop traversing up in the tree.

Additionally I'd say that it should check both the current selected page and while it's animating out the previous selected page as well. Otherwise resizing the window during the animation will cause visible problems if the animation is slow.

I'd code it like this (typeof is overkill in this case):
return selectedPage ? selectedPage.contains(element) : false

[addyosmani]

Hey @MajorBreakfast and @omarbelkhodja. Were either of you able to evaluate how well the above proposed workaround handles in your current apps using native Shadow DOM? I can't see immediate negative repercussions to the approach proposed, but am interested in how well this change would work outside our demos. It's definitely a bug.

Additionally I'd say that it should check both the current selected page and while it's animating out the previous selected page as well

I'm personally in favor of this. Does anyone want to submit an updated patch and we'll give it some further testing? 🚗

[MajorBreakfast]

I'll have a look this weekend.

[omarbelkhodja]

I have been using the fix "return selectedPage ? selectedPage.contains(element) : false" for many weeks now, and that fixed my problem. The exact file modified I'm using is https://github.com/omarbelkhodja/neon-animation/blob/master/neon-animated-pages.html. I'm not checking the previous selected page.
Now, I've just made a test using Chrome + shadow DOM, by adding

window.Polymer = window.Polymer || {};
window.Polymer.dom = 'shadow';

The behaviour is still the same : the problem is fixed. I have also verified in the Developer Tools that shadow DOM is really enabled.

Now, looking at https://www.w3.org/TR/domcore/#dom-node-contains, I'm not sure whether elements in the shadow DOM are checked by this API or not. It means that on Chrome, it is OK now, but it is not said that the API will still have the same beviour later, if the current behaviour is judged to be a bug.

[MajorBreakfast]

@omarbelkhodja Here is a quick jsfiddle. Both chrome and firefox (dom.webcomponents.enabled set to true) report false. So that method cannot work with the real shadow DOM!

[omarbelkhodja]

@MajorBreakfast Yes, right ! I've checked your code and what you say is true. But the thing is that my problem is solved with that piece of code... Looking further, I think we don't really want to notify the elements within the shadow DOM. The elements that need to be notified about the resize event are the elements that are inside the content of the page. They are not elements that are inside the shadow dom of one page. Each element should be in charge of forwarding or not the event inside its shadow DOM. What do you think about that ?

[MajorBreakfast]

@omarbelkhodja Take as an example a site that has tabs in its menu bar. Assume the menu bar is a webcomponent and has the tabs in its shadow DOM. Resize notifications must cross the shadow boundary for the tabs to work properly (jsfiddle).