From 5ef6774cd94e2bea18b94df47ed35938704fe1cd Mon Sep 17 00:00:00 2001 From: Elliott Sprehn Date: Thu, 15 Jan 2015 15:03:44 -0800 Subject: [PATCH] Add two way data binding. All reflected attributes two way bind on SkyElement, so now doing is enough to get two way binding on the value attribute so users doing will get the inputValue property updated as the user types. R=abarth@chromium.org, ojan@chromium.org Review URL: https://codereview.chromium.org/850383002 --- examples/widgets/index.sky | 13 +++++++-- framework/sky-element/observe.sky | 20 +++++-------- framework/sky-element/sky-binder.sky | 2 ++ framework/sky-element/sky-element.sky | 36 ++++++++++++++++++++++++ tests/framework/observe-expected.txt | 4 +-- tests/framework/observe.sky | 10 ++----- tests/framework/templates-expected.txt | 13 +++++---- tests/framework/templates.sky | 39 +++++++++++++++++++++++++- tests/resources/test-element.sky | 2 ++ 9 files changed, 107 insertions(+), 32 deletions(-) diff --git a/examples/widgets/index.sky b/examples/widgets/index.sky index d4e55c0d108..98b0ab47a4d 100644 --- a/examples/widgets/index.sky +++ b/examples/widgets/index.sky @@ -25,7 +25,10 @@ } - + + +
value = {{ inputValue }}
+ Button @@ -37,7 +40,8 @@
Checkbox
highlight: {{ myCheckbox.highlight }}
checked: {{ myCheckbox.checked }}
-
Checkbox, default checked.
+
Checkbox, default checked.
+
checked: {{ checked }}
 @@ -61,6 +65,8 @@ module.exports = class extends SkyElement { this.myCheckbox = null; this.myText = null; this.clickCount = 0; + this.inputValue = "Ready"; + this.checked = false; } attached() { this.myButton = this.shadowRoot.getElementById('button'); @@ -70,7 +76,8 @@ module.exports = class extends SkyElement { } handleClick(event) { this.clickCount++; - this.myText.value = "Moar clicking " + this.clickCount; + this.checked = !this.checked; + this.inputValue = "Moar clicking " + this.clickCount; } }.register(); diff --git a/framework/sky-element/observe.sky b/framework/sky-element/observe.sky index 23c6fb79358..415b4b8f108 100644 --- a/framework/sky-element/observe.sky +++ b/framework/sky-element/observe.sky @@ -601,6 +601,9 @@ Observer.prototype = { return this.value_; }, + setValue: function(newValue) { + }, + close: function() { if (this.state_ != OPENED) return; @@ -887,17 +890,12 @@ CompoundObserver.prototype = createObject({ function identFn(value) { return value; } -function ObserverTransform(observable, getValueFn, setValueFn, - dontPassThroughSet) { +function ObserverTransform(observable, getValueFn) { this.callback_ = undefined; this.target_ = undefined; this.value_ = undefined; this.observable_ = observable; this.getValueFn_ = getValueFn || identFn; - this.setValueFn_ = setValueFn || identFn; - // TODO(rafaelw): This is a temporary hack. PolymerExpressions needs this - // at the moment because of a bug in it's dependency tracking. - this.dontPassThroughSet_ = dontPassThroughSet; } ObserverTransform.prototype = { @@ -918,6 +916,9 @@ ObserverTransform.prototype = { this.callback_.call(this.target_, this.value_, oldValue); }, + setValue: function(oldValue) { + }, + discardChanges: function() { this.value_ = this.getValueFn_(this.observable_.discardChanges()); return this.value_; @@ -927,12 +928,6 @@ ObserverTransform.prototype = { return this.observable_.deliver(); }, - setValue: function(value) { - value = this.setValueFn_(value); - if (!this.dontPassThroughSet_ && this.observable_.setValue) - return this.observable_.setValue(value); - }, - close: function() { if (this.observable_) this.observable_.close(); @@ -941,7 +936,6 @@ ObserverTransform.prototype = { this.observable_ = undefined; this.value_ = undefined; this.getValueFn_ = undefined; - this.setValueFn_ = undefined; } } diff --git a/framework/sky-element/sky-binder.sky b/framework/sky-element/sky-binder.sky index 8eff2f29875..aca84a3e839 100644 --- a/framework/sky-element/sky-binder.sky +++ b/framework/sky-element/sky-binder.sky @@ -160,6 +160,8 @@ class PropertyDirective { node[name] = value; }); } + if (typeof node.addPropertyBinding == 'function') + node.addPropertyBinding(this.name, observable); return observable; } } diff --git a/framework/sky-element/sky-element.sky b/framework/sky-element/sky-element.sky index a76874a5f12..7b7933ab357 100644 --- a/framework/sky-element/sky-element.sky +++ b/framework/sky-element/sky-element.sky @@ -160,6 +160,8 @@ class SkyElement extends HTMLElement { createdCallback() { this.isAttached = false; + this.propertyBindings = null; + this.dirtyPropertyBindings = null; this.created(); Object.preventExtensions(this); @@ -208,6 +210,8 @@ class SkyElement extends HTMLElement { } notifyPropertyChanged(name, oldValue, newValue) { + if (oldValue == newValue) + return; var notifier = Object.getNotifier(this); notifier.notify({ type: 'update', @@ -217,6 +221,38 @@ class SkyElement extends HTMLElement { var handler = this[name + 'Changed']; if (typeof handler == 'function') handler.call(this, oldValue, newValue); + this.schedulePropertyBindingUpdate(name); + } + + addPropertyBinding(name, binding) { + if (!this.propertyBindings) + this.propertyBindings = new Map(); + this.propertyBindings.set(name, binding); + } + + getPropertyBinding(name) { + if (!this.propertyBindings) + return null; + return this.propertyBindings.get(name); + } + + schedulePropertyBindingUpdate(name) { + if (!this.dirtyPropertyBindings) { + this.dirtyPropertyBindings = new Set(); + Promise.resolve().then(this.updatePropertyBindings.bind(this)); + } + this.dirtyPropertyBindings.add(name); + } + + updatePropertyBindings() { + for (var name of this.dirtyPropertyBindings) { + var binding = this.getPropertyBinding(name); + if (binding) { + binding.setValue(this[name]); + binding.discardChanges(); + } + } + this.dirtyPropertyBindings = null; } }; diff --git a/tests/framework/observe-expected.txt b/tests/framework/observe-expected.txt index dac0010ae06..b7db27a36cc 100644 --- a/tests/framework/observe-expected.txt +++ b/tests/framework/observe-expected.txt @@ -1,7 +1,7 @@ ERROR: Exception caught during observer callback: ouch -SOURCE: http://127.0.0.1:8000/sky/framework/sky-element/observe.sky:627 +SOURCE: http://127.0.0.1:8000/sky/framework/sky-element/observe.sky:630 ERROR: Exception caught during observer callback: ouch -SOURCE: http://127.0.0.1:8000/sky/framework/sky-element/observe.sky:627 +SOURCE: http://127.0.0.1:8000/sky/framework/sky-element/observe.sky:630 Running 79 tests ok 1 Path constructor throws ok 2 Path path validity diff --git a/tests/framework/observe.sky b/tests/framework/observe.sky index 2fa89259ba6..a2bc3955b74 100644 --- a/tests/framework/observe.sky +++ b/tests/framework/observe.sky @@ -334,11 +334,8 @@ describe('ObserverTransform', function() { function valueFn(value) { return value * 2; } - function setValueFn(value) { return value / 2; } - observer = new ObserverTransform(new PathObserver(obj, 'foo'), - valueFn, - setValueFn); + valueFn); observer.open(callback); obj.foo = 2; @@ -347,11 +344,10 @@ describe('ObserverTransform', function() { assertNoChanges(); observer.setValue(2); - assert.strictEqual(obj.foo, 1); - assertPathChanges(2, 4); + assert.strictEqual(obj.foo, 2); obj.foo = 10; - assertPathChanges(20, 2); + assertPathChanges(20, 4); observer.close(); }); diff --git a/tests/framework/templates-expected.txt b/tests/framework/templates-expected.txt index 587ea4eadca..b59402a7378 100644 --- a/tests/framework/templates-expected.txt +++ b/tests/framework/templates-expected.txt @@ -1,4 +1,4 @@ -Running 12 tests +Running 13 tests ok 1 SkyElement should stamp when the element is inserted ok 2 SkyElement should update isAttached when inserting ok 3 SkyElement should handle parser created elements with attributes @@ -8,9 +8,10 @@ ok 6 SkyElement should convert boolean reflected attributes ok 7 SkyElement should convert string reflected attributes ok 8 SkyElement should convert number reflected attributes ok 9 SkyElement should connect data binding -ok 10 SkyElement should connect template event handlers -ok 11 SkyElement should connect host event handlers -ok 12 SkyElement should call shadowRootReady after creating the template instance -12 tests -12 pass +ok 10 SkyElement should two way bind attributes +ok 11 SkyElement should connect template event handlers +ok 12 SkyElement should connect host event handlers +ok 13 SkyElement should call shadowRootReady after creating the template instance +13 tests +13 pass 0 fail diff --git a/tests/framework/templates.sky b/tests/framework/templates.sky index aca46d80f44..747d9067bff 100644 --- a/tests/framework/templates.sky +++ b/tests/framework/templates.sky @@ -131,6 +131,43 @@ describe("SkyElement", function() { }); }); + it("should two way bind attributes", function(done) { + sandbox.appendChild(element); + var checkbox = element.shadowRoot.getElementById("checkbox"); + assert.isFalse(checkbox.checked); + assert.isFalse(element.checked); + element.checked = true; + assert.isTrue(element.checked); + assert.isFalse(checkbox.checked); + Promise.resolve().then(function() { + assert.isTrue(checkbox.checked); + checkbox.checked = false; + assert.isFalse(checkbox.checked); + return Promise.resolve().then(function() { + assert.isFalse(element.checked); + assert.isFalse(checkbox.checked); + checkbox.checked = true; + assert.isTrue(checkbox.checked); + return Promise.resolve().then(function() { + assert.isTrue(element.checked); + element.checked = true; + assert.isTrue(element.checked); + assert.isTrue(checkbox.checked); + element.checked = false; + assert.isFalse(element.checked); + assert.isTrue(checkbox.checked); + return Promise.resolve().then(function() { + assert.isFalse(checkbox.checked); + assert.isFalse(element.checked); + done(); + }); + }); + }); + }).catch(function(e) { + done(e); + }); + }); + it("should connect template event handlers", function() { sandbox.appendChild(element); var inside = element.shadowRoot.getElementById("inside"); @@ -160,4 +197,4 @@ describe("SkyElement", function() { }); }); - \ No newline at end of file + diff --git a/tests/resources/test-element.sky b/tests/resources/test-element.sky index 5c48950b7f1..be6403c9ce0 100644 --- a/tests/resources/test-element.sky +++ b/tests/resources/test-element.sky @@ -4,6 +4,7 @@ // found in the LICENSE file. --> +