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

[ASTextKitFontSizeAdjuster] Replace use of boundingRectWithSize:options:context: with boundingRectForGlyphRange: inTextContainer: #251

Merged
merged 2 commits into from
May 11, 2017

Conversation

rcancro
Copy link
Contributor

@rcancro rcancro commented May 8, 2017

boundingRectWithSize:options:context: started returning different values for the same strings between iOS 10.2 and iOS 10.3. Switching to using NSLayoutManager’s boundingRectForGlyphRange: inTextContainer: fixed this. It also makes sure we are consistent with what ASTextKitTailTruncater uses.

…ns:context: with boundingRectForGlyphRange: inTextContainer:

`boundingRectWithSize:options:context:`  started returning different values for the same strings between iOS 10.2 and iOS 10.3. Switching to using `NSLayoutManager`’s `boundingRectForGlyphRange: inTextContainer:` fixed this. It also makes sure we are consistent with what `ASTextKitTailTruncater` uses.
@CLAassistant
Copy link

CLAassistant commented May 8, 2017

CLA assistant check
All committers have signed the CLA.

@nguyenhuy nguyenhuy requested a review from maicki May 10, 2017 18:07
Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man NSAttributedString measurement/drawing methods are f***ucked! This is a nice improvement anyway, since it's a little silly to create a layout manager & text container, and then call the attributed string method in the same class.

@Adlai-Holler Adlai-Holler merged commit 002f470 into TextureGroup:master May 11, 2017
@maicki
Copy link
Contributor

maicki commented May 11, 2017

@rcancro Thanks for working on that!

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.

4 participants