From 2e4b2eacf2aa4bdf4e749f445a1be4c6b71791f5 Mon Sep 17 00:00:00 2001 From: Nedyalko Nikolov Date: Thu, 30 Apr 2015 13:48:52 +0300 Subject: [PATCH 1/3] Fixed issue when bindingContext is bound more than once. --- CrossPlatformModules.csproj | 4 +++ apps/tests/ui/bindable-tests.ts | 14 ++++++++ apps/tests/ui/bindingContext_testPage.ts | 34 ++++++++++++++++++ apps/tests/ui/bindingContext_testPage.xml | 15 ++++++++ apps/tests/ui/helper.ts | 10 ++++++ file-system/file-system.d.ts | 1 + ui/core/bindable.ts | 36 ++++++++++--------- ui/core/dependency-observable.ts | 23 +++++++----- ui/core/view-common.ts | 44 ++++++++++++----------- ui/core/view.d.ts | 1 + 10 files changed, 136 insertions(+), 46 deletions(-) create mode 100644 apps/tests/ui/bindingContext_testPage.ts create mode 100644 apps/tests/ui/bindingContext_testPage.xml diff --git a/CrossPlatformModules.csproj b/CrossPlatformModules.csproj index f227ba18d..89b37e2b2 100644 --- a/CrossPlatformModules.csproj +++ b/CrossPlatformModules.csproj @@ -137,6 +137,9 @@ + + bindingContext_testPage.xml + time-picker-tests-native.d.ts @@ -593,6 +596,7 @@ + diff --git a/apps/tests/ui/bindable-tests.ts b/apps/tests/ui/bindable-tests.ts index 9fe2f0407..09276a554 100644 --- a/apps/tests/ui/bindable-tests.ts +++ b/apps/tests/ui/bindable-tests.ts @@ -10,6 +10,7 @@ import utils = require("utils/utils"); import pageModule = require("ui/page"); import stackLayoutModule = require("ui/layouts/stack-layout"); import bindingBuilder = require("ui/builder/binding-builder"); +import labelModule = require("ui/label"); // // For information and examples how to use bindings please refer to special [**Data binding**](../../../../bindings.md) topic. @@ -459,4 +460,17 @@ export var test_getBindableOptionsFromStringTwoParamsNamedFormat = function () { TKUnit.assert(bindOptions.targetProperty === "targetBindProperty", "Expected: targetBindProperty, Actual: " + bindOptions.targetProperty); TKUnit.assert(bindOptions.expression === "bindProperty * 2", "Expected: bindProperty * 2, Actual:" + bindOptions.expression); TKUnit.assert(bindOptions.twoWay === true, "Expected: true, Actual: " + bindOptions.twoWay); +} + +export var test_TwoElementsBindingToSameBindingContext = function () { + var testFunc = function (page: pageModule.Page) { + var upperStackLabel = (page.getViewById("upperStackLabel")); + var label1 = (page.getViewById("label1")); + var label2 = (page.getViewById("label2")); + + TKUnit.assertEqual(upperStackLabel.text, label1.text); + TKUnit.assertEqual(upperStackLabel.text, label2.text); + } + + helper.navigateToModuleAndRunTest("./tests/ui/bindingContext_testPage", testFunc); } \ No newline at end of file diff --git a/apps/tests/ui/bindingContext_testPage.ts b/apps/tests/ui/bindingContext_testPage.ts new file mode 100644 index 000000000..eabc1483b --- /dev/null +++ b/apps/tests/ui/bindingContext_testPage.ts @@ -0,0 +1,34 @@ +import observableModule = require("data/observable"); +import pageModule = require("ui/page"); + +class MainViewModel extends observableModule.Observable { + private _item: any; + + constructor() { + super(); + + this.item = { Title: "Alabala" }; + } + + get item(): any { + return this._item; + } + + set item(value: any) { + if (this._item !== value) { + this._item = value; + this.notifyPropertyChanged("item", value); + } + } + + notifyPropertyChanged(propertyName: string, value: any) { + this.notify({ object: this, eventName: observableModule.Observable.propertyChangeEvent, propertyName: propertyName, value: value }); + } +} + +var viewModel = new MainViewModel(); + +export function pageLoaded(args: observableModule.EventData) { + var page = args.object; + page.bindingContext = viewModel; +} \ No newline at end of file diff --git a/apps/tests/ui/bindingContext_testPage.xml b/apps/tests/ui/bindingContext_testPage.xml new file mode 100644 index 000000000..de18da7a5 --- /dev/null +++ b/apps/tests/ui/bindingContext_testPage.xml @@ -0,0 +1,15 @@ + + + + diff --git a/apps/tests/ui/helper.ts b/apps/tests/ui/helper.ts index 78b932ccd..33288f30f 100644 --- a/apps/tests/ui/helper.ts +++ b/apps/tests/ui/helper.ts @@ -143,6 +143,16 @@ export function buildUIAndRunTest(controlToTest, testFunction, pageCss?) { } } +export function navigateToModuleAndRunTest(moduleName, testFunction) { + navigateToModule(moduleName); + try { + testFunction(frame.topmost().currentPage); + } + finally { + goBack(); + } +} + export function buildUIWithWeakRefAndInteract(createFunc: () => T, interactWithViewFunc?: (view: T) => void) { var newPage: page.Page; var testFinished = false; diff --git a/file-system/file-system.d.ts b/file-system/file-system.d.ts index 1e2880116..8f5c6768f 100644 --- a/file-system/file-system.d.ts +++ b/file-system/file-system.d.ts @@ -154,6 +154,7 @@ declare module "file-system" { /** * Gets the root folder for the current application. This Folder is private for the application and not accessible from Users/External apps. + * iOS - this folder is read-only and contains the app and all its resources. */ export function currentApp(): Folder; } diff --git a/ui/core/bindable.ts b/ui/core/bindable.ts index 894608083..f5ed4535e 100644 --- a/ui/core/bindable.ts +++ b/ui/core/bindable.ts @@ -7,13 +7,19 @@ import types = require("utils/types"); import trace = require("trace"); import polymerExpressions = require("js-libs/polymer-expressions"); import bindingBuilder = require("../builder/binding-builder"); +import viewModule = require("ui/core/view"); var bindingContextProperty = new dependencyObservable.Property( "bindingContext", "Bindable", - new dependencyObservable.PropertyMetadata(undefined, dependencyObservable.PropertyMetadataSettings.Inheritable) // TODO: Metadata options? + new dependencyObservable.PropertyMetadata(undefined, dependencyObservable.PropertyMetadataSettings.Inheritable, onBindingContextChanged) ); +function onBindingContextChanged(data: dependencyObservable.PropertyChangeData) { + var bindable = data.object; + bindable._onBindingContextChanged(data.oldValue, data.newValue); +} + var contextKey = "context"; var resourcesKey = "resources"; @@ -73,24 +79,21 @@ export class Bindable extends dependencyObservable.DependencyObservable implemen public _onPropertyChanged(property: dependencyObservable.Property, oldValue: any, newValue: any) { trace.write("Bindable._onPropertyChanged(" + this + ") " + property.name, trace.categories.Binding); super._onPropertyChanged(property, oldValue, newValue); - - if (property === Bindable.bindingContextProperty) { - this._onBindingContextChanged(oldValue, newValue); - } - - var binding = this._bindings[property.name]; - if (binding) { - // we should remove (unbind and delete) binding if binding is oneWay and update is not triggered - // by binding itself. - var shouldRemoveBinding = !binding.updating && !binding.options.twoWay; - if (shouldRemoveBinding) { - trace.write("_onPropertyChanged(" + this + ") removing binding for property: " + property.name, trace.categories.Binding); - this.unbind(property.name); + if (this instanceof viewModule.View) { + if (((this))._isInheritedChange() === true) { + return } - else { + } + var binding = this._bindings[property.name]; + if (binding && !binding.updating) { + if (binding.options.twoWay) { trace.write("_updateTwoWayBinding(" + this + "): " + property.name, trace.categories.Binding); this._updateTwoWayBinding(property.name, newValue); } + else { + trace.write("_onPropertyChanged(" + this + ") removing binding for property: " + property.name, trace.categories.Binding); + this.unbind(property.name); + } } } @@ -99,8 +102,7 @@ export class Bindable extends dependencyObservable.DependencyObservable implemen for (var p in this._bindings) { binding = this._bindings[p]; - if (binding.options.targetProperty === Bindable.bindingContextProperty.name && binding.updating) { - // Updating binding context trough binding should not rebind the binding context. + if (binding.updating) { continue; } diff --git a/ui/core/dependency-observable.ts b/ui/core/dependency-observable.ts index dbd301aa1..71eac3778 100644 --- a/ui/core/dependency-observable.ts +++ b/ui/core/dependency-observable.ts @@ -23,9 +23,19 @@ function validateRegisterParameters(name: string, ownerType: string) { } } -function getPropertyByNameAndType(name: string, ownerType: string): Property { - var key = generatePropertyKey(name, ownerType); - return propertyFromKey[key]; +function getPropertyByNameAndType(name: string, owner: any): Property { + var baseClasses = types.getBaseClasses(owner); + var i; + var result; + var key; + for (i = 0; i < baseClasses.length; i++) { + key = generatePropertyKey(name, baseClasses[i]); + result = propertyFromKey[key]; + if (result) { + break; + } + } + return result; } export module PropertyMetadataSettings { @@ -254,13 +264,10 @@ export class PropertyEntry implements definition.PropertyEntry { } export class DependencyObservable extends observable.Observable { - // TODO: measure the performance of the dictionary vs. JS Object with numeric keys - // private _values = new containers.Dictionary(new containers.StringComparer()); private _propertyEntries = {}; public set(name: string, value: any) { - // TODO: Properties must be registered with the correct owner type for this routine to work - var property = getPropertyByNameAndType(name, this.typeName); + var property = getPropertyByNameAndType(name, this); if (property) { this._setValue(property, value, ValueSource.Local); } else { @@ -269,7 +276,7 @@ export class DependencyObservable extends observable.Observable { } public get(name: string): any { - var property = getPropertyByNameAndType(name, this.typeName); + var property = getPropertyByNameAndType(name, this); if (property) { return this._getValue(property); } else { diff --git a/ui/core/view-common.ts b/ui/core/view-common.ts index d0330abfb..87d1a90dd 100644 --- a/ui/core/view-common.ts +++ b/ui/core/view-common.ts @@ -133,6 +133,7 @@ export class View extends proxy.ProxyObject implements definition.View { public _cssClasses: Array = []; private _gesturesObserver: gestures.GesturesObserver; + private _updatingInheritedProperties: boolean; public _options: definition.Options; @@ -397,24 +398,40 @@ export class View extends proxy.ProxyObject implements definition.View { public _onPropertyChanged(property: dependencyObservable.Property, oldValue: any, newValue: any) { super._onPropertyChanged(property, oldValue, newValue); - // Implement Binding inheritance here. if (this._childrenCount > 0) { var shouldUpdateInheritableProps = ((property.metadata && property.metadata.inheritable) && - property.name !== "bindingContext" && !(property instanceof styling.Property)); + var that = this; if (shouldUpdateInheritableProps) { var notifyEachChild = function (child: View) { - child._setValue(property, newValue, dependencyObservable.ValueSource.Inherited); + child._setValue(property, that._getValue(property), dependencyObservable.ValueSource.Inherited); return true; }; + this._updatingInheritedProperties = true; this._eachChildView(notifyEachChild); + this._updatingInheritedProperties = false; } } this._checkMetadataOnPropertyChanged(property.metadata); } + public _isInheritedChange() { + if (this._updatingInheritedProperties) { + return true; + } + var parentView: View; + parentView = (this.parent); + while (parentView) { + if (parentView._updatingInheritedProperties) { + return true; + } + parentView = (parentView.parent); + } + return false; + } + public _checkMetadataOnPropertyChanged(metadata: dependencyObservable.PropertyMetadata) { if (metadata.affectsLayout) { this.requestLayout(); @@ -675,21 +692,6 @@ export class View extends proxy.ProxyObject implements definition.View { return changed; } - public _onBindingContextChanged(oldValue: any, newValue: any) { - super._onBindingContextChanged(oldValue, newValue); - - if (this._childrenCount === 0) { - return; - } - - var thatContext = this.bindingContext; - var eachChild = function (child: View): boolean { - child._setValue(bindable.Bindable.bindingContextProperty, thatContext, dependencyObservable.ValueSource.Inherited); - return true; - } - this._eachChildView(eachChild); - } - private _applyStyleFromScope() { var rootPage = getAncestor(this, "Page"); if (!rootPage || !rootPage.isLoaded) { @@ -784,7 +786,7 @@ export class View extends proxy.ProxyObject implements definition.View { private _inheritProperties(parentView: View) { var that = this; var inheritablePropertySetCallback = function (property: dependencyObservable.Property) { - if (property instanceof styling.Property || property.name === "bindingContext") { + if (property instanceof styling.Property) { return true; } if (property.metadata && property.metadata.inheritable) { @@ -827,7 +829,7 @@ export class View extends proxy.ProxyObject implements definition.View { view._setValue(bindable.Bindable.bindingContextProperty, undefined, dependencyObservable.ValueSource.Inherited); var inheritablePropertiesSetCallback = function (property: dependencyObservable.Property) { - if (property instanceof styling.Property || property.name === "bindingContext") { + if (property instanceof styling.Property) { return true; } if (property.metadata && property.metadata.inheritable) { @@ -892,4 +894,4 @@ export class View extends proxy.ProxyObject implements definition.View { public focus(): boolean { return undefined; } -} +} \ No newline at end of file diff --git a/ui/core/view.d.ts b/ui/core/view.d.ts index 85f00d75c..bbe9232c4 100644 --- a/ui/core/view.d.ts +++ b/ui/core/view.d.ts @@ -386,6 +386,7 @@ declare module "ui/core/view" { // TODO: Implement logic for stripping these lines out //@private + _isInheritedChange(): boolean; _domId: number; _cssClasses: Array; From 80336d2ea5d8e0541f62751ade2ba5cfbf07435b Mon Sep 17 00:00:00 2001 From: Nedyalko Nikolov Date: Fri, 8 May 2015 13:50:49 +0300 Subject: [PATCH 2/3] Property changed disabled for inheritable properties only. --- ui/core/bindable.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/core/bindable.ts b/ui/core/bindable.ts index f5ed4535e..cc924fe3a 100644 --- a/ui/core/bindable.ts +++ b/ui/core/bindable.ts @@ -80,7 +80,7 @@ export class Bindable extends dependencyObservable.DependencyObservable implemen trace.write("Bindable._onPropertyChanged(" + this + ") " + property.name, trace.categories.Binding); super._onPropertyChanged(property, oldValue, newValue); if (this instanceof viewModule.View) { - if (((this))._isInheritedChange() === true) { + if (property.metadata.inheritable && ((this))._isInheritedChange() === true) { return } } From 4a6850a77e8f415750e7124e2bdb10de2ff7ca6f Mon Sep 17 00:00:00 2001 From: Nedyalko Nikolov Date: Fri, 8 May 2015 15:16:51 +0300 Subject: [PATCH 3/3] Added missing semicolon. --- ui/core/bindable.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/core/bindable.ts b/ui/core/bindable.ts index cc924fe3a..f5fc7465d 100644 --- a/ui/core/bindable.ts +++ b/ui/core/bindable.ts @@ -81,7 +81,7 @@ export class Bindable extends dependencyObservable.DependencyObservable implemen super._onPropertyChanged(property, oldValue, newValue); if (this instanceof viewModule.View) { if (property.metadata.inheritable && ((this))._isInheritedChange() === true) { - return + return; } } var binding = this._bindings[property.name];