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(ios): label measure correct height when using custom numberOfLines #9945

Conversation

mukaschultze
Copy link
Contributor

@mukaschultze mukaschultze commented Jun 29, 2022

PR Checklist

What is the current behavior?

Right now, iOS labels include all the lines on the height measurement and do not respect the value specified by the numberOfLines property.

What is the new behavior?

The new behavior respects the numberOfLines property and properly measures the label height. When the user doesn't specify a custom value for the property it'll be zero and thus all the lines will be considered in the measurement, just as before.

Fixes #6037

@cla-bot cla-bot bot added the cla: yes label Jun 29, 2022
@NathanWalker
Copy link
Contributor

Thank you 🙏

@@ -104,7 +104,7 @@ export class Label extends TextBase implements LabelDefinition {
private _measureNativeView(width: number, widthMode: number, height: number, heightMode: number): { width: number; height: number } {
const view = <UILabel>this.nativeTextViewProtected;

const nativeSize = view.textRectForBoundsLimitedToNumberOfLines(CGRectMake(0, 0, widthMode === 0 /* layout.UNSPECIFIED */ ? Number.POSITIVE_INFINITY : layout.toDeviceIndependentPixels(width), heightMode === 0 /* layout.UNSPECIFIED */ ? Number.POSITIVE_INFINITY : layout.toDeviceIndependentPixels(height)), 0).size;
const nativeSize = view.textRectForBoundsLimitedToNumberOfLines(CGRectMake(0, 0, widthMode === 0 /* layout.UNSPECIFIED */ ? Number.POSITIVE_INFINITY : layout.toDeviceIndependentPixels(width), heightMode === 0 /* layout.UNSPECIFIED */ ? Number.POSITIVE_INFINITY : layout.toDeviceIndependentPixels(height)), view.numberOfLines).size;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be a breaking change and thus targeting 8.3 however we should regression test with this due to fact this would not default to 0 as it was this way anymore.
https://developer.apple.com/documentation/uikit/uilabel/1620539-numberoflines

The default value for this property is 1. To remove any maximum limit, and use as many lines as needed, set the value of this property to 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NathanWalker Yes, the default native value is 1, but on {N} setting textWrap to true (or the whiteSpace style) sets it to 0, and this _measureNativeView method is only called when textWrap is true, otherwise it uses layout.measureNativeView.

Following the code path I don't see any way that this is called with the value of 1 unless the user sets the numberOfLines prop to 1 explicitly

Copy link
Contributor

Choose a reason for hiding this comment

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

Great analysis @mukaschultze I'm going to include this on the next 8.3.x alpha - we are beginning to qualify things around the next minor and would make great candidate for that at any rate. Thanks for your effort here.

@NathanWalker NathanWalker added this to the 8.3 milestone Jun 30, 2022
@NathanWalker NathanWalker changed the base branch from master to alpha June 30, 2022 20:53
@NathanWalker NathanWalker changed the title fix(ios/label): measure correct height when using custom numberOfLines for iOS labels fix(ios): label measure correct height when using custom numberOfLines Jun 30, 2022
@NathanWalker NathanWalker merged commit 441d1b4 into NativeScript:alpha Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Label nativeElement numberOfLines doesn't resize frame
2 participants