Skip to content

Commit 7fce2cb

Browse files
author
Nedyalko Nikolov
committed
Merge pull request NativeScript#129 from NativeScript/nnikolov/BindingContextIssue
Fixed issue when bindingContext is bound more than once.
2 parents 7609f1f + 4a6850a commit 7fce2cb

File tree

10 files changed

+135
-45
lines changed

10 files changed

+135
-45
lines changed

‎CrossPlatformModules.csproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@
137137
<TypeScriptCompile Include="apps\tests\fps-meter-tests.ts" />
138138
<TypeScriptCompile Include="apps\tests\trace-tests.ts" />
139139
<TypeScriptCompile Include="apps\tests\ui-test.ts" />
140+
<TypeScriptCompile Include="apps\tests\ui\bindingContext_testPage.ts">
141+
<DependentUpon>bindingContext_testPage.xml</DependentUpon>
142+
</TypeScriptCompile>
140143
<TypeScriptCompile Include="apps\tests\ui\time-picker\time-picker-tests-native.android.ts">
141144
<DependentUpon>time-picker-tests-native.d.ts</DependentUpon>
142145
</TypeScriptCompile>
@@ -593,6 +596,7 @@
593596
<Content Include="apps\template-settings\app.css" />
594597
<Content Include="apps\tests\app\location-example.xml" />
595598
<Content Include="apps\tests\pages\page18.xml" />
599+
<Content Include="apps\tests\ui\bindingContext_testPage.xml" />
596600
<Content Include="apps\tests\ui\label\label-tests-wrong.css" />
597601
<Content Include="apps\tests\pages\files\other.xml" />
598602
<Content Include="apps\tests\pages\files\test.android.phone.xml" />

‎apps/tests/ui/bindable-tests.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import utils = require("utils/utils");
1010
import pageModule = require("ui/page");
1111
import stackLayoutModule = require("ui/layouts/stack-layout");
1212
import bindingBuilder = require("ui/builder/binding-builder");
13+
import labelModule = require("ui/label");
1314

1415
// <snippet module="ui/core/bindable" title="bindable">
1516
// 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 () {
459460
TKUnit.assert(bindOptions.targetProperty === "targetBindProperty", "Expected: targetBindProperty, Actual: " + bindOptions.targetProperty);
460461
TKUnit.assert(bindOptions.expression === "bindProperty * 2", "Expected: bindProperty * 2, Actual:" + bindOptions.expression);
461462
TKUnit.assert(bindOptions.twoWay === true, "Expected: true, Actual: " + bindOptions.twoWay);
463+
}
464+
465+
export var test_TwoElementsBindingToSameBindingContext = function () {
466+
var testFunc = function (page: pageModule.Page) {
467+
var upperStackLabel = <labelModule.Label>(page.getViewById("upperStackLabel"));
468+
var label1 = <labelModule.Label>(page.getViewById("label1"));
469+
var label2 = <labelModule.Label>(page.getViewById("label2"));
470+
471+
TKUnit.assertEqual(upperStackLabel.text, label1.text);
472+
TKUnit.assertEqual(upperStackLabel.text, label2.text);
473+
}
474+
475+
helper.navigateToModuleAndRunTest("./tests/ui/bindingContext_testPage", testFunc);
462476
}

‎apps/tests/ui/bindingContext_testPage.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import observableModule = require("data/observable");
2+
import pageModule = require("ui/page");
3+
4+
class MainViewModel extends observableModule.Observable {
5+
private _item: any;
6+
7+
constructor() {
8+
super();
9+
10+
this.item = { Title: "Alabala" };
11+
}
12+
13+
get item(): any {
14+
return this._item;
15+
}
16+
17+
set item(value: any) {
18+
if (this._item !== value) {
19+
this._item = value;
20+
this.notifyPropertyChanged("item", value);
21+
}
22+
}
23+
24+
notifyPropertyChanged(propertyName: string, value: any) {
25+
this.notify({ object: this, eventName: observableModule.Observable.propertyChangeEvent, propertyName: propertyName, value: value });
26+
}
27+
}
28+
29+
var viewModel = new MainViewModel();
30+
31+
export function pageLoaded(args: observableModule.EventData) {
32+
var page = <pageModule.Page>args.object;
33+
page.bindingContext = viewModel;
34+
}

‎apps/tests/ui/bindingContext_testPage.xml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<Page xmlns="http://www.nativescript.org/tns.xsd"
2+
xmlns:g="components/grid-view/grid-view"
3+
loaded="pageLoaded">
4+
<StackLayout id="upperStack">
5+
<Label id="upperStackLabel" text="{{ item.Title }}" />
6+
<StackLayout id="firstStack" bindingContext="{{ item }}" orientation="horizontal">
7+
<Label id="labelText1" text="1" />
8+
<Label id="label1" text="{{ Title }}" />
9+
</StackLayout>
10+
<StackLayout id="secondStack" bindingContext="{{ item }}" orientation="horizontal">
11+
<Label id="labelText2" text="2" />
12+
<Label id="label2" text="{{ Title }}" />
13+
</StackLayout>
14+
</StackLayout>
15+
</Page>

‎apps/tests/ui/helper.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,16 @@ export function buildUIAndRunTest(controlToTest, testFunction, pageCss?) {
143143
}
144144
}
145145

146+
export function navigateToModuleAndRunTest(moduleName, testFunction) {
147+
navigateToModule(moduleName);
148+
try {
149+
testFunction(frame.topmost().currentPage);
150+
}
151+
finally {
152+
goBack();
153+
}
154+
}
155+
146156
export function buildUIWithWeakRefAndInteract<T extends view.View>(createFunc: () => T, interactWithViewFunc?: (view: T) => void) {
147157
var newPage: page.Page;
148158
var testFinished = false;

‎file-system/file-system.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ declare module "file-system" {
154154

155155
/**
156156
* Gets the root folder for the current application. This Folder is private for the application and not accessible from Users/External apps.
157+
* iOS - this folder is read-only and contains the app and all its resources.
157158
*/
158159
export function currentApp(): Folder;
159160
}

‎ui/core/bindable.ts‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,19 @@ import types = require("utils/types");
77
import trace = require("trace");
88
import polymerExpressions = require("js-libs/polymer-expressions");
99
import bindingBuilder = require("../builder/binding-builder");
10+
import viewModule = require("ui/core/view");
1011

1112
var bindingContextProperty = new dependencyObservable.Property(
1213
"bindingContext",
1314
"Bindable",
14-
new dependencyObservable.PropertyMetadata(undefined, dependencyObservable.PropertyMetadataSettings.Inheritable) // TODO: Metadata options?
15+
new dependencyObservable.PropertyMetadata(undefined, dependencyObservable.PropertyMetadataSettings.Inheritable, onBindingContextChanged)
1516
);
1617

18+
function onBindingContextChanged(data: dependencyObservable.PropertyChangeData) {
19+
var bindable = <Bindable>data.object;
20+
bindable._onBindingContextChanged(data.oldValue, data.newValue);
21+
}
22+
1723
var contextKey = "context";
1824
var resourcesKey = "resources";
1925

@@ -73,24 +79,21 @@ export class Bindable extends dependencyObservable.DependencyObservable implemen
7379
public _onPropertyChanged(property: dependencyObservable.Property, oldValue: any, newValue: any) {
7480
trace.write("Bindable._onPropertyChanged(" + this + ") " + property.name, trace.categories.Binding);
7581
super._onPropertyChanged(property, oldValue, newValue);
76-
77-
if (property === Bindable.bindingContextProperty) {
78-
this._onBindingContextChanged(oldValue, newValue);
82+
if (this instanceof viewModule.View) {
83+
if (property.metadata.inheritable && (<viewModule.View>(<any>this))._isInheritedChange() === true) {
84+
return;
85+
}
7986
}
80-
8187
var binding = this._bindings[property.name];
82-
if (binding) {
83-
// we should remove (unbind and delete) binding if binding is oneWay and update is not triggered
84-
// by binding itself.
85-
var shouldRemoveBinding = !binding.updating && !binding.options.twoWay;
86-
if (shouldRemoveBinding) {
87-
trace.write("_onPropertyChanged(" + this + ") removing binding for property: " + property.name, trace.categories.Binding);
88-
this.unbind(property.name);
89-
}
90-
else {
88+
if (binding && !binding.updating) {
89+
if (binding.options.twoWay) {
9190
trace.write("_updateTwoWayBinding(" + this + "): " + property.name, trace.categories.Binding);
9291
this._updateTwoWayBinding(property.name, newValue);
9392
}
93+
else {
94+
trace.write("_onPropertyChanged(" + this + ") removing binding for property: " + property.name, trace.categories.Binding);
95+
this.unbind(property.name);
96+
}
9497
}
9598
}
9699

@@ -99,8 +102,7 @@ export class Bindable extends dependencyObservable.DependencyObservable implemen
99102
for (var p in this._bindings) {
100103
binding = this._bindings[p];
101104

102-
if (binding.options.targetProperty === Bindable.bindingContextProperty.name && binding.updating) {
103-
// Updating binding context trough binding should not rebind the binding context.
105+
if (binding.updating) {
104106
continue;
105107
}
106108

‎ui/core/dependency-observable.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,19 @@ function validateRegisterParameters(name: string, ownerType: string) {
2323
}
2424
}
2525

26-
function getPropertyByNameAndType(name: string, ownerType: string): Property {
27-
var key = generatePropertyKey(name, ownerType);
28-
return propertyFromKey[key];
26+
function getPropertyByNameAndType(name: string, owner: any): Property {
27+
var baseClasses = types.getBaseClasses(owner);
28+
var i;
29+
var result;
30+
var key;
31+
for (i = 0; i < baseClasses.length; i++) {
32+
key = generatePropertyKey(name, baseClasses[i]);
33+
result = propertyFromKey[key];
34+
if (result) {
35+
break;
36+
}
37+
}
38+
return result;
2939
}
3040

3141
export module PropertyMetadataSettings {
@@ -254,13 +264,10 @@ export class PropertyEntry implements definition.PropertyEntry {
254264
}
255265

256266
export class DependencyObservable extends observable.Observable {
257-
// TODO: measure the performance of the dictionary vs. JS Object with numeric keys
258-
// private _values = new containers.Dictionary<string, any>(new containers.StringComparer());
259267
private _propertyEntries = {};
260268

261269
public set(name: string, value: any) {
262-
// TODO: Properties must be registered with the correct owner type for this routine to work
263-
var property = getPropertyByNameAndType(name, this.typeName);
270+
var property = getPropertyByNameAndType(name, this);
264271
if (property) {
265272
this._setValue(property, value, ValueSource.Local);
266273
} else {
@@ -269,7 +276,7 @@ export class DependencyObservable extends observable.Observable {
269276
}
270277

271278
public get(name: string): any {
272-
var property = getPropertyByNameAndType(name, this.typeName);
279+
var property = getPropertyByNameAndType(name, this);
273280
if (property) {
274281
return this._getValue(property);
275282
} else {

‎ui/core/view-common.ts‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ export class View extends proxy.ProxyObject implements definition.View {
133133
public _cssClasses: Array<string> = [];
134134

135135
private _gesturesObserver: gestures.GesturesObserver;
136+
private _updatingInheritedProperties: boolean;
136137

137138
public _options: definition.Options;
138139

@@ -397,24 +398,40 @@ export class View extends proxy.ProxyObject implements definition.View {
397398

398399
public _onPropertyChanged(property: dependencyObservable.Property, oldValue: any, newValue: any) {
399400
super._onPropertyChanged(property, oldValue, newValue);
400-
// Implement Binding inheritance here.
401401

402402
if (this._childrenCount > 0) {
403403
var shouldUpdateInheritableProps = ((property.metadata && property.metadata.inheritable) &&
404-
property.name !== "bindingContext" &&
405404
!(property instanceof styling.Property));
405+
var that = this;
406406
if (shouldUpdateInheritableProps) {
407407
var notifyEachChild = function (child: View) {
408-
child._setValue(property, newValue, dependencyObservable.ValueSource.Inherited);
408+
child._setValue(property, that._getValue(property), dependencyObservable.ValueSource.Inherited);
409409
return true;
410410
};
411+
this._updatingInheritedProperties = true;
411412
this._eachChildView(notifyEachChild);
413+
this._updatingInheritedProperties = false;
412414
}
413415
}
414416

415417
this._checkMetadataOnPropertyChanged(property.metadata);
416418
}
417419

420+
public _isInheritedChange() {
421+
if (this._updatingInheritedProperties) {
422+
return true;
423+
}
424+
var parentView: View;
425+
parentView = <View>(this.parent);
426+
while (parentView) {
427+
if (parentView._updatingInheritedProperties) {
428+
return true;
429+
}
430+
parentView = <View>(parentView.parent);
431+
}
432+
return false;
433+
}
434+
418435
public _checkMetadataOnPropertyChanged(metadata: dependencyObservable.PropertyMetadata) {
419436
if (metadata.affectsLayout) {
420437
this.requestLayout();
@@ -675,21 +692,6 @@ export class View extends proxy.ProxyObject implements definition.View {
675692
return changed;
676693
}
677694

678-
public _onBindingContextChanged(oldValue: any, newValue: any) {
679-
super._onBindingContextChanged(oldValue, newValue);
680-
681-
if (this._childrenCount === 0) {
682-
return;
683-
}
684-
685-
var thatContext = this.bindingContext;
686-
var eachChild = function (child: View): boolean {
687-
child._setValue(bindable.Bindable.bindingContextProperty, thatContext, dependencyObservable.ValueSource.Inherited);
688-
return true;
689-
}
690-
this._eachChildView(eachChild);
691-
}
692-
693695
private _applyStyleFromScope() {
694696
var rootPage = getAncestor(this, "Page");
695697
if (!rootPage || !rootPage.isLoaded) {
@@ -784,7 +786,7 @@ export class View extends proxy.ProxyObject implements definition.View {
784786
private _inheritProperties(parentView: View) {
785787
var that = this;
786788
var inheritablePropertySetCallback = function (property: dependencyObservable.Property) {
787-
if (property instanceof styling.Property || property.name === "bindingContext") {
789+
if (property instanceof styling.Property) {
788790
return true;
789791
}
790792
if (property.metadata && property.metadata.inheritable) {
@@ -827,7 +829,7 @@ export class View extends proxy.ProxyObject implements definition.View {
827829

828830
view._setValue(bindable.Bindable.bindingContextProperty, undefined, dependencyObservable.ValueSource.Inherited);
829831
var inheritablePropertiesSetCallback = function (property: dependencyObservable.Property) {
830-
if (property instanceof styling.Property || property.name === "bindingContext") {
832+
if (property instanceof styling.Property) {
831833
return true;
832834
}
833835
if (property.metadata && property.metadata.inheritable) {
@@ -892,4 +894,4 @@ export class View extends proxy.ProxyObject implements definition.View {
892894
public focus(): boolean {
893895
return undefined;
894896
}
895-
}
897+
}

‎ui/core/view.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ declare module "ui/core/view" {
386386

387387
// TODO: Implement logic for stripping these lines out
388388
//@private
389+
_isInheritedChange(): boolean;
389390
_domId: number;
390391
_cssClasses: Array<string>;
391392

0 commit comments

Comments
 (0)