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

Fix bug in sizeWithFont #1917

Merged
merged 3 commits into from
Feb 9, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Frameworks/UIKit/NSString+UIKitAdditions.mm
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,8 @@ - (CGSize)_sizeWithAttributes:(NSDictionary<NSString*, id>*)attributes constrain
size.width = std::numeric_limits<CGFloat>::max();
}

if (size.height == 0.0) {
size.height = std::numeric_limits<CGFloat>::max();
}
// Return the height that the text actually requires rather than the constrained size
Copy link
Contributor

@yiyang-msft yiyang-msft Feb 4, 2017

Choose a reason for hiding this comment

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

if (size.height == 0.0) { [](start = 4, length = 25)

no wonder with this, it works. :) #Resolved

size.height = std::numeric_limits<CGFloat>::max();
Copy link
Contributor

Choose a reason for hiding this comment

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

so wait the expectation of the constrainedToSize: contract is that the height passed in doesn't matter at all? that seems... wrong...

Copy link
Contributor

Choose a reason for hiding this comment

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

that does seem a bit odd


In reply to: 99448184 [](ancestors = 99448184)

Copy link
Contributor Author

@aballway aballway Feb 6, 2017

Choose a reason for hiding this comment

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

@yiyang-msft and I reproduced this on the reference platform, but it's likely that this only applies to a case where we are only drawing one line but unable to fit it. I'm going to be doing more testing to see where height is respected and where it is not, and if this also applies to CTFramesetterSuggestFrameSizeWithConstraints. #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

@alballw@microsoft.com , make sense.


In reply to: 99671953 [](ancestors = 99671953)

Copy link
Contributor

Choose a reason for hiding this comment

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

also,please verified with MM. check with the introduction label on first screen.


In reply to: 99942712 [](ancestors = 99942712,99671953)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this change the introduction label in Islandwood house is being scaled properly


In reply to: 99943020 [](ancestors = 99943020,99942712,99671953)


return CTFramesetterSuggestFrameSizeWithConstraints(framesetter.get(), CFRangeMake(0, self.length), nullptr, size, nullptr);
}
Expand Down
6 changes: 6 additions & 0 deletions tests/unittests/UIKit/NSString+UIKitAdditionsTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,10 @@
[@"TEST" sizeWithFont:[UIFont systemFontOfSize:12.0f] constrainedToSize:CGSizeZero lineBreakMode:NSLineBreakByWordWrapping];
EXPECT_LT(0.0, size.width);
EXPECT_LT(0.0, size.height);
}

TEST(NSString_UIKitAdditions, ShouldNotConstrainHeightWhenTextIsLarger) {
CGSize givenSize = { 0, 15 };
CGSize size = [@"TEST" sizeWithFont:[UIFont systemFontOfSize:144] constrainedToSize:givenSize];
EXPECT_GT(size.height, givenSize.height);
}