fix: label text wrapping inside flexbox layout (#5781)

NativeScript · May 8, 2018 · 481043f · 481043f
1 parent 7edf561
commit 481043f
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 2 deletions.
diff --git a/apps/app/ui-tests-app/flexbox/flexbox-4834.css b/apps/app/ui-tests-app/flexbox/flexbox-4834.css
@@ -0,0 +1,14 @@
+image {
+    width: 40;
+    height: 40;
+    flex-grow: 0;
+    flex-shrink: 0;
+}
+
+Label {
+    border-width: 1;
+    border-color: black;
+    padding: 0;
+    flex-grow: 1;
+    flex-shrink: 1;
+}
diff --git a/apps/app/ui-tests-app/flexbox/flexbox-4834.xml b/apps/app/ui-tests-app/flexbox/flexbox-4834.xml
@@ -0,0 +1,55 @@
+<Page xmlns="http://schemas.nativescript.org/tns.xsd" class="page">
+
+    <StackLayout width="200" class="p-20">
+
+        <!-- Android -->
+        <FlexboxLayout>
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p q r s t" />
+        </FlexboxLayout>
+
+        <FlexboxLayout>
+            <Image src="https://placehold.it/40x40" />
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p" />
+        </FlexboxLayout>
+
+        <FlexboxLayout flexWrap="wrap">
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p q r s t" />
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p q r s t" />
+        </FlexboxLayout>
+
+
+        <!-- iOS -->
+        <FlexboxLayout>
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p" />
+        </FlexboxLayout>
+
+        <FlexboxLayout>
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p q" />
+        </FlexboxLayout>
+
+        <FlexboxLayout>
+            <Image src="https://placehold.it/40x40" />
+            <Label textWrap="true" text="a b c d e f g h i j k l m" />
+        </FlexboxLayout>
+
+        <FlexboxLayout>
+            <Image src="https://placehold.it/40x40" />
+            <Label textWrap="true" text="a b c d e f g h i j k l m n" />
+        </FlexboxLayout>
+
+        <FlexboxLayout flexWrap="wrap">
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p" />
+            <Label textWrap="true" text="a b c d e f g h i j k l m n o p q" />
+        </FlexboxLayout>
+
+
+        <!-- Common -->
+        <FlexboxLayout alignItems="flex-start">
+            <Label textWrap="true" text="a b" style="flex-shrink:0;" />
+            <Label textWrap="true" text="a b c d e f g h i" />
+            <Label textWrap="true" text="a b c d e f g h i j" />
+            <Label textWrap="true" text="a b c d e f g h i j k" />
+        </FlexboxLayout>
+
+    </StackLayout>
+</Page>     
diff --git a/apps/app/ui-tests-app/flexbox/flexbox-main-page.ts b/apps/app/ui-tests-app/flexbox/flexbox-main-page.ts
@@ -17,6 +17,7 @@ export function loadExamples() {
     examples.set("flexrepeat", "flexbox/flexbox-repeater");
     examples.set("flex-perf", "flexbox/flexbox-perf-comparison");
     examples.set("flexbox-4143", "flexbox/flexbox-4143");
+    examples.set("flexbox-4834", "flexbox/flexbox-4834");
 
     return examples;
 }
diff --git a/tns-core-modules/ui/label/label.ios.ts b/tns-core-modules/ui/label/label.ios.ts
@@ -68,7 +68,11 @@ export class Label extends TextBase implements LabelDefinition {
             this._fixedSize = (widthMode === layout.EXACTLY ? FixedSize.WIDTH : FixedSize.NONE)
                 | (heightMode === layout.EXACTLY ? FixedSize.HEIGHT : FixedSize.NONE);
 
-            const nativeSize = layout.measureNativeView(nativeView, width, widthMode, height, heightMode);
+            // NOTE: utils.measureNativeView(...) relies on UIView.sizeThatFits(...) that 
+            // seems to have various issues when laying out UILabel instances.
+            // We use custom measure logic here that relies on overriden 
+            // UILabel.textRectForBounds:limitedToNumberOfLines: in TNSLabel widget. 
+            const nativeSize = this._measureNativeView(width, widthMode, height, heightMode);
             let labelWidth = nativeSize.width;
 
             if (this.textWrap && widthMode === layout.AT_MOST) {
@@ -85,6 +89,22 @@ 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.nativeViewProtected;
+
+        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;
+
+        nativeSize.width = layout.round(layout.toDevicePixels(nativeSize.width));
+        nativeSize.height = layout.round(layout.toDevicePixels(nativeSize.height));
+        return nativeSize;
+    }
+
     [whiteSpaceProperty.setNative](value: WhiteSpace) {
         const nativeView = this.nativeViewProtected;
         switch (value) {

diff --git a/tns-core-modules/ui/layouts/flexbox-layout/flexbox-layout.ios.ts b/tns-core-modules/ui/layouts/flexbox-layout/flexbox-layout.ios.ts
@@ -595,8 +595,25 @@ export class FlexboxLayout extends FlexboxLayoutBase {
                     } else {
                         accumulatedRoundError = rawCalculatedWidth - roundedCalculatedWidth;
                     }
-                    child.measure(makeMeasureSpec(roundedCalculatedWidth, EXACTLY), makeMeasureSpec(child.getMeasuredHeight(), EXACTLY));
+
+                    const childWidthMeasureSpec = makeMeasureSpec(roundedCalculatedWidth, EXACTLY);
+
+                    // NOTE: for controls that support internal content wrapping (e.g. UILabel) reducing the width 
+                    // might result in increased height e.g. text that could be shown on one line for larger 
+                    // width needs to be wrapped in two when width is reduced. 
+                    // As a result we cannot unconditionally measure with EXACTLY the current measured height
+                    const childHeightMeasureSpec = FlexboxLayout.getChildMeasureSpec(this._currentHeightMeasureSpec,
+                        lp.effectivePaddingTop + lp.effectivePaddingBottom + lp.effectiveMarginTop
+                        + lp.effectiveMarginBottom, lp.effectiveHeight < 0 ? WRAP_CONTENT : lp.effectiveHeight);        
+
+                    child.measure(childWidthMeasureSpec, childHeightMeasureSpec);
                     child.effectiveMinWidth = minWidth;
+
+                    // make sure crossSize is up-to-date as child calculated height might have increased
+                    flexLine._crossSize = Math.max(
+                        flexLine._crossSize,
+                        child.getMeasuredHeight() + lp.effectiveMarginTop + lp.effectiveMarginBottom
+                    );
                 }
                 flexLine._mainSize += child.getMeasuredWidth() + lp.effectiveMarginLeft + lp.effectiveMarginRight;
             } else {