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

Attributed Truncation Token as URL Link UI fix #697

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hermandrew
Copy link

Correctly calculating the link range when added in the attributed truncation token.

The offset was previously calculated by taking the last line, and saying that the range of the link (and attribution token) basically started at (line length) - (attribution token length). This only works for monospaced fonts. My implementation looks at the first run of the truncated line, and uses its length as the starting position for the truncation token.

Here is a visual representation of the bug:

Before

before
After

after

Andrew Herman added 2 commits July 14, 2016 10:47
…ncation token.

The offset was previously calculated by taking the last line, and saying that the range of the link (and attribution token) basically started at (line length) - (attribution token length).  This only works for monospaced fonts.  My implementation looks at the first run of the truncated line, and uses its length as the starting position for the truncation token.
@codecov-io
Copy link

codecov-io commented Jul 14, 2016

Current coverage is 76.48%

Merging #697 into master will increase coverage by 6.25%

@@             master       #697   diff @@
==========================================
  Files             2          2          
  Lines           944        944          
  Methods         112        138    +26   
  Messages          0          0          
  Branches        225        132    -93   
==========================================
+ Hits            663        722    +59   
  Misses          128        128          
+ Partials        153         94    -59   

Sunburst

Powered by Codecov. Last updated by 6deb9e4...a3c5a80

@segiddins
Copy link
Member

Could you add test coverage for this fix? Thanks!

@ancorio
Copy link

ancorio commented Apr 11, 2018

UI is fine, but touch doesn't work coz characterIndexAtPoint: will not take truncation token into account.

@maltsugar
Copy link

click lose efficacy

@veila
Copy link

veila commented Apr 26, 2019

It's still wrong if text like this
"
M
M
M
M
M
M..See more "
text short and break lines so much, the text "See more" can not touch

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.

None yet

6 participants