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

Fix for mixins declaration with space before colon #2322

Merged
merged 1 commit into from
Oct 26, 2015

Conversation

nazar-pc
Copy link
Contributor

--working-mixin: {
    color: red;
}
--broken-mixin : {
    color: red;
}

With this fix both work fine.

nazar-pc added a commit to nazar-pc/CleverStyle-Framework that referenced this pull request Aug 20, 2015
@nazar-pc nazar-pc force-pushed the fix-for-mixins-parsing branch from 60cb3bc to d3bda2b Compare August 27, 2015 06:26
@tjsavage
Copy link
Contributor

Thanks for the pull request - apologies for the delay here. This looks like a solid fix - would you mind squashing the commits?

@nazar-pc nazar-pc force-pushed the fix-for-mixins-parsing branch from 0d31daf to e91048c Compare September 30, 2015 19:38
@nazar-pc
Copy link
Contributor Author

Rebased against master and squashed into single commit.

@samccone
Copy link
Contributor

samccone commented Oct 5, 2015

👍 lgtm

Allow any space character or even `{` and `}` (before and after capturing pattern correspondingly) as pattern boundaries instead of new lines only.
In minified sources there might be no space, semicolon or line start, so we need to account that as well.
@nazar-pc nazar-pc force-pushed the fix-for-mixins-parsing branch from e91048c to 883aa5c Compare October 8, 2015 22:08
nazar-pc added a commit to nazar-pc/CleverStyle-Framework that referenced this pull request Oct 8, 2015
@samccone
Copy link
Contributor

I think this should be good to go, @tjsavage anything else here?

@tjsavage
Copy link
Contributor

LGTM - @azakus any idea why the tests would be failing?

@dfreedm
Copy link
Member

dfreedm commented Oct 26, 2015

It doesn't look like these tests have been run since our fix in webcomponentsjs for the failing IE tests.

@nazar-pc
Copy link
Contributor Author

How to trigger it?

@dfreedm
Copy link
Member

dfreedm commented Oct 26, 2015

The IE failures are fixed with 1.2.0, this PR should pass after merging.

@dfreedm
Copy link
Member

dfreedm commented Oct 26, 2015

@nazar-pc already did. This PR was using the old homegrown CI, new PRs will use Travis CI.

@dfreedm
Copy link
Member

dfreedm commented Oct 26, 2015

Confirm IE 10 and IE 11 passing after merging with Polymer on master.

@dfreedm
Copy link
Member

dfreedm commented Oct 26, 2015

LGTM

dfreedm added a commit that referenced this pull request Oct 26, 2015
Fix for mixins declaration with space before colon
@dfreedm dfreedm merged commit 30da1b3 into Polymer:master Oct 26, 2015
@nazar-pc nazar-pc deleted the fix-for-mixins-parsing branch November 6, 2015 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants