-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix regression with imported css #4991
Conversation
Fix case where `<link rel="import" type="css">` is the only styling for an element Fixes #4986
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like an easy fix, just one question regarding implementation (but given the test passes it is correct).
lib/mixins/element-mixin.html
Outdated
let templateStyle = templateStyles[templateStyleIndex]; | ||
// if no styles in the template, fall back to first element child of template | ||
let templateStyle = templateStyles.length ? | ||
templateStyles[templateStyleIndex] : template.content.firstElementChild; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lacking the knowledge, but I assume the firstElementChild
represents the style
tag represents the .css
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just says put any non-local styles either before the closest local template style or at the top of the template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the <link>
element won't make it into the template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will still throw if there's no firstElementChild
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, good point. Let me make the smallest possible repro.
Always put `<link rel="import" type="css">` styles at the top
Fix case where
<link rel="import" type="css">
is the only styling foran element
Fixes #4986