From 47c454a315b6c0260c8f65e70ae9b30f924650df Mon Sep 17 00:00:00 2001
From: Misko Hevery
Date: Fri, 10 Dec 2010 13:55:18 -0800
Subject: change to keydown from keyup; add delayed $updateView
- There was a perceived lag when typing do to the fact that we were
listening on the keyup event instead of keydown. The issue with
keydown is that we can not read the value of the input field. To
solve this we schedule a defer call and perform the model update
then.
- To prevent calling $eval on root scope too many times as well as to
prevent drowning the browser with too many updates we now call the
$eval only after 25ms and any additional requests get ignored. The
new update service is called $updateView
---
.gitignore | 3 ++-
CHANGELOG.md | 7 ++++++
docs/collect.js | 3 ++-
docs/docs.js | 8 +++----
docs/service.template | 6 ++++-
perf/noangular.html | 19 +++++++++++++++
src/Browser.js | 15 ++++++------
src/Compiler.js | 1 +
src/Injector.js | 10 +++++++-
src/directives.js | 12 +++++-----
src/scenario/Scenario.js | 2 +-
src/services.js | 62 +++++++++++++++++++++++++++++++++++++++++++++---
src/validators.js | 2 +-
src/widgets.js | 22 ++++++++---------
test/BinderSpec.js | 3 ++-
test/servicesSpec.js | 57 ++++++++++++++++++++++++++++++++++++++++++--
test/testabilityPatch.js | 2 +-
test/widgetsSpec.js | 21 +++++++++-------
18 files changed, 204 insertions(+), 51 deletions(-)
create mode 100644 perf/noangular.html
diff --git a/.gitignore b/.gitignore
index ee16a0c8..1d447a4a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,5 +2,6 @@ build/
angularjs.netrc
jstd.log
.DS_Store
-regression/temp.html
+regression/temp*.html
+performance/temp*.html
.idea/workspace.xml
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9f04983b..a72281dc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,13 @@
not needed.
- $location service now listens for `onhashchange` events (if supported by browser) instead of
constant polling.
+- input widgets known listens on keydown events instead of keyup which improves perceived
+ performance
+
+### API
+
+- new service $updateView which should be used in favor of $root.$eval() to run a complete eval on
+ the entire document. This service bulks and throttles DOM updates to improve performance.
### Breaking changes
- API for accessing registered services — `scope.$inject` — was renamed to
diff --git a/docs/collect.js b/docs/collect.js
index e1af14ef..eb540051 100644
--- a/docs/collect.js
+++ b/docs/collect.js
@@ -244,7 +244,7 @@ var TAG = {
name: function(doc, name, value) {
var parts = value.split(/\./);
doc.name = value;
- doc.shortName = parts.pop();
+ doc.shortName = parts.pop().replace('#', '.');
doc.depth = parts.length;
},
param: function(doc, name, value){
@@ -378,6 +378,7 @@ function processNgDoc(documentation, doc) {
if (doc.methodOf) {
if (parent = documentation.byName[doc.methodOf]) {
(parent.method = parent.method || []).push(doc);
+ parent.method.sort(keywordSort);
} else {
throw 'Owner "' + doc.methodOf + '" is not defined.';
}
diff --git a/docs/docs.js b/docs/docs.js
index 7e6c2ac4..6bf86ed3 100644
--- a/docs/docs.js
+++ b/docs/docs.js
@@ -1,5 +1,3 @@
-SyntaxHighlighter['defaults'].toolbar = false;
-
DocsController.$inject = ['$location', '$browser', '$window'];
function DocsController($location, $browser, $window) {
this.pages = NG_PAGES;
@@ -38,10 +36,12 @@ function DocsController($location, $browser, $window) {
return "mailto:angular@googlegroups.com?" +
"subject=" + escape("Feedback on " + $location.href) + "&" +
"body=" + escape("Hi there,\n\nI read " + $location.href + " and wanted to ask ....");
- }
+ };
}
angular.filter('short', function(name){
return (name||'').split(/\./).pop();
-});
\ No newline at end of file
+});
+
+SyntaxHighlighter['defaults'].toolbar = false;
diff --git a/docs/service.template b/docs/service.template
index ee3a284e..c28bddc9 100644
--- a/docs/service.template
+++ b/docs/service.template
@@ -25,19 +25,23 @@
{{/requires}}
+{{#method.length}}
diff --git a/perf/noangular.html b/perf/noangular.html
new file mode 100644
index 00000000..8e0db311
--- /dev/null
+++ b/perf/noangular.html
@@ -0,0 +1,19 @@
+
+
+
+
+
+
+ Your name:
+
+ Hello {{yourname}}!
+
+
\ No newline at end of file
diff --git a/src/Browser.js b/src/Browser.js
index c93f115c..377c740c 100644
--- a/src/Browser.js
+++ b/src/Browser.js
@@ -136,9 +136,7 @@ function Browser(window, document, body, XHR, $log) {
* @methodOf angular.service.$browser
*/
self.poll = function() {
- foreach(pollFns, function(pollFn){
- pollFn();
- });
+ foreach(pollFns, function(pollFn){ pollFn(); });
};
/**
@@ -319,22 +317,23 @@ function Browser(window, document, body, XHR, $log) {
/**
* @workInProgress
- * @ngdoc
+ * @ngdoc method
* @name angular.service.$browser#defer
* @methodOf angular.service.$browser
+ * @param {function()} fn A function, who's execution should be defered.
+ * @param {int=} [delay=0] of milliseconds to defer the function execution.
*
* @description
- * Executes a fn asynchroniously via `setTimeout(fn, 0)`.
+ * Executes a fn asynchroniously via `setTimeout(fn, delay)`.
*
* Unlike when calling `setTimeout` directly, in test this function is mocked and instead of using
* `setTimeout` in tests, the fns are queued in an array, which can be programaticaly flushed via
* `$browser.defer.flush()`.
*
- * @param {function()} fn A function, who's execution should be defered.
*/
- self.defer = function(fn) {
+ self.defer = function(fn, delay) {
outstandingRequestCount++;
- setTimeout(function() { completeOutstandingRequest(fn); }, 0);
+ setTimeout(function() { completeOutstandingRequest(fn); }, delay || 0);
};
//////////////////////////////////////////////////////////////
diff --git a/src/Compiler.js b/src/Compiler.js
index eea36263..a18341f4 100644
--- a/src/Compiler.js
+++ b/src/Compiler.js
@@ -74,6 +74,7 @@ Template.prototype = {
*/
function retrieveScope(element) {
var scope;
+ element = jqLite(element);
while (element && !(scope = element.data($$scope))) {
element = element.parent();
}
diff --git a/src/Injector.js b/src/Injector.js
index 78858e86..f6cb897f 100644
--- a/src/Injector.js
+++ b/src/Injector.js
@@ -67,4 +67,12 @@ function createInjector(providerScope, providers, cache) {
}
return returnValue;
};
-}
\ No newline at end of file
+}
+
+function injectService(services, fn) {
+ return extend(fn, {$inject:services});;
+}
+
+function injectUpdateView(fn) {
+ return injectService(['$updateView'], fn);
+}
diff --git a/src/directives.js b/src/directives.js
index d40d6120..bf215cc8 100644
--- a/src/directives.js
+++ b/src/directives.js
@@ -423,14 +423,14 @@ angularDirective("ng:bind-attr", function(expression){
* TODO: maybe we should consider allowing users to control event propagation in the future.
*/
angularDirective("ng:click", function(expression, element){
- return function(element){
+ return injectUpdateView(function($updateView, element){
var self = this;
element.bind('click', function(event){
self.$tryEval(expression, element);
- self.$root.$eval();
+ $updateView();
event.stopPropagation();
});
- };
+ });
});
@@ -471,14 +471,14 @@ angularDirective("ng:click", function(expression, element){
* server and reloading the current page).
*/
angularDirective("ng:submit", function(expression, element) {
- return function(element) {
+ return injectUpdateView(function($updateView, element) {
var self = this;
element.bind('submit', function(event) {
self.$tryEval(expression, element);
- self.$root.$eval();
+ $updateView();
event.preventDefault();
});
- };
+ });
});
diff --git a/src/scenario/Scenario.js b/src/scenario/Scenario.js
index 1723412e..a2926b5b 100644
--- a/src/scenario/Scenario.js
+++ b/src/scenario/Scenario.js
@@ -285,7 +285,7 @@ function browserTrigger(element, type) {
(function(fn){
var parentTrigger = fn.trigger;
fn.trigger = function(type) {
- if (/(click|change|keyup)/.test(type)) {
+ if (/(click|change|keydown)/.test(type)) {
return this.each(function(index, node) {
browserTrigger(node, type);
});
diff --git a/src/services.js b/src/services.js
index 91bd226d..1a2aada6 100644
--- a/src/services.js
+++ b/src/services.js
@@ -405,6 +405,62 @@ angularServiceInject('$exceptionHandler', function($log){
};
}, ['$log'], EAGER);
+/**
+ * @workInProgress
+ * @ngdoc service
+ * @name angular.service.$updateView
+ * @requires $browser
+ *
+ * @description
+ * Calling `$updateView` enqueues the eventual update of the view. (Update the DOM to reflect the
+ * model). The update is eventual, since there are often multiple updates to the model which may
+ * be deferred. The default update delayed is 25 ms. This means that the view lags the model by
+ * that time. (25ms is small enough that it is perceived as instantaneous by the user). The delay
+ * can be adjusted by setting the delay property of the service.
+ *
+ *
angular.service('$updateView').delay = 10
+ *
+ * The delay is there so that multiple updates to the model which occur sufficiently close
+ * together can be merged into a single update.
+ *
+ * You don't usually call '$updateView' directly since angular does it for you in most cases,
+ * but there are some cases when you need to call it.
+ *
+ * - `$updateView()` called automatically by angular:
+ * - Your Application Controllers: Your controller code is called by angular and hence
+ * angular is aware that you may have changed the model.
+ * - Your Services: Your service is usually called by your controller code, hence same rules
+ * apply.
+ * - May need to call `$updateView()` manually:
+ * - Widgets / Directives: If you listen to any DOM events or events on any third party
+ * libraries, then angular is not aware that you may have changed state state of the
+ * model, and hence you need to call '$updateView()' manually.
+ * - 'setTimeout'/'XHR': If you call 'setTimeout' (instead of {@link angular.service.$defer})
+ * or 'XHR' (instead of {@link angular.service.$xhr}) then you may be changing the model
+ * without angular knowledge and you may need to call '$updateView()' directly.
+ *
+ * NOTE: if you wish to update the view immediately (without delay), you can do so by calling
+ * {@link scope.$eval} at any time from your code:
+ *
scope.$root.$eval()
+ *
+ * In unit-test mode the update is instantaneous and synchronous to simplify writing tests.
+ *
+ */
+angularServiceInject('$updateView', extend(function factory($browser){
+ var rootScope = this;
+ var scheduled;
+ function update(){
+ scheduled = false;
+ rootScope.$eval();
+ }
+ return $browser.isMock ? update : function(){
+ if (!scheduled) {
+ scheduled = true;
+ $browser.defer(update, factory.delay);
+ }
+ };
+}, {delay:25}), ['$browser']);
+
/**
* @workInProgress
* @ngdoc service
@@ -815,7 +871,7 @@ angularServiceInject('$xhr.bulk', function($xhr, $error, $log){
*
* @param {function()} fn A function, who's execution should be deferred.
*/
-angularServiceInject('$defer', function($browser, $exceptionHandler) {
+angularServiceInject('$defer', function($browser, $exceptionHandler, $updateView) {
var scope = this;
return function(fn) {
@@ -825,11 +881,11 @@ angularServiceInject('$defer', function($browser, $exceptionHandler) {
} catch(e) {
$exceptionHandler(e);
} finally {
- scope.$eval();
+ $updateView();
}
});
};
-}, ['$browser', '$exceptionHandler']);
+}, ['$browser', '$exceptionHandler', '$updateView']);
/**
diff --git a/src/validators.js b/src/validators.js
index 3de98d61..8030c0d0 100644
--- a/src/validators.js
+++ b/src/validators.js
@@ -398,7 +398,7 @@ extend(angularValidator, {
$invalidWidgets.markValid(element);
}
element.data($$validate)();
- scope.$root.$eval();
+ scope.$service('$updateView')();
});
} else if (inputState.inFlight) {
// request in flight, mark widget invalid, but don't show it to user
diff --git a/src/widgets.js b/src/widgets.js
index 05979281..5ff4a28f 100644
--- a/src/widgets.js
+++ b/src/widgets.js
@@ -376,7 +376,7 @@ function optionsAccessor(scope, element) {
function noopAccessor() { return { get: noop, set: noop }; }
-var textWidget = inputWidget('keyup change', modelAccessor, valueAccessor, initWidgetValue(), true),
+var textWidget = inputWidget('keydown change', modelAccessor, valueAccessor, initWidgetValue(), true),
buttonWidget = inputWidget('click', noopAccessor, noopAccessor, noop),
INPUT_TYPE = {
'text': textWidget,
@@ -454,8 +454,8 @@ function radioInit(model, view, element) {
expect(binding('checkboxCount')).toBe('1');
});
*/
-function inputWidget(events, modelAccessor, viewAccessor, initFn, dirtyChecking) {
- return function(element) {
+function inputWidget(events, modelAccessor, viewAccessor, initFn, textBox) {
+ return injectService(['$updateView', '$defer'], function($updateView, $defer, element) {
var scope = this,
model = modelAccessor(scope, element),
view = viewAccessor(scope, element),
@@ -464,25 +464,25 @@ function inputWidget(events, modelAccessor, viewAccessor, initFn, dirtyChecking)
if (model) {
initFn.call(scope, model, view, element);
this.$eval(element.attr('ng:init')||'');
- // Don't register a handler if we are a button (noopAccessor) and there is no action
- if (action || modelAccessor !== noopAccessor) {
- element.bind(events, function (){
+ element.bind(events, function(event){
+ function handler(){
var value = view.get();
- if (!dirtyChecking || value != lastValue) {
+ if (!textBox || value != lastValue) {
model.set(value);
lastValue = model.get();
scope.$tryEval(action, element);
- scope.$root.$eval();
+ $updateView();
}
- });
- }
+ }
+ event.type == 'keydown' ? $defer(handler) : handler();
+ });
scope.$watch(model.get, function(value){
if (lastValue !== value) {
view.set(lastValue = value);
}
});
}
- };
+ });
}
function inputWidgetSelector(element){
diff --git a/test/BinderSpec.js b/test/BinderSpec.js
index 9515accb..33bcb091 100644
--- a/test/BinderSpec.js
+++ b/test/BinderSpec.js
@@ -248,7 +248,8 @@ describe('Binder', function(){
assertEquals('b', second.val());
first.val('ABC');
- browserTrigger(first, 'keyup');
+ browserTrigger(first, 'keydown');
+ c.scope.$service('$browser').defer.flush();
assertEquals(c.scope.items[0].x, 'ABC');
});
diff --git a/test/servicesSpec.js b/test/servicesSpec.js
index 6df83beb..8918f415 100644
--- a/test/servicesSpec.js
+++ b/test/servicesSpec.js
@@ -394,7 +394,7 @@ describe("service", function(){
it('should call eval even if an exception is thrown in callback', function() {
var eval = this.spyOn(scope, '$eval').andCallThrough();
- $defer(function() {throw "Test Error"});
+ $defer(function() {throw "Test Error";});
expect(eval).wasNotCalled();
$browser.defer.flush();
@@ -594,7 +594,7 @@ describe("service", function(){
$browser.defer.flush();
expect(eval).wasCalled();
- })
+ });
});
});
@@ -777,4 +777,57 @@ describe("service", function(){
expect(match[10]).toEqual('?book=moby');
});
});
+
+ describe('$updateView', function(){
+ var scope, browser, evalCount, $updateView;
+
+ beforeEach(function(){
+ browser = new MockBrowser();
+ // Pretend that you are real Browser so that we see the delays
+ browser.isMock = false;
+ browser.defer = jasmine.createSpy('defer');
+
+ scope = angular.scope(null, null, {$browser:browser});
+ $updateView = scope.$service('$updateView');
+ scope.$onEval(function(){ evalCount++; });
+ evalCount = 0;
+ });
+
+ it('should eval root scope after a delay', function(){
+ $updateView();
+ expect(evalCount).toEqual(0);
+ expect(browser.defer).toHaveBeenCalled();
+ expect(browser.defer.mostRecentCall.args[1]).toEqual(25);
+ browser.defer.mostRecentCall.args[0]();
+ expect(evalCount).toEqual(1);
+ });
+
+ it('should allow changing of delay time', function(){
+ var oldValue = angular.service('$updateView').delay;
+ angular.service('$updateView').delay = 50;
+ $updateView();
+ expect(evalCount).toEqual(0);
+ expect(browser.defer).toHaveBeenCalled();
+ expect(browser.defer.mostRecentCall.args[1]).toEqual(50);
+ angular.service('$updateView').delay = oldValue;
+ });
+
+ it('should ignore multiple requests for update', function(){
+ $updateView();
+ $updateView();
+ expect(evalCount).toEqual(0);
+ expect(browser.defer).toHaveBeenCalled();
+ expect(browser.defer.callCount).toEqual(1);
+ browser.defer.mostRecentCall.args[0]();
+ expect(evalCount).toEqual(1);
+ });
+
+ it('should update immediatelly in test/mock mode', function(){
+ scope = angular.scope();
+ scope.$onEval(function(){ evalCount++; });
+ expect(evalCount).toEqual(0);
+ scope.$service('$updateView')();
+ expect(evalCount).toEqual(1);
+ });
+ });
});
diff --git a/test/testabilityPatch.js b/test/testabilityPatch.js
index d389ae19..6cbf91e9 100644
--- a/test/testabilityPatch.js
+++ b/test/testabilityPatch.js
@@ -1,7 +1,7 @@
/**
* Here is the problem: http://bugs.jquery.com/ticket/7292
* basically jQuery treats change event on some browsers (IE) as a
- * special event and changes it form 'change' to 'click/keyup' and
+ * special event and changes it form 'change' to 'click/keydown' and
* few others. This horrible hack removes the special treatment
*/
_jQuery.event.special.change = undefined;
diff --git a/test/widgetsSpec.js b/test/widgetsSpec.js
index e56e895b..8dab4630 100644
--- a/test/widgetsSpec.js
+++ b/test/widgetsSpec.js
@@ -22,7 +22,7 @@ describe("widget", function(){
describe("input", function(){
describe("text", function(){
- it('should input-text auto init and handle keyup/change events', function(){
+ it('should input-text auto init and handle keydown/change events', function(){
compile('');
expect(scope.$get('name')).toEqual("Misko");
expect(scope.$get('count')).toEqual(0);
@@ -32,7 +32,10 @@ describe("widget", function(){
expect(element.val()).toEqual("Adam");
element.val('Shyam');
- browserTrigger(element, 'keyup');
+ browserTrigger(element, 'keydown');
+ // keydown event must be deferred
+ expect(scope.$get('name')).toEqual('Adam');
+ scope.$service('$browser').defer.flush();
expect(scope.$get('name')).toEqual('Shyam');
expect(scope.$get('count')).toEqual(1);
@@ -46,7 +49,7 @@ describe("widget", function(){
compile('');
expect(scope.name).toEqual("Misko");
expect(scope.count).toEqual(0);
- browserTrigger(element, 'keyup');
+ browserTrigger(element, 'keydown');
expect(scope.name).toEqual("Misko");
expect(scope.count).toEqual(0);
});
@@ -69,7 +72,7 @@ describe("widget", function(){
expect(element.val()).toEqual("x, y, z");
element.val('1, 2, 3');
- browserTrigger(element, 'keyup');
+ browserTrigger(element);
expect(scope.$get('list')).toEqual(['1', '2', '3']);
});
@@ -191,7 +194,7 @@ describe("widget", function(){
expect(element.attr('ng-validation-error')).toBeFalsy();
element.val('x');
- browserTrigger(element, 'keyup');
+ browserTrigger(element);
expect(element.hasClass('ng-validation-error')).toBeTruthy();
expect(element.attr('ng-validation-error')).toEqual('Not a number');
});
@@ -245,7 +248,7 @@ describe("widget", function(){
expect(element.attr('ng-validation-error')).toBeFalsy();
element.val('');
- browserTrigger(element, 'keyup');
+ browserTrigger(element);
expect(element.hasClass('ng-validation-error')).toBeTruthy();
expect(element.attr('ng-validation-error')).toEqual('Required');
});
@@ -270,7 +273,7 @@ describe("widget", function(){
expect(element.attr('ng-validation-error')).toEqual('Required');
element.val('abc');
- browserTrigger(element, 'keyup');
+ browserTrigger(element);
expect(element.hasClass('ng-validation-error')).toBeFalsy();
expect(element.attr('ng-validation-error')).toBeFalsy();
});
@@ -284,11 +287,11 @@ describe("widget", function(){
expect(element.val()).toEqual("Adam");
element.val('Shyam');
- browserTrigger(element, 'keyup');
+ browserTrigger(element);
expect(scope.$get('name')).toEqual('Shyam');
element.val('Kai');
- browserTrigger(element, 'change');
+ browserTrigger(element);
expect(scope.$get('name')).toEqual('Kai');
});
--
cgit v1.2.3