bezoerb / text-metrics

An efficient text measurement set for the browser
MIT License
63 stars 4 forks source link

fix(lines-wordwrap): support wordbreak and hyphens option on lines method #3

Closed arnaud-zg closed 6 years ago

arnaud-zg commented 6 years ago

Details on issue #2

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.8%) to 84.307% when pulling 78ccfdd97773273de44e61bc09e650f50e3612e0 on arnaud-zg:fix/lines-support-long-word into 8740058ddab2f1ceb0b556439d22f92377755ada on bezoerb:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.8%) to 84.307% when pulling 3862a096662496009b1a29fcb270b87a7c3605ff on arnaud-zg:fix/lines-support-long-word into 8740058ddab2f1ceb0b556439d22f92377755ada on bezoerb:master.

bezoerb commented 6 years ago

Thanks @arnaud-zg for working on this :) I'll take a deeper look at the PR tomorrow.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.6%) to 84.133% when pulling 5dfa4471b2164103dbafd449c7df46b9ba78956b on arnaud-zg:fix/lines-support-long-word into 8740058ddab2f1ceb0b556439d22f92377755ada on bezoerb:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.6%) to 84.133% when pulling b3ef288fcbacb936d92582e70351066a1cb54ab8 on arnaud-zg:fix/lines-support-long-word into 8740058ddab2f1ceb0b556439d22f92377755ada on bezoerb:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.6%) to 84.133% when pulling 53adbb8cf744a8304c6228c55fbda0c1d36f22c1 on arnaud-zg:fix/lines-support-long-word into 8740058ddab2f1ceb0b556439d22f92377755ada on bezoerb:master.

arnaud-zg commented 6 years ago

It's been a while that my pull request is on pending status, actually I need this feature on my project. It would be great if it can be merged this week.

bezoerb commented 6 years ago

@arnaud-zg: sorry for taking so long to come back. This implementation may compute wrong values as the browser would not break the words like this (won't add - at the end) I think it's important that the result reflects the behavior from the browser. Did you know that you can use the word-break: 'break-all' css rule (either in css or overwrite in your call)

 textMetrics(el).lines(text, {}, {'word-break': 'break-all'})
/*
[ 'Craspharetraph',
  'aretragravida.Vi',
  'vamusconsequ',
  'atlacusvelposu',
  'erecongue.Duis',
  'aloremvitaeexa',
  'uctorscelerisqu',
  'enoneuturpis.Ut',
  'imperdietmagn',
  'asitametjustobi',
  'bendumvehicul',
  'a.' ]
*/

Browser: image

Sorry to not mention this before. Does this help with your project or do you depend on text-metrics adding the ­ to each line? You should also keep in mind, that adding the separator will result in grammatical errors

arnaud-zg commented 6 years ago

According to word-break and hyphens documentation, I updated my code and added those two option. Case 1

textMetrics(el).lines(text, {}, {wordBreak: 'break-all'})
/*
[ 'Craspharetraph',
  'aretragravida.Vi',
  'vamusconsequ',
  'atlacusvelposu',
  'erecongue.Duis',
  'aloremvitaeexa',
  'uctorscelerisqu',
  'enoneuturpis.Ut',
  'imperdietmagn',
  'asitametjustobi',
  'bendumvehicul',
  'a.' ]
*/

Case 2

textMetrics(el).lines(text, {}, {hyphens: 'auto'})
/*
[ 'Craspharetraph-',
  'aretragravida.Vi-',
  'vamusconsequ-',
  'atlacusvelposu-',
  'erecongue.Duis-',
  'aloremvitaeexa-',
  'uctorscelerisqu-',
  'enoneuturpis.Ut-',
  'imperdietmagn-',
  'asitametjustobi-',
  'bendumvehicul-',
  'a.' ]
*/
coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.9%) to 84.42% when pulling 9f2116a1034e6be80f37c8edee01d95e3258b2db on arnaud-zg:fix/lines-support-long-word into 8740058ddab2f1ceb0b556439d22f92377755ada on bezoerb:master.

arnaud-zg commented 6 years ago

Up

bezoerb commented 6 years ago

Sorry @arnaud-zg, i don't think it's a good idea to add this feature as this won't match the expected behavior. When doing auto hyphens this should be done properly with grammatical correct results.

arnaud-zg commented 6 years ago

:+1: