From b27c05e52c5b36b6333eee9dcc0b859e973c4065 Mon Sep 17 00:00:00 2001 From: hshristov Date: Thu, 2 Apr 2015 15:43:04 +0300 Subject: [PATCH 1/5] Fix label ellipsis issue on zoomed iPhone 6 Plus. Fix Image.imageSource propertyMetadata to affects layout. --- apps/tests/ui/image/image-tests.ts | 5 +++++ apps/tests/ui/label/label-tests.ts | 18 ++++++++++++++++++ ui/core/view-common.ts | 4 ++-- ui/image/image-common.ts | 2 +- ui/image/image.d.ts | 8 ++++---- 5 files changed, 30 insertions(+), 7 deletions(-) diff --git a/apps/tests/ui/image/image-tests.ts b/apps/tests/ui/image/image-tests.ts index 53851f756..0585136d0 100644 --- a/apps/tests/ui/image/image-tests.ts +++ b/apps/tests/ui/image/image-tests.ts @@ -249,3 +249,8 @@ export var test_SettingStretch_none = function () { helper.buildUIAndRunTest(image, testFunc); } + +export var test_src_property_affects_layout = function () { + var sourceProperty = ImageModule.Image.imageSourceProperty; + TKUnit.assertEqual(sourceProperty.metadata.affectsLayout, true, "sourceProperty should invalidate layout"); +} \ No newline at end of file diff --git a/apps/tests/ui/label/label-tests.ts b/apps/tests/ui/label/label-tests.ts index 729ab7669..a13824d81 100644 --- a/apps/tests/ui/label/label-tests.ts +++ b/apps/tests/ui/label/label-tests.ts @@ -87,6 +87,24 @@ export var test_Set_Text_Native = function () { helper.buildUIAndRunTest(label, test); } +export var test_measuredWidth_is_not_clipped = function () { + var label = new LabelModule.Label(); + label.horizontalAlignment = "left"; + label.text = "i"; + label.fontSize = 9; + + var test = function (views: Array) { + + TKUnit.waitUntilReady(() => { return label.isLayoutValid; }); + + var expectedValue = 3; + var measuredWidth = label.getMeasuredWidth(); + TKUnit.assertEqual(measuredWidth, expectedValue, "measuredWidth should not be rounded down."); + } + + helper.buildUIAndRunTest(label, test); +} + export var test_Set_TextWrap_TNS = function () { // // ### How to turn on text wrapping for a label diff --git a/ui/core/view-common.ts b/ui/core/view-common.ts index 7ed335ac6..4d0d03acd 100644 --- a/ui/core/view-common.ts +++ b/ui/core/view-common.ts @@ -476,7 +476,7 @@ export class View extends proxy.ProxyObject implements definition.View { case utils.layout.AT_MOST: if (specSize < size) { - result = Math.round(specSize) | utils.layout.MEASURED_STATE_TOO_SMALL; + result = Math.round(specSize + 0.499) | utils.layout.MEASURED_STATE_TOO_SMALL; } break; @@ -485,7 +485,7 @@ export class View extends proxy.ProxyObject implements definition.View { break; } - return Math.round(result) | (childMeasuredState & utils.layout.MEASURED_STATE_MASK); + return Math.round(result + 0.499) | (childMeasuredState & utils.layout.MEASURED_STATE_MASK); } public static layoutChild(parent: View, child: View, left: number, top: number, right: number, bottom: number): void { diff --git a/ui/image/image-common.ts b/ui/image/image-common.ts index b719f7a09..998d27f2d 100644 --- a/ui/image/image-common.ts +++ b/ui/image/image-common.ts @@ -65,7 +65,7 @@ export class Image extends view.View implements definition.Image { IMAGE, new proxy.PropertyMetadata( undefined, - dependencyObservable.PropertyMetadataSettings.None + dependencyObservable.PropertyMetadataSettings.AffectsLayout ) ); diff --git a/ui/image/image.d.ts b/ui/image/image.d.ts index 1f394ab45..3a506e352 100644 --- a/ui/image/image.d.ts +++ b/ui/image/image.d.ts @@ -10,8 +10,8 @@ declare module "ui/image" { * Represents a class that provides functionality for loading and streching image(s). */ export class Image extends view.View { - public static urlProperty: dependencyObservable.Property; - public static sourceProperty: dependencyObservable.Property; + public static srcProperty: dependencyObservable.Property; + public static imageSourceProperty: dependencyObservable.Property; public static isLoadingProperty: dependencyObservable.Property; public static stretchProperty: dependencyObservable.Property; @@ -53,12 +53,12 @@ declare module "ui/image" { /** * Gets or sets the image source of the image. */ - source: imageSource.ImageSource; + imageSource: imageSource.ImageSource; /** * Gets or sets the URL of the image. */ - url: string; + src: string; /** * Gets or sets the image stretch mode. Possible values are contained in the [Stretch enumeration](../enums/Stretch/README.md). From 710c4236096772efa50d39f09b5e27ea1d11d768 Mon Sep 17 00:00:00 2001 From: hshristov Date: Tue, 7 Apr 2015 14:28:20 +0300 Subject: [PATCH 2/5] Fix ios transition issue Revert imageSource property metadata to None Improve ListView preparecell check --- ui/frame/frame.ios.ts | 70 ++++++++++++++++++++++++++++++----- ui/image/image-common.ts | 3 +- ui/list-view/list-view.ios.ts | 27 +++++++------- 3 files changed, 77 insertions(+), 23 deletions(-) diff --git a/ui/frame/frame.ios.ts b/ui/frame/frame.ios.ts index c018cc505..5c9be26f1 100644 --- a/ui/frame/frame.ios.ts +++ b/ui/frame/frame.ios.ts @@ -4,6 +4,8 @@ import trace = require("trace"); import imageSource = require("image-source"); import pages = require("ui/page"); import enums = require("ui/enums"); +import utils = require("utils/utils"); +import view = require("ui/core/view"); declare var exports; require("utils/module-merge").merge(frameCommon, exports); @@ -15,7 +17,8 @@ var navDepth = 0; export class Frame extends frameCommon.Frame { private _ios: iOSFrame; private _paramToNavigate: any; - public shouldSkipNativePop: boolean = false; + public _shouldSkipNativePop: boolean = false; + public _navigateToEntry: definition.BackstackEntry; constructor() { super(); @@ -39,7 +42,7 @@ export class Frame extends frameCommon.Frame { this._paramToNavigate = param; } } - + public _navigateCore(backstackEntry: definition.BackstackEntry) { var viewController = backstackEntry.resolvedPage.ios; if (!viewController) { @@ -63,7 +66,7 @@ export class Frame extends frameCommon.Frame { public _goBackCore(entry: definition.NavigationEntry) { navDepth--; trace.write("Frame<" + this._domId + ">.popViewControllerAnimated depth = " + navDepth, trace.categories.Navigation); - if (!this.shouldSkipNativePop) { + if (!this._shouldSkipNativePop) { this._ios.controller.popViewControllerAnimated(this._getIsAnimatedNavigation(entry)); } } @@ -109,6 +112,32 @@ export class Frame extends frameCommon.Frame { } } + public onMeasure(widthMeasureSpec: number, heightMeasureSpec: number): void { + + var width = utils.layout.getMeasureSpecSize(widthMeasureSpec); + var widthMode = utils.layout.getMeasureSpecMode(widthMeasureSpec); + + var height = utils.layout.getMeasureSpecSize(heightMeasureSpec); + var heightMode = utils.layout.getMeasureSpecMode(heightMeasureSpec); + + var result = view.View.measureChild(this, this.currentPage, widthMeasureSpec, utils.layout.makeMeasureSpec(height - this.navigationBarHeight, heightMode)); + if (this._navigateToEntry) { + view.View.measureChild(this, this._navigateToEntry.resolvedPage, widthMeasureSpec, utils.layout.makeMeasureSpec(height - this.navigationBarHeight, heightMode)); + } + + var widthAndState = view.View.resolveSizeAndState(result.measuredWidth, width, widthMode, 0); + var heightAndState = view.View.resolveSizeAndState(result.measuredHeight, height, heightMode, 0); + + this.setMeasuredDimension(widthAndState, heightAndState); + } + + public onLayout(left: number, top: number, right: number, bottom: number): void { + view.View.layoutChild(this, this.currentPage, 0, this.navigationBarHeight, right - left, bottom - top); + if (this._navigateToEntry) { + view.View.layoutChild(this, this._navigateToEntry.resolvedPage, 0, this.navigationBarHeight, right - left, bottom - top); + } + } + public layoutNativeView(left: number, top: number, right: number, bottom: number): void { // We don't call super here because we set frame on our first subview. // This is done because when rotated in iOS7 there is rotation applied on the first subview on the Window which is our frame.nativeView.view. @@ -196,6 +225,29 @@ class UINavigationControllerImpl extends UINavigationController implements UINav this._owner._updateLayout(); } + public navigationControllerWillShowViewControllerAnimated(navigationController: UINavigationController, viewController: UIViewController, animated: boolean): void { + // This method is needed otherwise it will be too late and transition will completed before page is layouted. + var frame = this._owner; + var backStack = frame.backStack; + var currentEntry = backStack.length > 0 ? backStack[backStack.length - 1] : null; + var newEntry: definition.BackstackEntry = viewController[ENTRY]; + + var isBack = currentEntry && newEntry === currentEntry; + if (!isBack) { + var newPage = newEntry.resolvedPage; + + if (!frame._currentEntry) { + frame._currentEntry = newEntry; + } + else { + frame._navigateToEntry = newEntry; + } + + frame._addView(newPage); + frame.populateMenuItems(newPage); + } + } + public navigationControllerDidShowViewControllerAnimated(navigationController: UINavigationController, viewController: UIViewController, animated: boolean): void { var frame: Frame = this._owner; @@ -203,26 +255,26 @@ class UINavigationControllerImpl extends UINavigationController implements UINav var currentEntry = backStack.length > 0 ? backStack[backStack.length - 1] : null; var newEntry: definition.BackstackEntry = viewController[ENTRY]; - if (newEntry === currentEntry && currentEntry) { + var isBack: boolean = currentEntry && newEntry === currentEntry; + if (isBack) { try { - frame.shouldSkipNativePop = true; + frame._shouldSkipNativePop = true; frame.goBack(); } finally { - frame.shouldSkipNativePop = false; + frame._shouldSkipNativePop = false; } } var page = frame.currentPage; - if (page) { + if (page && isBack) { frame._removeView(page); } var newPage = newEntry.resolvedPage; + frame._navigateToEntry = null; frame._currentEntry = newEntry; - frame._addView(newPage); - frame.populateMenuItems(newPage); frame.updateNavigationBar(); // notify the page diff --git a/ui/image/image-common.ts b/ui/image/image-common.ts index 998d27f2d..a9b491128 100644 --- a/ui/image/image-common.ts +++ b/ui/image/image-common.ts @@ -65,7 +65,8 @@ export class Image extends view.View implements definition.Image { IMAGE, new proxy.PropertyMetadata( undefined, - dependencyObservable.PropertyMetadataSettings.AffectsLayout + // None on purpose. for iOS we trigger it manually if needed. Android layout handles it. + dependencyObservable.PropertyMetadataSettings.None ) ); diff --git a/ui/list-view/list-view.ios.ts b/ui/list-view/list-view.ios.ts index 4bff16324..4d022cc28 100644 --- a/ui/list-view/list-view.ios.ts +++ b/ui/list-view/list-view.ios.ts @@ -186,7 +186,7 @@ export class ListView extends common.ListView { public refresh() { this._ios.reloadData(); - this.requestLayout(); + this.requestLayout(); } public getHeight(index: number): number { @@ -227,21 +227,22 @@ export class ListView extends common.ListView { public _prepareCell(tableCell: UITableViewCell, indexPath: NSIndexPath): number { var cell: any = tableCell; - if (!cell.view) { - cell.view = this._getItemTemplateContent(indexPath.row); - } - - var args = notifyForItemAtIndex(this, cell, ITEMLOADING, indexPath); - var view = cell.view = args.view || this._getDefaultItemContent(indexPath.row); - - if (view && !view.parent && view.ios) { - cell.contentView.addSubview(view.ios); - this._addView(view); - } - var cellHeight: number; + try { this._preparingCell = true; + if (!cell.view) { + cell.view = this._getItemTemplateContent(indexPath.row); + } + + var args = notifyForItemAtIndex(this, cell, ITEMLOADING, indexPath); + var view = cell.view = args.view || this._getDefaultItemContent(indexPath.row); + + if (view && !view.parent && view.ios) { + cell.contentView.addSubview(view.ios); + this._addView(view); + } + this._prepareItem(view, indexPath.row); cellHeight = this._layoutCell(view, indexPath); } From 0386d07604cbe5f97476076e54735f79bb5bc700 Mon Sep 17 00:00:00 2001 From: hshristov Date: Tue, 7 Apr 2015 14:42:01 +0300 Subject: [PATCH 3/5] Delete one unittest - not needed --- apps/tests/ui/image/image-tests.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/tests/ui/image/image-tests.ts b/apps/tests/ui/image/image-tests.ts index 0585136d0..8d3ceb7a2 100644 --- a/apps/tests/ui/image/image-tests.ts +++ b/apps/tests/ui/image/image-tests.ts @@ -248,9 +248,4 @@ export var test_SettingStretch_none = function () { } helper.buildUIAndRunTest(image, testFunc); -} - -export var test_src_property_affects_layout = function () { - var sourceProperty = ImageModule.Image.imageSourceProperty; - TKUnit.assertEqual(sourceProperty.metadata.affectsLayout, true, "sourceProperty should invalidate layout"); } \ No newline at end of file From 44e22495dad6447b668293d1090888934de92214 Mon Sep 17 00:00:00 2001 From: hshristov Date: Tue, 7 Apr 2015 17:46:58 +0300 Subject: [PATCH 4/5] fix android onSelectedItem handler. fix ios transition (previous fix was not enough) --- ui/frame/frame.android.ts | 2 +- ui/frame/frame.ios.ts | 32 +++++++++++++------------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/ui/frame/frame.android.ts b/ui/frame/frame.android.ts index 3643150d6..57a74fde6 100644 --- a/ui/frame/frame.android.ts +++ b/ui/frame/frame.android.ts @@ -182,7 +182,7 @@ class PageFragmentBody extends android.app.Fragment { return true; } - super.onOptionsItemSelected(item); + return super.onOptionsItemSelected(item); } } diff --git a/ui/frame/frame.ios.ts b/ui/frame/frame.ios.ts index 5c9be26f1..420b557a6 100644 --- a/ui/frame/frame.ios.ts +++ b/ui/frame/frame.ios.ts @@ -226,26 +226,18 @@ class UINavigationControllerImpl extends UINavigationController implements UINav } public navigationControllerWillShowViewControllerAnimated(navigationController: UINavigationController, viewController: UIViewController, animated: boolean): void { - // This method is needed otherwise it will be too late and transition will completed before page is layouted. + // In this method we need to layout the new page otherwise page will be shown empty and update after that which is bad UX. var frame = this._owner; - var backStack = frame.backStack; - var currentEntry = backStack.length > 0 ? backStack[backStack.length - 1] : null; var newEntry: definition.BackstackEntry = viewController[ENTRY]; - - var isBack = currentEntry && newEntry === currentEntry; - if (!isBack) { - var newPage = newEntry.resolvedPage; - - if (!frame._currentEntry) { - frame._currentEntry = newEntry; - } - else { - frame._navigateToEntry = newEntry; - } - + var newPage = newEntry.resolvedPage; + if (!newPage.parent) { + frame._navigateToEntry = newEntry; frame._addView(newPage); frame.populateMenuItems(newPage); } + else if (newPage.parent !== frame) { + throw new Error("Page is already shown on another frame."); + } } public navigationControllerDidShowViewControllerAnimated(navigationController: UINavigationController, viewController: UIViewController, animated: boolean): void { @@ -255,6 +247,8 @@ class UINavigationControllerImpl extends UINavigationController implements UINav var currentEntry = backStack.length > 0 ? backStack[backStack.length - 1] : null; var newEntry: definition.BackstackEntry = viewController[ENTRY]; + // This code check if navigation happened through UI (e.g. back button or swipe gesture). + // When calling goBack on frame isBack will be false. var isBack: boolean = currentEntry && newEntry === currentEntry; if (isBack) { try { @@ -267,16 +261,16 @@ class UINavigationControllerImpl extends UINavigationController implements UINav } var page = frame.currentPage; - if (page && isBack) { + if (page && !navigationController.viewControllers.containsObject(page.ios)) { frame._removeView(page); } - var newPage = newEntry.resolvedPage; - frame._navigateToEntry = null; frame._currentEntry = newEntry; frame.updateNavigationBar(); - + + var newPage = newEntry.resolvedPage; + // notify the page newPage.onNavigatedTo(newEntry.entry.context); frame._processNavigationQueue(newPage); From 5cce9e206bf7e27d68c358352933415006563c91 Mon Sep 17 00:00:00 2001 From: hshristov Date: Wed, 8 Apr 2015 10:54:16 +0300 Subject: [PATCH 5/5] Update unittest helper method navigation checks --- apps/tests/ui/helper.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/tests/ui/helper.ts b/apps/tests/ui/helper.ts index 6d9389179..a75ec1003 100644 --- a/apps/tests/ui/helper.ts +++ b/apps/tests/ui/helper.ts @@ -194,7 +194,6 @@ export function navigateToModule(moduleName: string) { frame.topmost().navigate({ moduleName: moduleName, animated: false }); TKUnit.waitUntilReady(() => { return frame.topmost().currentPage !== currentPage; }); TKUnit.assert(frame.topmost().currentPage.isLoaded, "Current page should be loaded!"); - TKUnit.assert(!currentPage.isLoaded, "Previous page should be unloaded!"); } export function goBack(): void {