search

Igosuki / compass-mixins

A collection of compass' stylesheet for bower dependencies and libsass
Other
592 stars 197 forks source link

Always define compact #86

Closed xzyfer closed 8 years ago

xzyfer commented 8 years ago

This implement of compact is functionally equivalent of the unofficial native one that has been permanently removed from LibSass. There is no risk in always using this version. The version of LibSass that had a compact function is extremely difficult to obtain, and is long unsupported.

More importantly defining function inside control structures is not valid in Sass and produces errors for people using this library.

Closes #73 Fixes #84 Fixes #85

alebelcor commented 8 years ago

I can't see why the build failed in Travis. Any clues?

xzyfer commented 8 years ago

The transition mixin has always been broken in Sass. I've updated so it now works as expected on both Node Sass and Sass.

tjenkinson commented 8 years ago

Yes please can this be merged?

Refs: https://github.com/clappr/clappr/pull/970

tjenkinson commented 8 years ago

FYI I've created a fork with this fix: https://github.com/tjenkinson/compass-mixins (because it looks like this might not be maintained anymore).

lemoustachiste commented 8 years ago

This is a blocker for me, I have fixed my initial issue by installing node-sass 3.7.0 but this remains. Is this going to get merged and released?

phazei commented 8 years ago

@Igosuki please merge these, many projects depend on it, thank you

shri3k commented 8 years ago

Agreed with @phazei. I don't know why node-sass didn't mark this as breaking change. @tjenkinson, I'm guessing your fork hasn't been published to npm has it? (even though I can use github's link in my package.json I'm a little lazy. 😜 )

xzyfer commented 8 years ago

As far as node-sass is concerned it wasn't a breaking change. You can read more about why in https://medium.com/@xzyfer/why-node-sass-broke-your-code-and-semver-1b3e409c57b9#.av46a780g On 13 May 2016 8:18 AM, "Yojan Shrestha" notifications@github.com wrote:

Agreed with @phazei https://github.com/phazei. I don't know why node-sass didn't mark this as breaking change. @tjenkinson https://github.com/tjenkinson, I'm guessing your fork hasn't been published to npm has it? (even though I can use github's link in my package.json I'm a little lazy. 😜 )

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/Igosuki/compass-mixins/pull/86#issuecomment-218902214

tjenkinson commented 8 years ago

@shri3k yes I just did

npm install tjenkinson-compass-mixins

https://www.npmjs.com/package/tjenkinson-compass-mixins

Hopefully this will start being maintained again though.

shri3k commented 8 years ago

@xzyfer Ah, that sort of clarifies things. @tjenkinson Nice. Thank you and agreed I'd rather like this to be maintained instead.

shri3k commented 8 years ago

@xzyfer Thank you for merging. I'm confirming that this works. 🎉