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

template is="dom-if" throws if template contains comment #2029

Closed
ebidel opened this issue Jul 4, 2015 · 4 comments
Closed

template is="dom-if" throws if template contains comment #2029

ebidel opened this issue Jul 4, 2015 · 4 comments

Comments

@ebidel
Copy link
Contributor

ebidel commented Jul 4, 2015

Given an dom-if template with a comment:

<template is="dom-if" if="{{fetchingMail}}">
  <!-- div class="layout vertical center-center" style="height:100%">
    <paper-spinner active></paper-spinner>
  </div -->
  <div id="refresh" class="fit layout vertical center-center">
    ...
  </div>
</div>
</template>

Polymer 1.0.5 throws an error trying to display: none it:

screen shot 2015-07-04 at 12 14 54 pm

@masonlouchart
Copy link

I worked a little bit on the dom-if yesterday. This error happens if the dom-if contains a node without style property (like text_node, comment_node). I'll update my pull request for this case. Please, see the PR #2027 .

@wesleycho
Copy link

Here is a simple reproduction (code here).

The _showHideChildren method in dom-if needs to correctly handle comment nodes - what would the desired behavior be for this situation?

I can file a PR if you like given guidance on the proper implementation here.

Edit: Oh, someone worked on this from the time I went to sleep heh.

@kevinpschaaf
Copy link
Member

This is a dupe of #1786.

This PR will be merged this week: #1992

@masonlouchart
Copy link

Should I make a new PR to keep only the first commit of the PR #2027 who solves the issue #1725 ?

Edit:
Or maybe you could keep the PR #2027 who contains also the same fix as yours ( #1992 ) ?

Edit2:
I have got the answer, yours has been merged few minutes ago. Sorry for the flood of comments. I'll make a new PR for the issue #1725 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants