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

Fixed problem with non-integer line height #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

t-schroeder
Copy link

This library couldn't handle non-integer line heights.

E.g. if you want to clamp a text at two lines and the line height is 22.5px the code would parseInt that to 22. Multiplied by two that's a max height of 44px. However, the browser displays two lines with height 45px. 45 > 44, so another line would be clamped so that only one line is left.

Solution: Replaced some parseInt calls with parseFloat.

Another problem: Say you have a line height of 20.89px and you want to clamp at two lines again. Now multiplied by two that makes a max height of 41.78px. However, the browser reports the two lines of text as height 42px. Since 42 > 41.78 another line would be clamped again so that only one line is left.

Solution: Math.ceil the max height.

aamir1995 added a commit to aamir1995/clamp.ts that referenced this pull request Jan 31, 2022
aamir1995 added a commit to aamir1995/clamp.ts that referenced this pull request Jan 31, 2022
…#3)

* fix": rename interface.

* fix: https://github.com/josephschmitt/Clamp.js/pull/18/files

* fix: don't append 'px' to opt.clamp. it already has it.

copied from josephschmitt/Clamp.js#20

* fix initial height check.

copied from: josephschmitt/Clamp.js#44

* fix: minor type

* fix: return if `target` is not defined in `truncate`

* fix: pass jshint

copied from: josephschmitt/Clamp.js#54

* fix: problem with non-integer line height.

partially copied from: josephschmitt/Clamp.js#66

* update readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant