Skip to content

Commit

Permalink
change to keydown from keyup; add delayed $updateView
Browse files Browse the repository at this point in the history
 - 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
  • Loading branch information
mhevery authored and IgorMinar committed Jan 7, 2011
1 parent 16086aa commit 47c454a
Show file tree
Hide file tree
Showing 18 changed files with 204 additions and 51 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ build/
angularjs.netrc
jstd.log
.DS_Store
regression/temp.html
regression/temp*.html
performance/temp*.html
.idea/workspace.xml
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion docs/collect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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){
Expand Down Expand Up @@ -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.';
}
Expand Down
8 changes: 4 additions & 4 deletions docs/docs.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
SyntaxHighlighter['defaults'].toolbar = false;

DocsController.$inject = ['$location', '$browser', '$window'];
function DocsController($location, $browser, $window) {
this.pages = NG_PAGES;
Expand Down Expand Up @@ -38,10 +36,12 @@ function DocsController($location, $browser, $window) {
return "mailto:[email protected]?" +
"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();
});
});

SyntaxHighlighter['defaults'].toolbar = false;
6 changes: 5 additions & 1 deletion docs/service.template
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,23 @@
{{/requires}}
</ul>

{{#method.length}}
<h2>Methods</h2>
<ul>
{{#method}}
<li><tt>{{shortName}}</tt>: {{{description}}}</li>
<li><tt>{{shortName}}()</tt>: {{{description}}}</li>
{{/method}}
</ul>
{{/method.length}}

{{#property.length}}
<h2>Properties</h2>
<ul>
{{#property}}
<li><tt>{{name}}:{{#type}}{{type}}{{/type}}</tt>{{#description}}: {{{description}}}{{/description}}</li>
{{/property}}
</ul>
{{/property.length}}

{{#example}}
<h2>Example</h2>
Expand Down
19 changes: 19 additions & 0 deletions perf/noangular.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
<html xmlns:ng="http://angularjs.org">
<head>
<script>
function el(id) {
return document.getElementById(id);
}
function update() {
el("output").innerHTML = el("input").value;
}
</script>
</head>
<body>
Your name: <input id="input" type="text" value="World"
onkeydown="setTimeout(update,0)"/>
<hr/>
Hello <span id="output">{{yourname}}</span>!
</body>
</html>
15 changes: 7 additions & 8 deletions src/Browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(); });
};

/**
Expand Down Expand Up @@ -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);
};

//////////////////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions src/Compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ Template.prototype = {
*/
function retrieveScope(element) {
var scope;
element = jqLite(element);
while (element && !(scope = element.data($$scope))) {
element = element.parent();
}
Expand Down
10 changes: 9 additions & 1 deletion src/Injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,12 @@ function createInjector(providerScope, providers, cache) {
}
return returnValue;
};
}
}

function injectService(services, fn) {
return extend(fn, {$inject:services});;
}

function injectUpdateView(fn) {
return injectService(['$updateView'], fn);
}
12 changes: 6 additions & 6 deletions src/directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
};
});
});


Expand Down Expand Up @@ -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();
});
};
});
});


Expand Down
2 changes: 1 addition & 1 deletion src/scenario/Scenario.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
62 changes: 59 additions & 3 deletions src/services.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <pre>angular.service('$updateView').delay = 10</pre>
*
* 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:
* <pre>scope.$root.$eval()</pre>
*
* 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
Expand Down Expand Up @@ -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) {
Expand All @@ -825,11 +881,11 @@ angularServiceInject('$defer', function($browser, $exceptionHandler) {
} catch(e) {
$exceptionHandler(e);
} finally {
scope.$eval();
$updateView();
}
});
};
}, ['$browser', '$exceptionHandler']);
}, ['$browser', '$exceptionHandler', '$updateView']);


/**
Expand Down
2 changes: 1 addition & 1 deletion src/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 11 additions & 11 deletions src/widgets.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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){
Expand Down
3 changes: 2 additions & 1 deletion test/BinderSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down
Loading

0 comments on commit 47c454a

Please sign in to comment.