Changing parseInt of lineHeight to parseFloat to avoid rounding errors

Tuizi commented 8 years ago

Please add unit tests to cover your use case and to be sure nothing else is broken

rogeriotaques commented 8 years ago

I'd like to suggest you another approach to fix that:

    if (this.options.lineHeight === 'auto') {

      ...

      if (lineHeightCss !== "normal") {
        lineHeight = Math.round(parseFloat(lineHeightCss));
      }

      this.options.lineHeight = lineHeight;
    }

    if (this.options.maxHeight === undefined) {
      this.options.maxHeight = Math.round(parseFloat(this.options.lines)) * Math.round(parseFloat(this.options.lineHeight));
    }

    ...

What do you think? It has worked nicely for me!

Tuizi commented 8 years ago

Yes it's correct. But I'm not sure someone will one day say: I want 1.6 lines :) (this.options.lines).

So I think the best think to do is: this.options.maxHeight = Math.round(parseFloat(this.options.lines)) * parseFloat(this.options.lineHeight));

@rogeriotaques Can you create a PR with a fix and with test? @drukepple did not answer. Or if you are patient I will do it myself. This fix will be in the version 1.2 anyway

rogeriotaques commented 7 years ago

Fabien,

I'm so sorry for this late reply. I've had busy days lately and could not manage to reply or pull request.

On Wed, 19 Oct 2016 at 22:42 Fabien Rogeret notifications@github.com wrote:

Yes it's correct. But I'm not sure someone will one day say: I want 1.6 lines :) (``this.options.lines).

So I think the best think to do is: this.options.maxHeight = Math.round(parseFloat(this.options.lines)) * parseFloat(this.options.lineHeight));

@rogeriotaques https://github.com/rogeriotaques Can you create a PR with a fix and with test? @drukepple https://github.com/drukepple did not answer. Or if you are patient I will do it myself. This fix will be in the version 1.2 anyway

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffchan/truncate.js/pull/32#issuecomment-254815943, or mute the thread https://github.com/notifications/unsubscribe-auth/ABY3-4MlqlNb4MLiCZHJD61OZpaJnhiJks5q1h5HgaJpZM4JTtT6 .

rogeriotaques commented 4 years ago

Hey, @Tuizi!

Oh my Gosh! It has been quite a (very) long time since you proposed that solution and I totally forgot to give you a PR, really sorry. 🙇‍♂️

I checked the release page and I don't think the version 1.2 was rolled out, am I right?

Is this project still active?

jeffchan / truncate.js

Changing parseInt of lineHeight to parseFloat to avoid rounding errors #32