Merge pull request #129 from NativeScript/nnikolov/BindingContextIssue

Fixed issue when bindingContext is bound more than once.

CrossPlatformModules.csproj

@@ -137,6 +137,9 @@
     <TypeScriptCompile Include="apps\tests\fps-meter-tests.ts" />
     <TypeScriptCompile Include="apps\tests\trace-tests.ts" />
     <TypeScriptCompile Include="apps\tests\ui-test.ts" />
+    <TypeScriptCompile Include="apps\tests\ui\bindingContext_testPage.ts">
+      <DependentUpon>bindingContext_testPage.xml</DependentUpon>
+    </TypeScriptCompile>
     <TypeScriptCompile Include="apps\tests\ui\time-picker\time-picker-tests-native.android.ts">
       <DependentUpon>time-picker-tests-native.d.ts</DependentUpon>
     </TypeScriptCompile>
@@ -593,6 +596,7 @@
     <Content Include="apps\template-settings\app.css" />
     <Content Include="apps\tests\app\location-example.xml" />
     <Content Include="apps\tests\pages\page18.xml" />
+    <Content Include="apps\tests\ui\bindingContext_testPage.xml" />
     <Content Include="apps\tests\ui\label\label-tests-wrong.css" />
     <Content Include="apps\tests\pages\files\other.xml" />
     <Content Include="apps\tests\pages\files\test.android.phone.xml" />

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");
 
 // <snippet module="ui/core/bindable" title="bindable">
 // 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 = <labelModule.Label>(page.getViewById("upperStackLabel"));
+        var label1 = <labelModule.Label>(page.getViewById("label1"));
+        var label2 = <labelModule.Label>(page.getViewById("label2"));
+
+        TKUnit.assertEqual(upperStackLabel.text, label1.text);
+        TKUnit.assertEqual(upperStackLabel.text, label2.text);
+    }
+
+    helper.navigateToModuleAndRunTest("./tests/ui/bindingContext_testPage", testFunc);
 }

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 = <pageModule.Page>args.object;
+    page.bindingContext = viewModel;
+}

apps/tests/ui/bindingContext_testPage.xml

@@ -0,0 +1,15 @@
+<Page xmlns="http://www.nativescript.org/tns.xsd"
+      xmlns:g="components/grid-view/grid-view"
+      loaded="pageLoaded">
+	<StackLayout id="upperStack">
+		<Label id="upperStackLabel" text="{{ item.Title }}" />
+		<StackLayout id="firstStack" bindingContext="{{ item }}" orientation="horizontal">
+			<Label id="labelText1" text="1" />
+			<Label id="label1" text="{{ Title }}" />
+		</StackLayout>
+		<StackLayout id="secondStack" bindingContext="{{ item }}" orientation="horizontal">
+			<Label id="labelText2" text="2" />
+			<Label id="label2" text="{{ Title }}" />
+		</StackLayout>
+	</StackLayout>
+</Page>

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<T extends view.View>(createFunc: () => T, interactWithViewFunc?: (view: T) => void) {
     var newPage: page.Page;
     var testFinished = false;

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;
     }

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 = <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 (this instanceof viewModule.View) {
-        if (property === Bindable.bindingContextProperty) {
+            if (property.metadata.inheritable && (<viewModule.View>(<any>this))._isInheritedChange() === true) {
-            this._onBindingContextChanged(oldValue, newValue);
+                return;
-        }
-
-        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);
             }
-            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) {
+            if (binding.updating) {
-                // Updating binding context trough binding should not rebind the binding context.
                 continue;
             }
 

ui/core/dependency-observable.ts

@@ -23,9 +23,19 @@ function validateRegisterParameters(name: string, ownerType: string) {
     }
 }
 
-function getPropertyByNameAndType(name: string, ownerType: string): Property {
+function getPropertyByNameAndType(name: string, owner: any): Property {
-    var key = generatePropertyKey(name, ownerType);
+    var baseClasses = types.getBaseClasses(owner);
-    return propertyFromKey[key];
+    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<string, any>(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);
-        var property = getPropertyByNameAndType(name, this.typeName);
         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 {

ui/core/view-common.ts

@@ -133,6 +133,7 @@ export class View extends proxy.ProxyObject implements definition.View {
     public _cssClasses: Array<string> = [];
 
     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 = <View>(this.parent);
+        while (parentView) {
+            if (parentView._updatingInheritedProperties) {
+                return true;
+            }
+            parentView = <View>(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;
     }
-}
+}

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<string>;