Skip to content
This repository has been archived by the owner on Jan 4, 2019. It is now read-only.

Make sure inlined style imports are ordered correctly #577

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

dfreedm
Copy link
Member

@dfreedm dfreedm commented Jul 7, 2017

Previously, <link rel="import" type="css"> styles were inlined by
prepending to the top of the <dom-module> template.

However, if there were multiple imports, the order would be reversed,
which can cause incorrect styling when overriding.

This commit makes sure the order remains correct for multiple style
imports.

Fixes #575

Previously, `<link rel="import" type="css">` styles were inlined by
prepending to the top of the `<dom-module>` template.

However, if there were multiple imports, the order would be reversed,
which can cause incorrect styling when overriding.

This commit makes sure the order remains correct for multiple style
imports.

Fixes #575
@dfreedm dfreedm requested review from usergenic and kevinpschaaf July 7, 2017 18:25
this.prepend(domTemplate.childNodes[0], style);
} else {
// put this style behind the last polymer external style
dom5.insertBefore(domTemplate.childNodes[0], nextSibling(lastPolymerExternalStyle), style);
Copy link
Member

Choose a reason for hiding this comment

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

Seems like it would be easier to have the promises resolve with the style, and then in the then take the array of styles from Promise.all and append them in one go?

Or else just reverse the array of links to make inserting easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm really not inclined to go refactor this deprecated branch too much.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, LGTM then

@stramel
Copy link

stramel commented Jul 7, 2017

I thought the <link rel="import" type="css" was deprecated?

@dfreedm
Copy link
Member Author

dfreedm commented Jul 7, 2017

@stramel deprecated, but not removed for Polymer 1.x, nor for 2.x.

Copy link
Contributor

@usergenic usergenic left a comment

Choose a reason for hiding this comment

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

LGTM!

Mergin'

@usergenic usergenic merged commit 3761884 into 1.x Jul 7, 2017
@usergenic usergenic deleted the 1.x-fix-reordered-style-imports branch July 7, 2017 21:54
usergenic pushed a commit that referenced this pull request Jul 7, 2017
* Port of logic fix from #575
* Added tests to verify order of inlined stylesheets is correct.
* Updated CHANGELOG wrt the fix for reversed stylesheet imports.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants