Interpolation Problem (LibSass related?) - Found Fix #23

Closed
igregson opened this Issue Jul 9, 2014 · 10 comments

Comments

Projects
None yet
6 participants
@igregson

igregson commented Jul 9, 2014

Greetings,

First off, love the project. Many thanks for your work.

Now, I'm using LibSass (via GulpSass) to compile. In general, LibSass is always a bit slow to implement new Sass features (though it's incredibly fast, which is why I use it). Anyways, I found that some of the typeplate functions were being returned with spaces between measurements and their values (like font-size: 32 rem instead of `font:size: 32rem'), so naturally this was read by the browser and the User Agent Stylesheet defaults were being used.

Took me a while to realize what was going on, but here's the fix that worked for me:

Around line 200 of _typeplate.scss

@function context-calc($scale, $base, $value) {
  @return ($scale/$base) + $value;    // was @return ($scale/$base)#{$value};
}
@function measure-margin($scale, $measure, $value) {
  @return ($measure/$scale) + $value;    /// was @return ($measure/$scale)#{$value};
}

And in the line 225 block:

@if $value == 
  font-size: #{$scale + px};    // was font-size: $scale#{px};
}

Not sure if this should be integrated into a pull request or if you'd rather leave things as they were before for some reason. Not sure if this will cause issues for other Sass compiling methods, but it's working for me.

@grayghostvisuals

This comment has been minimized.

Show comment Hide comment
@grayghostvisuals

grayghostvisuals Jul 10, 2014

Contributor

Thanks @Isaaki for the kind words. This smells like a LibSass problem that might warrant an isolated case filed with them as well (I could be wrong of course, but just my gut reaction). To start I need to know the following…

  1. LibSass Version
  2. GulpSass Version
  3. Sass Version
Contributor

grayghostvisuals commented Jul 10, 2014

Thanks @Isaaki for the kind words. This smells like a LibSass problem that might warrant an isolated case filed with them as well (I could be wrong of course, but just my gut reaction). To start I need to know the following…

  1. LibSass Version
  2. GulpSass Version
  3. Sass Version
@rhumbus

This comment has been minimized.

Show comment Hide comment
@rhumbus

rhumbus Sep 18, 2014

Same issue here, using:

  1. Typeplate 1.1.14
  2. gulp-sass 0.7.3
  3. node-sass 0.9.3
  4. libsass 1.3.14

Thanks, @Isaaki, for the workaround.

Just to say, I love Typeplate. It has become the only css framework I ever start a project with - it doesn't dictate design patterns, yet brings the subliminal magic needed for harmony.
Good work.

p.s - will it help if I add a comment on that issue you opened on the libsass git?

rhumbus commented Sep 18, 2014

Same issue here, using:

  1. Typeplate 1.1.14
  2. gulp-sass 0.7.3
  3. node-sass 0.9.3
  4. libsass 1.3.14

Thanks, @Isaaki, for the workaround.

Just to say, I love Typeplate. It has become the only css framework I ever start a project with - it doesn't dictate design patterns, yet brings the subliminal magic needed for harmony.
Good work.

p.s - will it help if I add a comment on that issue you opened on the libsass git?

@grayghostvisuals

This comment has been minimized.

Show comment Hide comment
@grayghostvisuals

grayghostvisuals Sep 18, 2014

Contributor

@rhumbus Thanks! That's so awesome to hear. I use it all the time myself ;)

If you feel like adding to the issue thread on libsass because you have something meaningful to add by all means, but I wouldn't clog it up just because.

Contributor

grayghostvisuals commented Sep 18, 2014

@rhumbus Thanks! That's so awesome to hear. I use it all the time myself ;)

If you feel like adding to the issue thread on libsass because you have something meaningful to add by all means, but I wouldn't clog it up just because.

@blustemy

This comment has been minimized.

Show comment Hide comment
@blustemy

blustemy Feb 13, 2015

Hi,

I'm still getting the error with the extra space before the unit with libSass. But this is the fix I'm using in _typeplate-functions.scss:

@function typl8-context-calc($scale, $base, $value) {
  @return #{$scale/$base}#{$value}; // chaining two interpolations
}

And:

@function typl8-measure-margin($scale, $measure, $value) {
  $pixelValue: $measure/$scale;
  $remValue: $pixelValue * $typl8-font-base;

  @if $value == rem {
    @return #{$pixelValue}#{$value};
  } @else if $value == em {
    @return #{$remValue/$scale}#{$value};
  } @else {
    @return #{$remValue}#{$value};
  }
}

And I love Typeplate too!

Thanks,

Fred

Hi,

I'm still getting the error with the extra space before the unit with libSass. But this is the fix I'm using in _typeplate-functions.scss:

@function typl8-context-calc($scale, $base, $value) {
  @return #{$scale/$base}#{$value}; // chaining two interpolations
}

And:

@function typl8-measure-margin($scale, $measure, $value) {
  $pixelValue: $measure/$scale;
  $remValue: $pixelValue * $typl8-font-base;

  @if $value == rem {
    @return #{$pixelValue}#{$value};
  } @else if $value == em {
    @return #{$remValue/$scale}#{$value};
  } @else {
    @return #{$remValue}#{$value};
  }
}

And I love Typeplate too!

Thanks,

Fred

@grayghostvisuals

This comment has been minimized.

Show comment Hide comment
@grayghostvisuals

grayghostvisuals Feb 13, 2015

Contributor

@blustemy If you click here you'll see this is still not resolved on libsass' end. Sorry, but it's in their field right now. Thanks btw for the kind words. I'm glad you find this project useful. I use it everyday myself :)

Contributor

grayghostvisuals commented Feb 13, 2015

@blustemy If you click here you'll see this is still not resolved on libsass' end. Sorry, but it's in their field right now. Thanks btw for the kind words. I'm glad you find this project useful. I use it everyday myself :)

@ramsaylanier

This comment has been minimized.

Show comment Hide comment
@ramsaylanier

ramsaylanier Feb 16, 2015

@blustemy +1 - this fixed worked for me.

@blustemy +1 - this fixed worked for me.

@grayghostvisuals

This comment has been minimized.

Show comment Hide comment
@grayghostvisuals

grayghostvisuals Feb 16, 2015

Contributor

Here's the fix I plan to implement in Sassmeister until I can merge the code into the project http://sassmeister.com/gist/f8197ffbac32689e84a8

Contributor

grayghostvisuals commented Feb 16, 2015

Here's the fix I plan to implement in Sassmeister until I can merge the code into the project http://sassmeister.com/gist/f8197ffbac32689e84a8

@HugoGiraudel

This comment has been minimized.

Show comment Hide comment
@HugoGiraudel

HugoGiraudel Feb 16, 2015

Just to clarify what is going on: there is indeed an issue on LibSass end with string interpolation. Actually, it is not the first one, and will probably not be the last. That being said, the issue should not be a problem for Typeplate whatsoever when doing things slightly more elegantly.

Basically the problem here is that we deal with units as if they were random strings living on the right of some numbers. This is not how it works. We gotta see units as if they were completely indivisible from numbers. Units cannot live on their own.

My point is, rather than appending the unit as a string at the end of a number, we better multiply that number by 1 of this unit (or add 0 of this unit, it works as well).

So instead of:

@function typl8-measure-margin($scale, $measure, $value) {
  $pixelValue: $measure/$scale;
  $remValue: $pixelValue * $typl8-font-base;

  @if $value == rem {
    @return $pixelValue#{$value};
  } @else if $value == em {
    @return ($remValue/$scale)#{$value};
  } @else {
    @return $remValue#{$value};
  }
}

We could have:

@function typl8-measure-margin($scale, $measure, $value) {
  $pixel-value: ($measure / $scale);
  $rem-value: ($pixelValue * $typl8-font-base);

  @if unit($value) == 'rem' {
    @return $pixel-value * $value;
  } @else if unit($value) == 'em' {
    @return ($rem-value / $scale) * $value;
  } @else {
    @return $rem-value * $value;
  }
}

Or a bit more elegant (at least to me):

@function typl8-measure-margin($scale, $measure, $value) {
  $pixel-value: ($measure / $scale);
  $rem-value: ($pixelValue * $typl8-font-base);

  @if unit($value) == 'rem' {
    $v: $pixel-value;
  } @else if unit($value) == 'em' {
    $v: ($rem-value / $scale);
  } @else {
    $v: $rem-value;
  }

  @return $v * $value;
}

So at the end of the day, all we need is to make $value 1 item of a unit, for instance 1px rather than a string like px.

Just to clarify what is going on: there is indeed an issue on LibSass end with string interpolation. Actually, it is not the first one, and will probably not be the last. That being said, the issue should not be a problem for Typeplate whatsoever when doing things slightly more elegantly.

Basically the problem here is that we deal with units as if they were random strings living on the right of some numbers. This is not how it works. We gotta see units as if they were completely indivisible from numbers. Units cannot live on their own.

My point is, rather than appending the unit as a string at the end of a number, we better multiply that number by 1 of this unit (or add 0 of this unit, it works as well).

So instead of:

@function typl8-measure-margin($scale, $measure, $value) {
  $pixelValue: $measure/$scale;
  $remValue: $pixelValue * $typl8-font-base;

  @if $value == rem {
    @return $pixelValue#{$value};
  } @else if $value == em {
    @return ($remValue/$scale)#{$value};
  } @else {
    @return $remValue#{$value};
  }
}

We could have:

@function typl8-measure-margin($scale, $measure, $value) {
  $pixel-value: ($measure / $scale);
  $rem-value: ($pixelValue * $typl8-font-base);

  @if unit($value) == 'rem' {
    @return $pixel-value * $value;
  } @else if unit($value) == 'em' {
    @return ($rem-value / $scale) * $value;
  } @else {
    @return $rem-value * $value;
  }
}

Or a bit more elegant (at least to me):

@function typl8-measure-margin($scale, $measure, $value) {
  $pixel-value: ($measure / $scale);
  $rem-value: ($pixelValue * $typl8-font-base);

  @if unit($value) == 'rem' {
    $v: $pixel-value;
  } @else if unit($value) == 'em' {
    $v: ($rem-value / $scale);
  } @else {
    $v: $rem-value;
  }

  @return $v * $value;
}

So at the end of the day, all we need is to make $value 1 item of a unit, for instance 1px rather than a string like px.

@grayghostvisuals

This comment has been minimized.

Show comment Hide comment
@grayghostvisuals

grayghostvisuals Feb 16, 2015

Contributor

@HugoGiraudel You the man! Thanks again!

Contributor

grayghostvisuals commented Feb 16, 2015

@HugoGiraudel You the man! Thanks again!

@blustemy

This comment has been minimized.

Show comment Hide comment
@blustemy

blustemy Feb 17, 2015

That's an excellent piece of advice @HugoGiraudel, I'll also use it on my projects! Thanks a lot.

That's an excellent piece of advice @HugoGiraudel, I'll also use it on my projects! Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment