Skip to content

Commit

Permalink
Call deliver() automatically when widgets created.
Browse files Browse the repository at this point in the history 
Runs both for programatically and declarative widgets, after the user-specified parameters
have been applied.  Note that when widgets are created programatically, deliver() will
be called even if the widget isn't yet attached to the DOM.

Avoids widgets having a stale state (i.e. DOM nodes not updated or computed properties
not computed) when applications access the widget immediately after it has been created,
before the widget is updated automatically due to the timer firing.

Fixes ibm-js#385.
  • Loading branch information
@wkeese
wkeese committed Mar 19, 2015
1 parent 4ec344b commit 1a0aa55
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 166 deletions.
10 changes: 9 additions & 1 deletion Bidi.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,20 @@ define([
* @protected
*/
getInheritedDir: function () {
if (has("inherited-dir")) {
if (this.attached && has("inherited-dir")) {
return window.getComputedStyle(this, null).direction;
}
return this.ownerDocument.body.dir || this.ownerDocument.documentElement.dir || "ltr";
},

attachedCallback: function () {
if (has("inherited-dir")) {
// Now that the widget is attached to the DOM, need to retrigger computation of effectiveDir.
this.notifyCurrentValue("dir");
this.deliver();
}
},

/**
* Returns the right direction of text.
*
Expand Down
99 changes: 54 additions & 45 deletions CustomElement.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,50 @@ define([
this[pa.prop] = pa.value;
}
}, this);

if (!has("setter-on-native-prop")) {
var setterMap = this._nativePropSetterMap,
attrs = Object.keys(setterMap);

// Call custom setters for initial values of attributes with shadow properties (dir, tabIndex, etc)
attrs.forEach(function (attrName) {
if (this.hasAttribute(attrName)) { // initial value was specified
var value = this.getAttribute(attrName);
this.removeAttribute(attrName);
setterMap[attrName].call(this, value); // call custom setter
}
}, this);

// Begin watching for changes to those DOM attributes.
// Note that (at least on Chrome) I could use attributeChangedCallback() instead, which is
// synchronous, so Widget#deliver() will work as expected, but OTOH gets lots of notifications
// that I don't care about.
// If Polymer is loaded, use MutationObserver rather than WebKitMutationObserver
// to avoid error about "referencing a Node in a context where it does not exist".
/* global WebKitMutationObserver */
var MO = window.MutationObserver || WebKitMutationObserver; // for jshint
var observer = new MO(function (records) {
records.forEach(function (mr) {
var attrName = mr.attributeName,
setter = setterMap[attrName],
newValue = this.getAttribute(attrName);
if (newValue !== null) {
this.removeAttribute(attrName);
setter.call(this, newValue);
}
}, this);
}.bind(this));
observer.observe(this, {
subtree: false,
attributeFilter: attrs,
attributes: true
});
}

// TODO: deliver() should be called from Widget but I can't get dcl.advise() w/around advice to work.
if (this.deliver) {
this.deliver();
}
}
}),

Expand All @@ -182,52 +226,17 @@ define([
* @method
* @fires module:delite/CustomElement#customelement-attached
*/
attachedCallback: dcl.after(function () {
this.attached = true;
this.emit("customelement-attached", {
bubbles: false,
cancelable: false
});

// Prevent against repeated calls
this.attachedCallback = nop;

if (!has("setter-on-native-prop")) {
var setterMap = this._nativePropSetterMap,
attrs = Object.keys(setterMap);

// Call custom setters for initial values of attributes with shadow properties (dir, tabIndex, etc)
attrs.forEach(function (attrName) {
if (this.hasAttribute(attrName)) { // initial value was specified
var value = this.getAttribute(attrName);
this.removeAttribute(attrName);
setterMap[attrName].call(this, value); // call custom setter
}
}, this);
attachedCallback: dcl.advise({
before: function () {
this.attached = true;

// Begin watching for changes to those DOM attributes.
// Note that (at least on Chrome) I could use attributeChangedCallback() instead, which is synchronous,
// so Widget#deliver() will work as expected, but OTOH gets lots of notifications
// that I don't care about.
// If Polymer is loaded, use MutationObserver rather than WebKitMutationObserver
// to avoid error about "referencing a Node in a context where it does not exist".
/* global WebKitMutationObserver */
var MO = window.MutationObserver || WebKitMutationObserver; // for jshint
var observer = new MO(function (records) {
records.forEach(function (mr) {
var attrName = mr.attributeName,
setter = setterMap[attrName],
newValue = this.getAttribute(attrName);
if (newValue !== null) {
this.removeAttribute(attrName);
setter.call(this, newValue);
}
}, this);
}.bind(this));
observer.observe(this, {
subtree: false,
attributeFilter: attrs,
attributes: true
// Prevent against repeated calls
this.attachedCallback = nop;
},
after: function () {
this.emit("customelement-attached", {
bubbles: false,
cancelable: false
});
}
}),
Expand Down
3 changes: 3 additions & 0 deletions register.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ define([
node[name] = params[name];
}
}
if (node.deliver) {
node.deliver();
}

return node;
};
Expand Down
1 change: 0 additions & 1 deletion tests/unit/CssState.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ define([
disabled: true,
checked: true
});
widget.deliver();

assert($(widget).hasClass("d-error"), "error state");
assert($(widget).hasClass("d-disabled"), "disabled");
Expand Down
1 change: 0 additions & 1 deletion tests/unit/FormValueWidget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ define([
myWidget.deliver();
assert(myWidget.focusNode.readOnly, "readOnly set on focusNode");


myWidget.readOnly = false;
myWidget.deliver();
assert.isFalse(myWidget.focusNode.readOnly, "readOnly not set on focusNode");
Expand Down
51 changes: 26 additions & 25 deletions tests/unit/FormWidget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,52 +70,55 @@ define([
assert(myWidget.valueNode.disabled, "disabled set on valueNode");
assert(myWidget.focusNode.disabled, "disabled set on focusNode");


myWidget.disabled = false;
myWidget.deliver();
assert.isFalse(myWidget.valueNode.disabled, "disabled not set on valueNode");
assert.isFalse(myWidget.focusNode.disabled, "disabled not set on focusNode");
},

"#tabIndex": function () {
// When !has("setter-on-native-prop"), tabIndex changes reported asynchronously even if you call
// this.deliver(). See code in CustomElement.js.
var d = this.async(1000);

// default tabIndex
var myWidget = new FormWidgetTest();
myWidget.deliver();
assert.strictEqual(myWidget.focusNode.getAttribute("tabindex"), "0", "default tabIndex");
assert.isFalse(HTMLElement.prototype.hasAttribute.call(myWidget, "tabindex"), "no tabIndex on root 1");

// specify initial tabIndex
myWidget = new FormWidgetTest({
tabIndex: "3"
});
myWidget.placeAt(container);
//myWidget.setAttribute("tabindex", 3);
myWidget.deliver();
assert.strictEqual(myWidget.focusNode.getAttribute("tabindex"), "3", "specified tabIndex");
assert.isFalse(HTMLElement.prototype.hasAttribute.call(myWidget, "tabindex"), "no tabIndex on root 2");

// Change tabIndex. At least on Chrome, this always happens asynchronously even if you call deliver().
myWidget.tabIndex = 4;
setTimeout(this.async().callback(function () {
assert.strictEqual(myWidget.focusNode.getAttribute("tabindex"), "4", "changed tabIndex");
setTimeout(d.rejectOnError(function () {
assert.strictEqual(myWidget.focusNode.getAttribute("tabindex"), "0", "default tabIndex");
assert.isFalse(HTMLElement.prototype.hasAttribute.call(myWidget, "tabindex"),
"no tabIndex on root 3");
}), 100);
"no tabIndex on root 1");

// specify initial tabIndex
myWidget = new FormWidgetTest({
tabIndex: "3"
});
setTimeout(d.rejectOnError(function () {
assert.strictEqual(myWidget.focusNode.getAttribute("tabindex"), "3", "specified tabIndex");
assert.isFalse(HTMLElement.prototype.hasAttribute.call(myWidget, "tabindex"),
"no tabIndex on root 2");

// Change tabIndex.
myWidget.tabIndex = 4;
setTimeout(d.callback(function () {
assert.strictEqual(myWidget.focusNode.getAttribute("tabindex"), "4", "changed tabIndex");
assert.isFalse(HTMLElement.prototype.hasAttribute.call(myWidget, "tabindex"),
"no tabIndex on root 3");
}), 10);
}), 10);
}), 10);
},

"#alt": function () {
var myWidget = new FormWidgetTest({
alt: "hello world"
});
myWidget.deliver();
assert.strictEqual(myWidget.focusNode.getAttribute("alt"), "hello world");
},

"#name": function () {
var myWidget = new FormWidgetTest({
name: "bob"
});
myWidget.deliver();
assert.strictEqual(myWidget.valueNode.getAttribute("name"), "bob");
}
},
Expand Down Expand Up @@ -155,7 +158,6 @@ define([
register.parse(container);

var myWidget = container.firstChild;
myWidget.deliver();

// tabIndex
assert.strictEqual(myWidget.field1.getAttribute("tabindex"), "0", "field1 default tabIndex");
Expand Down Expand Up @@ -204,7 +206,6 @@ define([
assert(myWidget.field1.disabled, "disabled set on field1");
assert(myWidget.field2.disabled, "disabled set on field2");


myWidget.disabled = false;
myWidget.deliver();
assert.isFalse(myWidget.valueNode.disabled, "disabled not set on valueNode");
Expand Down
2 changes: 0 additions & 2 deletions tests/unit/Scrollable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ define([

"Default CSS": function () {
var w = (new MyScrollableWidget({id: "mysw"})).placeAt(container);
w.deliver();

assert.strictEqual(w.scrollableNode, w,
"The scrollableNode should be the widget itself!");
Expand Down Expand Up @@ -71,7 +70,6 @@ define([
})
});
var w = (new ScrollableWithCustomScrollableNode()).placeAt(container);
w.deliver();
assert.strictEqual(w.scrollableNode, w.createdScrollableNode,
"The scrollableNode property has changed since it was set in render!");
// Test that the CSS class d-scrollable has been added by the mixin delite/Scrollable
Expand Down
12 changes: 1 addition & 11 deletions tests/unit/Widget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,7 @@ define([
assert.isFalse(declarative.hasAttribute("tabindex"), "tabindex attr removed");
assert.isTrue(declarative.isrange, "isrange set");
assert.isTrue(declarative.isbool, "isbool set");

var d = this.async(1000);

setTimeout(d.callback(function () {
assert.strictEqual(declarative.value, "5", "value");
}), 10);

return d;
assert.strictEqual(declarative.value, "5", "value");
},

notInPrototype: function () {
Expand Down Expand Up @@ -199,8 +192,6 @@ define([
},

declarative: function () {
// And also test for declarative widgets, to make sure the tabIndex property is
// removed from the root node, to prevent an extra tab stop
container.innerHTML = "<test-dir id=dirTest dir='rtl'/>";
var declarative = document.getElementById("dirTest");
register.parse(container);
Expand Down Expand Up @@ -233,7 +224,6 @@ define([
var myWidget = new TestWidget();
container.appendChild(myWidget);
myWidget.attachedCallback();
myWidget.deliver();

assert($(myWidget).hasClass("base2"), "baseClass is base2");

Expand Down
Loading

0 comments on commit 1a0aa55

Please sign in to comment.