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

fixed text measurement regression #978

Merged
merged 3 commits into from Feb 9, 2017
Merged

fixed text measurement regression #978

merged 3 commits into from Feb 9, 2017

Conversation

kevinvangelder
Copy link
Contributor

When we don't round up the text always gets clipped.

@@ -213,8 +213,8 @@ private static YogaSize MeasureText(ReactTextShadowNode textNode, YogaNode node,
var normalizedHeight = YogaConstants.IsUndefined(height) ? double.PositiveInfinity : height;
textBlock.Measure(new Size(normalizedWidth, normalizedHeight));
return MeasureOutput.Make(
(float)textBlock.DesiredSize.Width,
(float)textBlock.DesiredSize.Height);
(float)Math.Ceiling(textBlock.DesiredSize.Width),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ceiling [](start = 32, length = 7)

That's strange that cropping occurs on WPF and not on UWP. We were going to use the same technique for UWP, but fixed the rounding issue in Yoga. facebook/yoga#300

Copy link
Contributor

Choose a reason for hiding this comment

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

does that mean we should publish a new package (same hash they published to npm/cocoapods), and then update our version reference to fix the issue?

Copy link
Collaborator

@rozele rozele Feb 9, 2017

Choose a reason for hiding this comment

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

I actually think this is also a problem on UWP still too. Can you add the Math.Ceiling to UWP as well?

@rozele
Copy link
Collaborator

rozele commented Feb 9, 2017

:shipit:

@rozele rozele merged commit 5e7839f into microsoft:master Feb 9, 2017
rozele pushed a commit that referenced this pull request Feb 10, 2017
* fixed text measurement regression

* also apply fix to UWP text

* also applied fix to textinput
rozele pushed a commit that referenced this pull request Feb 15, 2017
* fixed text measurement regression

* also apply fix to UWP text

* also applied fix to textinput
rozele pushed a commit that referenced this pull request Feb 15, 2017
* fixed text measurement regression

* also apply fix to UWP text

* also applied fix to textinput
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.

3 participants