Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

2.0.4 breaks measureWidth #30

Closed
namjul opened this issue Jan 24, 2017 · 11 comments
Closed

2.0.4 breaks measureWidth #30

namjul opened this issue Jan 24, 2017 · 11 comments

Comments

@namjul
Copy link

namjul commented Jan 24, 2017

I started using this great lib today but had a problems with it by not truncating to specified lines. It always truncated to more lines. I figured it this does not happen in 2.0.3.
When i revert the change made here: 324d1bf it works again.

@pablosichert
Copy link
Owner

Could you provide me with information about

  • which font-related CSS rules are applied to your truncated text
  • does npm test succeed locally for both revisions
  • screenshots

So I can help figuring out where the problem might be

@namjul
Copy link
Author

namjul commented Jan 25, 2017

The test are working in master and v2.0.3
Here a gif with 2.0.4:
output
2.0.3:
output2
The sample text has no styles applied.
Code:

<Truncate lines={1}>
  lkajsdöfasd aöskldjföalkjdsfölakjdöfiaüoeiasdfölajsdlfjalekjrbalsdjbflasjbdfabsdaö
</Truncate>

@pablosichert
Copy link
Owner

You are right, that commit 324d1bf is indeed faulty - which I'm going to revert on master.

The canvas context.font API does not expect a letter spacing / word spacing argument.

Looks like this might be a viable workaround.

pablosichert added a commit that referenced this issue Jan 25, 2017
context.font does not expect letter spacing / word spacing argument.
See #30 for more details.

This reverts commit c4880ec.
@namjul
Copy link
Author

namjul commented Jan 25, 2017

Thx for your fast response.

@thomasbaldwin
Copy link

@pablosichert did this workaround ever get added?
http://stackoverflow.com/a/40469992

@pablosichert
Copy link
Owner

I just tested some code:

const canvas = document.createElement('canvas');
// document.body.append(canvas);
canvas.style.letterSpacing = '1px';
const context = canvas.getContext('2d');
context.measureText('foo');

It only affects the measuring in Chrome and only if you uncomment the second line, which means that it only works if the canvas is mounted to the DOM.

I wouldn't like to add code that is so dependent on the browser.

@arasmussen
Copy link

arasmussen commented Apr 16, 2019

A better solution is to manually calculate letter/word spacing in measureWidth.

const targetStyle = window.getComputedStyle(target);
const letterSpacing = parseSpacing(targetStyle['letter-spacing']);
const wordSpacing = parseSpacing(targetStyle['word-spacing']);

const context = this.getCanvasContext(targetStyle);
const widthWithoutSpacing = context.measureText(text).width;
const widthWithSpacing = widthWithoutSpacing + (
  letterSpacing * (text.length - 1) +
  wordSpacing * (text.trim().split(/\s+/).length)
);

It works in all browsers, not just Chrome.

@pablosichert
Copy link
Owner

The previously mentioned solution would also work – as in not break. But the behavior would diverge depending on the browser.

I suppose you are suggesting to introduce a parseSpacing method that falls back to 0?

I think I'd rather use a partially-supported implementation than doing it manually, unless that would mean supporting a wider selection of browsers.

@arasmussen
Copy link

But the behavior would diverge depending on the browser.

In theory, every browser should calculate letter/word spacing the same way.

Manually calculating letter/word spacing, rather than relying on the browser to calculate it via canvasContext.measureText, works in every browser.

@pablosichert
Copy link
Owner

I see your point, could be worth a shot.

Would you like to open a PR for that? I'd assist you in updating the test

@arasmussen
Copy link

arasmussen commented Apr 19, 2019

Will take a look!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants