Shorthand expansion is busted #51
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm having the same issue. Would be great if juice parsed both shorthand and long properties when determining what to inline. |
|
Good stuff. |
|
I like the two-pass approach suggested in the issue. |
parshap
added a commit
to parshap/juice
that referenced
this pull request
Jan 15, 2015
Makes properties that affect each other (e.g., "padding" and "padding-bottom") work as expected. Fixes test cases from Automattic#51 (see Automattic#51).
|
Believe this was resolved by #96 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's a case where the shorthand form of
paddingbehaves differently before and after inlining.Using the styles directly, browsers set
padding-bottom: 0pxon the<p/>. Juice doesn't expandpadding: 0pxtopadding-bottom, so the<p/>ends up with bothpadding: 0px; padding-bottom: 10px.Anyone else caught by this? My only idea for fixing it is to expand shorthand properties as they're [parsed])(https://github.com/LearnBoost/juice/blob/master/lib/juice.js#L112). This could balloon style length. Might be worth another pass after inlining to combine styles back into shorthands where it's safe. Thoughts?