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

Compatibility fix for hybrid views for Polymer 1 and 2 #37

Merged
merged 1 commit into from
Dec 19, 2017

Conversation

warpech
Copy link
Contributor

@warpech warpech commented Dec 19, 2017

Problem: juicy-html does not assign the model to <template is="dom-bind"> if it is wrapped in <dom-bind>. This makes it impossible to have hybrid views for Polymer 1 and 2

Solution: apply the mirrored version of the Polymer's own solution for this problem: https://github.com/Polymer/polymer/pull/4537/files

This was discussed in https://starcounter.slack.com/archives/C5PEXSXQE/p1513614571000221

…nd"> if it is wrapped in <dom-bind>. This makes it impossible to have hybrid views for Polymer 1 and 2

Solution: apply the mirrored version of the Polymer's own solution for this problem: https://github.com/Polymer/polymer/pull/4537/files

//mirrored implementation of this fix: https://github.com/Polymer/polymer/pull/4537/files
const firstElemChild = elem.firstElementChild;
if (firstElemChild && firstElemChild.getAttribute("is") === elem.localName) {
Copy link
Member

@tomalec tomalec Dec 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we fine to do this for <foo><bar is="foo"> as well? (Polymer implements it in dom-* elements only, so by design they are filtering only those.

Copy link
Contributor Author

@warpech warpech Dec 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you know Polymer implements it in dom-* elements only? Looking here it seems that they do it for any element names.

I think it is fine. If it becomes a problem, we can do a hot fix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why it works only for dom-* elements

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you paste a link to a line?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets hope <foo><bar is="foo"> will not become a feature by that time.

Copy link
Member

@tomalec tomalec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that LGTM

@warpech warpech merged commit b94d1af into 2.x Dec 19, 2017
@warpech
Copy link
Contributor Author

warpech commented Jan 9, 2018

Setting as “Finished” because it is:

@tomalec tomalec deleted the fix-hybrid-polymer branch March 29, 2020 12:32
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

Successfully merging this pull request may close these issues.

3 participants