From 0b6ca23e25ae1a59ff068d188df70d19520461be Mon Sep 17 00:00:00 2001 From: Nedyalko Nikolov Date: Wed, 30 Sep 2015 11:18:14 +0300 Subject: [PATCH 1/3] BindingContext will be set before adding items to visual tree (ListView, Repeater). --- apps/tests/testRunner.ts | 2 +- apps/tests/ui/list-view/list-view-tests.ts | 7 +++++ apps/tests/ui/observable-tests.ts | 27 +++++++++++++++++++ .../repeaterItems-bindingToGestures.xml | 2 +- apps/tests/ui/text-field/text-field-tests.ts | 4 +++ apps/tests/ui/text-view/text-view-tests.ts | 4 +++ .../xml-declaration/xml-declaration-tests.ts | 2 +- data/observable/observable.ts | 2 +- ui/core/bindable.ts | 16 ++++++++++- ui/list-view/list-view.android.ts | 3 +-- ui/list-view/list-view.ios.ts | 2 +- ui/repeater/repeater.ts | 2 +- 12 files changed, 64 insertions(+), 9 deletions(-) diff --git a/apps/tests/testRunner.ts b/apps/tests/testRunner.ts index 83cb56918..ef6e75256 100644 --- a/apps/tests/testRunner.ts +++ b/apps/tests/testRunner.ts @@ -7,7 +7,7 @@ import uiTestModule = require("./ui-test"); frameModule.Frame.defaultAnimatedNavigation = false; -function isRunningOnEmulator(): boolean { +export function isRunningOnEmulator(): boolean { // This checks are not good enough to be added to modules but keeps unittests green. if (platform.device.os === platform.platformNames.android) { diff --git a/apps/tests/ui/list-view/list-view-tests.ts b/apps/tests/ui/list-view/list-view-tests.ts index de6a53567..3b6db9736 100644 --- a/apps/tests/ui/list-view/list-view-tests.ts +++ b/apps/tests/ui/list-view/list-view-tests.ts @@ -6,6 +6,7 @@ import observable = require("data/observable"); import types = require("utils/types"); import platform = require("platform"); import utils = require("utils/utils"); +import testRunner = require("../../testRunner"); //  // # ListView @@ -633,6 +634,9 @@ export function test_ConverterIsCalledJustOnce_onAddingItemsToListView() { } export function test_no_memory_leak_when_items_is_regular_array() { + if (testRunner.isRunningOnEmulator()) { + return; + } var createFunc = function (): listViewModule.ListView { var listView = new listViewModule.ListView(); listView.items = FEW_ITEMS; @@ -645,6 +649,9 @@ export function test_no_memory_leak_when_items_is_regular_array() { } export function test_no_memory_leak_when_items_is_observable_array() { + if (testRunner.isRunningOnEmulator()) { + return; + } // Keep the reference to the observable array to test the weakEventListener var colors = new observableArray.ObservableArray(["red", "green", "blue"]); diff --git a/apps/tests/ui/observable-tests.ts b/apps/tests/ui/observable-tests.ts index 23f76f057..b8bb5598e 100644 --- a/apps/tests/ui/observable-tests.ts +++ b/apps/tests/ui/observable-tests.ts @@ -378,3 +378,30 @@ export var test_Observable_WhenCreatedWithJSON_PropertyChangedWithBracketsNotati TKUnit.assert(receivedCount === 1, "PropertyChanged event not raised properly."); } + +export function test_AddingTwoEventHandlersAndRemovingWithinHandlerShouldRaiseAllEvents() { + var observableInstance = new observable.Observable(); + var firstHandlerCalled = false; + var secondHandlerCalled= false; + + var firstHandler = function (args) { + observableInstance.off(observable.Observable.propertyChangeEvent, firstHandler, firstObserver); + firstHandlerCalled = true; + } + + var secondHandler = function (args) { + observableInstance.off(observable.Observable.propertyChangeEvent, secondHandler, secondObserver); + secondHandlerCalled = true; + } + + var firstObserver = new observable.Observable(); + var secondObserver = new observable.Observable(); + + observableInstance.on(observable.Observable.propertyChangeEvent, firstHandler, firstObserver); + observableInstance.on(observable.Observable.propertyChangeEvent, secondHandler, secondObserver); + + observableInstance.set("someProperty", "some value"); + + TKUnit.assertEqual(firstHandlerCalled, true); + TKUnit.assertEqual(secondHandlerCalled, true); +} diff --git a/apps/tests/ui/repeater/repeaterItems-bindingToGestures.xml b/apps/tests/ui/repeater/repeaterItems-bindingToGestures.xml index addde5ae1..667c672a1 100644 --- a/apps/tests/ui/repeater/repeaterItems-bindingToGestures.xml +++ b/apps/tests/ui/repeater/repeaterItems-bindingToGestures.xml @@ -2,7 +2,7 @@ - diff --git a/apps/tests/ui/text-field/text-field-tests.ts b/apps/tests/ui/text-field/text-field-tests.ts index a6d78c6db..637535aef 100644 --- a/apps/tests/ui/text-field/text-field-tests.ts +++ b/apps/tests/ui/text-field/text-field-tests.ts @@ -1,4 +1,5 @@ import TKUnit = require("../../TKUnit"); +import testRunner = require("../../testRunner"); import helper = require("../helper"); import viewModule = require("ui/core/view"); import pagesModule = require("ui/page"); @@ -427,6 +428,9 @@ export var testNativeTextAlignmentFromLocal = function () { } export var testMemoryLeak = function () { + if (testRunner.isRunningOnEmulator()) { + return; + } helper.buildUIWithWeakRefAndInteract(_createTextFieldFunc, function (textField) { textFieldTestsNative.typeTextNatively(textField, "Hello, world!"); }); diff --git a/apps/tests/ui/text-view/text-view-tests.ts b/apps/tests/ui/text-view/text-view-tests.ts index 1666e99c7..631088a61 100644 --- a/apps/tests/ui/text-view/text-view-tests.ts +++ b/apps/tests/ui/text-view/text-view-tests.ts @@ -1,4 +1,5 @@ import TKUnit = require("../../TKUnit"); +import testRunner = require("../../testRunner"); import helper = require("../helper"); import viewModule = require("ui/core/view"); import pagesModule = require("ui/page"); @@ -468,6 +469,9 @@ export var testNativeTextAlignmentFromLocal = function () { } export var testMemoryLeak = function () { + if (testRunner.isRunningOnEmulator()) { + return; + } helper.buildUIWithWeakRefAndInteract(_createTextViewFunc, function (textView) { textViewTestsNative.typeTextNatively(textView, "Hello, world!"); }); diff --git a/apps/tests/xml-declaration/xml-declaration-tests.ts b/apps/tests/xml-declaration/xml-declaration-tests.ts index 4083ea09d..5ab1be618 100644 --- a/apps/tests/xml-declaration/xml-declaration-tests.ts +++ b/apps/tests/xml-declaration/xml-declaration-tests.ts @@ -603,7 +603,7 @@ export function test_parse_ShouldParseNestedListViewInListViewTemplate() { } export function test_parse_ShouldEvaluateEventBindingExpressionInListViewTemplate() { - var p = builder.parse(''); + var p = builder.parse(''); function testAction(views: Array) { var ctrl: segmentedBar.SegmentedBar; diff --git a/data/observable/observable.ts b/data/observable/observable.ts index 3fd51a3c3..565fc58e7 100644 --- a/data/observable/observable.ts +++ b/data/observable/observable.ts @@ -136,7 +136,7 @@ export class Observable implements definition.Observable { var i; var entry: ListenerEntry; var observersLength = observers.length; - for (i = 0; i < observersLength; i++) { + for (i = observersLength - 1; i >= 0 ; i--) { entry = observers[i]; if (entry.thisArg) { entry.callback.apply(entry.thisArg, [data]); diff --git a/ui/core/bindable.ts b/ui/core/bindable.ts index 9cc0f646a..0c4bc3c30 100644 --- a/ui/core/bindable.ts +++ b/ui/core/bindable.ts @@ -140,6 +140,15 @@ export class Binding { source: WeakRef; target: WeakRef; + public loadedHandlerVisualTreeBinding(args) { + var targetInstance = args.object; + targetInstance.off(viewModule.View.loadedEvent, this.loadedHandlerVisualTreeBinding, this); + this.unbind(); + if (!types.isNullOrUndefined(targetInstance.bindingContext)) { + this.bind(targetInstance.bindingContext); + } + }; + private propertyChangeListeners = {}; private sourceOptions: { instance: WeakRef; property: any }; @@ -213,6 +222,11 @@ export class Binding { if (parentView) { currentObject = parentView.bindingContext; } + else { + var targetInstance = this.target.get(); + targetInstance.off(viewModule.View.loadedEvent, this.loadedHandlerVisualTreeBinding, this); + targetInstance.on(viewModule.View.loadedEvent, this.loadedHandlerVisualTreeBinding, this); + } currentObjectChanged = true; } result.push({ instance: currentObject, property: objProp }); @@ -507,7 +521,7 @@ export class Binding { indexAsInt--; } } - else { + else if (types.isString(index)) { while (result && result.typeName !== index) { result = result.parent; } diff --git a/ui/list-view/list-view.android.ts b/ui/list-view/list-view.android.ts index fd41b3a1b..131754dc2 100644 --- a/ui/list-view/list-view.android.ts +++ b/ui/list-view/list-view.android.ts @@ -205,6 +205,7 @@ class ListViewAdapter extends android.widget.BaseAdapter { } if (args.view) { + this._listView._prepareItem(args.view, index); if (!args.view.parent) { if (args.view instanceof layoutBaseModule.LayoutBase) { this._listView._addView(args.view); @@ -221,8 +222,6 @@ class ListViewAdapter extends android.widget.BaseAdapter { this._listView._realizedItems[convertView.hashCode()] = args.view; // cache the realized index (used to raise the ItemLoading event upon scroll stop) args.view[REALIZED_INDEX] = index; - - this._listView._prepareItem(args.view, index); } return convertView; diff --git a/ui/list-view/list-view.ios.ts b/ui/list-view/list-view.ios.ts index 77b6c2fe8..9cfc07328 100644 --- a/ui/list-view/list-view.ios.ts +++ b/ui/list-view/list-view.ios.ts @@ -239,12 +239,12 @@ export class ListView extends common.ListView { var args = notifyForItemAtIndex(this, cell, ITEMLOADING, indexPath); var view = cell.view = args.view || this._getDefaultItemContent(indexPath.row); + this._prepareItem(view, 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); } finally { diff --git a/ui/repeater/repeater.ts b/ui/repeater/repeater.ts index a40540421..6600d7868 100644 --- a/ui/repeater/repeater.ts +++ b/ui/repeater/repeater.ts @@ -149,8 +149,8 @@ export class Repeater extends viewModule.CustomLayoutView implements definition. for (i = 0; i < this.items.length; i++) { var viewToAdd = !types.isNullOrUndefined(this.itemTemplate) ? builder.parse(this.itemTemplate, this) : this._getDefaultItemContent(i); if (!types.isNullOrUndefined(viewToAdd)) { - this.itemsLayout.addChild(viewToAdd); viewToAdd.bindingContext = this._getDataItem(i); + this.itemsLayout.addChild(viewToAdd); } } } From 113a1b6a69c3155e6d711a9c57dad2d6069e3cfd Mon Sep 17 00:00:00 2001 From: Vladimir Enchev Date: Tue, 29 Sep 2015 15:31:36 +0300 Subject: [PATCH 2/3] placeholder creatingView event fixed + tests --- CrossPlatformModules.csproj | 7 +- apps/tests/testRunner.ts | 1 + .../tests/ui/placeholder/placeholder-tests.ts | 78 +++++++++++++++++++ ui/core/view.ios.ts | 26 ++++--- ui/placeholder/placeholder.android.ts | 2 +- ui/placeholder/placeholder.ios.ts | 2 +- 6 files changed, 100 insertions(+), 16 deletions(-) create mode 100644 apps/tests/ui/placeholder/placeholder-tests.ts diff --git a/CrossPlatformModules.csproj b/CrossPlatformModules.csproj index 786adf343..8dcc19ea0 100644 --- a/CrossPlatformModules.csproj +++ b/CrossPlatformModules.csproj @@ -80,6 +80,7 @@ data-binding.xml + @@ -183,8 +184,8 @@ - nordic.xml - + nordic.xml + @@ -1984,7 +1985,7 @@ False - + \ No newline at end of file diff --git a/apps/tests/testRunner.ts b/apps/tests/testRunner.ts index 83cb56918..bb02546d6 100644 --- a/apps/tests/testRunner.ts +++ b/apps/tests/testRunner.ts @@ -61,6 +61,7 @@ allTests["IMAGE"] = require("./ui/image/image-tests"); allTests["SLIDER"] = require("./ui/slider/slider-tests"); allTests["SWITCH"] = require("./ui/switch/switch-tests"); allTests["PROGRESS"] = require("./ui/progress/progress-tests"); +allTests["PLACEHOLDER"] = require("./ui/placeholder/placeholder-tests"); allTests["PAGE"] = require("./ui/page/page-tests"); allTests["LISTVIEW"] = require("./ui/list-view/list-view-tests"); allTests["ACTIVITY-INDICATOR"] = require("./ui/activity-indicator/activity-indicator-tests"); diff --git a/apps/tests/ui/placeholder/placeholder-tests.ts b/apps/tests/ui/placeholder/placeholder-tests.ts new file mode 100644 index 000000000..97395cf15 --- /dev/null +++ b/apps/tests/ui/placeholder/placeholder-tests.ts @@ -0,0 +1,78 @@ +import TKUnit = require("../../TKUnit"); +import platform = require("platform"); +import utils = require("utils/utils"); +import helper = require("../helper"); +import viewModule = require("ui/core/view"); + +//  +// # Placeholder +// Using the placeholder requires the Placeholder module. +// ``` JavaScript +import placeholderModule = require("ui/placeholder"); +// ``` + +// Creating native view for the Placeholder using creatingView event. +//```XML +// +// {%raw%}{%endraw%} +// +//``` +//```JS +//var platform = require("platform"); +//var utils = require("utils/utils"); +// +//function creatingView(args) { +// var nativeView; +// if (platform.device.os === platform.platformNames.ios) { +// nativeView = new UITextView(); +// nativeView.text = "Native"; +// } else if (platform.device.os === platform.platformNames.android) { +// nativeView = new android.widget.TextView(utils.ad.getApplicationContext()); +// nativeView.setText("Native"); +// } +// +// args.view = nativeView; +//} +// +//exports.creatingView = creatingView; +//``` +//  + +export function test_placeholder_creatingView() { + var nativeView; + + var p = new placeholderModule.Placeholder(); + p.id = "test"; + p.on(placeholderModule.Placeholder.creatingViewEvent, (args: placeholderModule.CreateViewEventData) => { + if (platform.device.os === platform.platformNames.ios) { + nativeView = new UITextView(); + nativeView.text = "Native"; + } else if (platform.device.os === platform.platformNames.android) { + nativeView = new android.widget.TextView(utils.ad.getApplicationContext()); + nativeView.setText("Native"); + } + + args.view = nativeView; + }); + + if (platform.device.os === platform.platformNames.ios) { + TKUnit.assert(p.ios instanceof UITextView, "ios property should be UITextView. Current value: " + p.ios); + } else if (platform.device.os === platform.platformNames.android) { + p._emit("creatingView"); + TKUnit.assert(nativeView instanceof android.widget.TextView, "Native view should be android.widget.TextView. Current value: " + nativeView); + } +} + +export function test_placeholder_will_not_crash_wihout_creatingView() { + var p = new placeholderModule.Placeholder(); + + function testAction(views: Array) { + if (platform.device.os === platform.platformNames.ios) { + TKUnit.assert(p.ios === undefined, "ios property should be undefined. Current value: " + p.ios); + } else if (platform.device.os === platform.platformNames.android) { + TKUnit.assert(p.android === undefined, "android view should be undefined. Current value: " + p.android); + } + }; + + helper.buildUIAndRunTest(p, testAction); +} \ No newline at end of file diff --git a/ui/core/view.ios.ts b/ui/core/view.ios.ts index 922b9de36..02cc7751d 100644 --- a/ui/core/view.ios.ts +++ b/ui/core/view.ios.ts @@ -196,8 +196,10 @@ export class View extends viewCommon.View { public onMeasure(widthMeasureSpec: number, heightMeasureSpec: number): void { var view = this._nativeView; - if (view) { + let nativeWidth = 0; + let nativeHeight = 0; + if (view) { var width = utils.layout.getMeasureSpecSize(widthMeasureSpec); var widthMode = utils.layout.getMeasureSpecMode(widthMeasureSpec); @@ -213,15 +215,17 @@ export class View extends viewCommon.View { } var nativeSize = view.sizeThatFits(CGSizeMake(width, height)); - - var measureWidth = Math.max(nativeSize.width, this.minWidth); - var measureHeight = Math.max(nativeSize.height, this.minHeight); - - var widthAndState = View.resolveSizeAndState(measureWidth, width, widthMode, 0); - var heightAndState = View.resolveSizeAndState(measureHeight, height, heightMode, 0); - - this.setMeasuredDimension(widthAndState, heightAndState); + nativeWidth = nativeSize.width; + nativeHeight = nativeSize.height; } + + var measureWidth = Math.max(nativeWidth, this.minWidth); + var measureHeight = Math.max(nativeHeight, this.minHeight); + + var widthAndState = View.resolveSizeAndState(measureWidth, width, widthMode, 0); + var heightAndState = View.resolveSizeAndState(measureHeight, height, heightMode, 0); + + this.setMeasuredDimension(widthAndState, heightAndState); } public onLayout(left: number, top: number, right: number, bottom: number): void { @@ -305,12 +309,12 @@ export class CustomLayoutView extends View { if (this._nativeView && child._nativeView) { if (types.isNullOrUndefined(atIndex) || atIndex >= this._nativeView.subviews.count) { - this._nativeView.addSubview(child._nativeView); + this._nativeView.addSubview(child._nativeView); } else { this._nativeView.insertSubviewAtIndex(child._nativeView, atIndex); } - + return true; } diff --git a/ui/placeholder/placeholder.android.ts b/ui/placeholder/placeholder.android.ts index d3de56051..ebae384f5 100644 --- a/ui/placeholder/placeholder.android.ts +++ b/ui/placeholder/placeholder.android.ts @@ -9,7 +9,7 @@ export class Placeholder extends common.Placeholder { public _createUI() { var args = { eventName: common.Placeholder.creatingViewEvent, object: this, view: undefined, context: this._context }; this.notify(args); - this._android = args.view || new android.view.View(this._context); + this._android = args.view; } get android(): android.view.View { diff --git a/ui/placeholder/placeholder.ios.ts b/ui/placeholder/placeholder.ios.ts index b4ca8eb2a..c05f20456 100644 --- a/ui/placeholder/placeholder.ios.ts +++ b/ui/placeholder/placeholder.ios.ts @@ -10,7 +10,7 @@ export class Placeholder extends common.Placeholder { if (!this._ios) { var args = { eventName: common.Placeholder.creatingViewEvent, object: this, view: undefined, context: undefined }; super.notify(args); - this._ios = args.view || new UIView(); + this._ios = args.view; } return this._ios; } From 7517afd80bf728bba59cf226a8cfbad45b29e4e0 Mon Sep 17 00:00:00 2001 From: Vladimir Enchev Date: Wed, 30 Sep 2015 14:42:25 +0300 Subject: [PATCH 3/3] Message will be set only if you do not have actions since message and actions are mutually exclusive. --- ui/dialogs/dialogs.android.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/dialogs/dialogs.android.ts b/ui/dialogs/dialogs.android.ts index 1431dac06..625e1639c 100644 --- a/ui/dialogs/dialogs.android.ts +++ b/ui/dialogs/dialogs.android.ts @@ -242,7 +242,9 @@ export function action(arg: any): Promise { if (title) { alert.setTitle(title); - alert.setMessage(message); + if (!options.actions) { + alert.setMessage(message); + } } else { alert.setTitle(message);