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

[0.8] ShadyDOM seems to make it hard to work with other data binding frameworks. #1405

Closed
trading-peter opened this issue Apr 13, 2015 · 19 comments

Comments

@trading-peter
Copy link
Contributor

My example is about AngularJS, but I suppose this will happen with any data binding enabled framework. I stumbled upon this when I tried to use ng-repeat to generate some <paper-tab> elements.

Somewhat like so:

var app = angular.module('app', []);

// A simple silly controller
app.controller('myCtrl', function($scope) {
    $scope.tabs = [{name: 'Step 1'}, {name: 'Step 2'}];
});

Then somewhere I have some html snippet like this:

<div ng-controller="myCtrl">
    <paper-tabs>
        <paper-tab ng-repeat="tab in tabs">{{tab.name}}</paper-tab>
    </paper-tabs>
</div>

If you render this while ShadyDOM is in use you'll get something like this:
Tabs-problem
I added markers because I don't have any styling on my code right now.
The blue marked elements are the tabs rendered in the ShadyDOM, while the red ones are unwanted text node.
I think what happens here is that Polymer builds the ShadyDOM, adding the additional layout into the element but angular is also in the process of doing it's data binding magic and inserts the text node in the somewhat "wrong place" (or not wrong place as far as angular is concerned).

Here's what the markup looks like (first paper-tab expanded):
developer tools - http___localhost_4443_install

If you enable ShadowDOM this will not happen (of course).

@schnie
Copy link

schnie commented Jun 12, 2015

@pkaske having similar issues with 1.0 and meteor. Tried to enable shadow dom rather than shady dom, but having trouble getting it to work in firefox without using the ?dom=shadow query param. Curious if you had tested using shadow dom with firefox?

@trading-peter
Copy link
Contributor Author

@schnie No I didn't test it in Firefox. I expected the problem to be the exact same as firefox doesn't support native shadow dom out of the box.

@ebidel
Copy link
Contributor

ebidel commented Aug 25, 2015

https://github.com/PolymerLabs/polymer-experiments/blob/master/patch-dom.html might help. Load it in an html import after Polymer. It patches the DOM methods to work with shady dom.

@trading-peter
Copy link
Contributor Author

Thank you. I don't work with angular currently but hopefully it helps the meteor folks.

@emmanuelbuah
Copy link

@ebidel I know patch-dom is experimental and needs more work. We are struggling with Polymer + other rendering engine because of this issue. It would be great if the polymer team could help us with more work on patch-dom so we can enjoy using polymer with existing rendering engines. We need help.

FYI - I'm also using polymer with meteor and since 1.0, I've had to abandon polymer because of rendering issue with shady dom.

@ebidel
Copy link
Contributor

ebidel commented Sep 12, 2015

Can you elaborate what you're missing. Is there something not working?

On Sat, Sep 12, 2015 at 11:18 AM kwame notifications@github.com wrote:

@ebidel https://github.com/ebidel I know patch-dom is experimental and
needs more work. We are struggling with Polymer + other rendering engine
because of this issue. It would be great if the polymer team could help us
with more work on patch-dom so we can enjoy using polymer with existing
rendering engines. We need help.


Reply to this email directly or view it on GitHub
#1405 (comment).

@emmanuelbuah
Copy link

@ebidel Sure. I will put together and share a repo for demonstrate rendering issues with meteor.

@emmanuelbuah
Copy link

@ebidel Per last message, here is a link to publish simple polymer site (using meteor rendering) - http://polymershadow.meteor.com. Polymer is configured to use shadow dom on the page. Rendering of button on page 1 works fine on load or refresh but has issues rerending if you navigate from page 2 back to page 1. Below are steps to reproduce the issue.

Steps to reproduce rerending issue

  1. Refrest site
  2. Navigate to page 2
  3. Navigate back to page 1

This issue is applicable only to firefox and safari. I haven't tested on Edge or IE. Works fine on chrome but happens on chrome when polymer is configured using shady dom.

FY- The issue persists after apply patch-dom.html to the page.

Source code here - https://github.com/emmanuelbuah/meteorpolymer

@emmanuelbuah
Copy link

@ebidel I have an app that currently stuck in 0.5 due to this issue. I reported a similar issue at #2453 (comment). What can I do to help address this issue? Let me know if there anything I can do to help.

@ebidel
Copy link
Contributor

ebidel commented Sep 26, 2015

@emmanuelbuah
Copy link

@ebidel I uncommented patch-dom and redeployed to http://polymershadow.meteor.com as well as updating the repo. Everything works fine in chrome and opera but not in firefox or safari.

@emmanuelbuah
Copy link

@ebidel I just wanted to say thanks for attending to this ticket. I have decide to drop polymer for my service www.huqoo.com because we've been struggling with 1.0 upgrade. This ticket being one of them. You will find more out meteor + polymer community concerns here - https://gitter.im/emmanuelbuah/MeteorPolymer. In any case, I know you guys are busy and can't attend to every issue. I just wanted to share this. Again, thanks.

@a4xrbj1
Copy link

a4xrbj1 commented Nov 9, 2015

Have to agree with the sentiments expressed before like from @emmanuelbuah . The problem of Meteor & Polymer isn't the only framework that's causing problems and yet it still doesn't seem to bother the developers. You guys are missing a golden opportunity to rectify the general view about Polymer based on the 0.5 debacle. If you want developers to talk more positive about Polymer 1.0 then support them and address their problems/issues reported. Especially the major ones like this. Otherwise repeat history and don't wonder why developers have stopped development with Polymer and turned to alternatives as they won't wait forever (yes, there are developers out there that just don't do it for fun but have real paid applications to be build).

@ebidel
Copy link
Contributor

ebidel commented Nov 9, 2015

rectify the general view about Polymer based on the 0.5 debacle

Can you explain this? We were very clear from the beginning that 0.5 was a developer preview. Despite this, people still built production apps using it, including Google. Big changes for a 1.0 were to be expected. I too had to update a number of apps to 1.0 and have felt the pain first-hand. Tools like http://polymerlabs.github.io/polyup/ are really helpful.

then support them and address their problems/issues reported

We have 2,500+ open issues across Polymer, PolymerLabs, and PolymerElements with dozens of issues reported daily across may different repos. Prioritization becomes the bottleneck. We're addressing critical bugs and performance issues that affect the broadest set of users, first.

I offered a workaround in August and have been asking for more info as comments come in. Unfortunately, we've been given repo links to full apps, code that has my suggested workaround commented out, and an app (http://polymershadow.meteor.com) where the repo steps do not work in FF/Safari. Clicking the page buttons don't do anything for me. This back and forth increases the amount of noise on the issue and makes it very hard to help debug problems.

If folks on this thread could please post concrete specifics on things that are not working, that would be very helpful to move this issue forward.

@emmanuelbuah
Copy link

@ebidel I agree. There is just a lot of back and forth. I do know both sides want to address the issue. Per my previous message. You have been helpful and continue to be. I appreciate that. I will do my best to clarify.

The sample app hosted at http://polymershadow.meteor.com contains your suggest fix/workaround in August. The fix addresses the issue in chrome and opera but not in safari and firefox.

The expected result would have been the site (http://polymershadow.meteor.com) works on chrome, opera, safari and firefox. I haven't even run the sample on IE so I don't know if it would work or not.

In summary, dom-patch addressed the issue in chrome and opera but not in safari and firefox, and I don't know why?

Let me know if there is any specific information you need to help arrive at the cause for this issue.

@arthurevans
Copy link

@emmanuelbuah I believe it's working on Chrome and Safari not because of patch-dom, but because you're enabling native shadow DOM on Chrome and Opera:

https://github.com/emmanuelbuah/meteorpolymer/blob/master/meteorpolymer.html#L6

The patch-dom import doesn't do anything if you have native shadow DOM -- it's not needed:

if (nativeShadow || this.node.__patched ||

When I looked at the Meteor issues before, it appeared that they related to the way that the templating system (Blaze?) applied diffs to the DOM. Digging into it any further was beyond my abilities, but perhaps @sorvell can figure out what about the timing is preventing patch-dom from working here (because it appears to me that all of the elements are getting patched correctly in Safari, but I'm guessing somehow the way the elements are created and/or added to the DOM is foiling patch-dom.

@emmanuelbuah
Copy link

@arthurevans @ebidel You guys are awesome. Thanks for your continuing effort. @sorvell Your help to figure out what's foiling patch-dom in relation to this issue will be greatly appreciated.

@stale
Copy link

stale bot commented Mar 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 13, 2020
@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically closed after being marked stale. If you're still facing this problem with the above solution, please comment and we'll reopen!

@stale stale bot closed this as completed Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants