r2833 - autocomplete: code review
Author: joern.zaefferer
Date: Tue Jun 23 05:59:50 2009
New Revision: 2833
Modified:
branches/dev/autocomplete/ui/ui.autocomplete.js
Log:
autocomplete: code review
Modified: branches/dev/autocomplete/ui/ui.autocomplete.js
==============================================================================
--- branches/dev/autocomplete/ui/ui.autocomplete.js (original)
+++ branches/dev/autocomplete/ui/ui.autocomplete.js Tue Jun 23 05:59:50 2009
@@ -15,7 +15,7 @@
$.widget("ui.autocomplete", {
_init: function() {
- // TODO move these to instance properties?
+ // TODO move these to instance properties; overwrite setData to update
$.extend(this.options, {
delay: this.options.delay != undefined ? this.options.delay :
(this.options.url? this.options.ajaxDelay : this.options.localDelay),
max: this.options.max != undefined ? this.options.max :
(this.options.scroll? this.options.scrollMax : this.options.noScrollMax),
@@ -23,6 +23,7 @@
formatMatch: this.options.formatMatch || this.options.formatItem // if
the formatMatch option is not specified, then use formatItem for backwards
compatibility
});
+ // TODO clean up; try to remove these
var input = this.element[0],
options = this.options,
// Create $ object for input element
@@ -37,12 +38,15 @@
timeout,
blockSubmit,
lastKeyPressCode,
+ // TODO refactor select into its own widget? if not, try to merge with
widget methods
select = $.ui.autocomplete.select(options, input, selectCurrent,
config);
- if(options.result) {
+ // TODO is there a more generic way for callbacks?
+ if (options.result) {
$input.bind('result.autocomplete', options.result);
}
+ // TODO verify
// prevent form submit in opera when selecting with return key
$.browser.opera && $(input.form).bind("submit.autocomplete", function() {
if (blockSubmit) {
@@ -51,6 +55,7 @@
}
});
+ // TODO verify
// only opera doesn't trigger keydown multiple times while pressed,
others don't work with keypress at all
$input.bind(($.browser.opera ? "keypress" : "keydown")
+ ".autocomplete", function(event) {
// track last key pressed
@@ -93,8 +98,10 @@
}
break;
+ // TODO remove multiple case
// matches also semicolon
case options.multiple && $.trim(options.multipleSeparator) == "," &&
KEY.COMMA:
+ // TODO implement behaviour based on focus-option
case KEY.TAB:
case KEY.ENTER:
if( selectCurrent() ) {
@@ -147,27 +154,32 @@
if( typeof fn == "function" ) fn(result);
else $input.trigger("result.autocomplete", result && [result.data,
result.value]);
}
+ // TODO remove trimWords/multiple handling
$.each(trimWords($input.val()), function(i, value) {
request(value, findValueCallback, findValueCallback);
});
})
+ // TODO replace with public instance method
.bind("flushCache.autocomplete", function() {
cache.flush();
})
+ // TODO replace with setData override
.bind("setOptions.autocomplete", function() {
$.extend(options, arguments[1]);
// if we've updated the data, repopulate
if ( "data" in arguments[1] )
cache.populate();
})
+ // TODO replace with destroy override, if necessary at all
.bind("unautocomplete", function() {
+ // TODO rename select.unbind() to .remove(); thats what it does anyway
select.unbind();
- //$input.unbind(); << this would unbind all events. Bad news if we've
had another plugin applied to this element. AAP 03.12.09
$(input).unbind(".autocomplete");
$(input.form).unbind(".autocomplete");
});
// Private methods
+ // TODO move to instance method
function selectCurrent() {
var selected = select.selected();
if( !selected ) return false;
@@ -175,6 +187,7 @@
var v = selected.result;
previousValue = v;
+ // TODO remove
if ( options.multiple ) {
var words = trimWords($input.val());
if ( words.length > 1 ) {
@@ -189,6 +202,8 @@
return true;
};
+ // TODO verify usefullness of skipPrevCheck; also try to get rid of
obviously useless first argument
+ // where onChange is used as event handler directly, wrap it instead and
provide the right arguments
function onChange(crap, skipPrevCheck) {
if( lastKeyPressCode == KEY.DELETE ) {
select.hide();
@@ -202,6 +217,7 @@
previousValue = currentValue;
+ // TODO refactor; the request-response workflow is currently scattered
over way too many places
currentValue = lastWord(currentValue);
if ( currentValue.length >= options.minChars) {
$input.addClass(options.loadingClass);
@@ -214,6 +230,7 @@
}
};
+ // TODO without the multiple stuff, this shouldn't be necessary anymore
function trimWords(value) {
if ( !value ) {
return [""];
@@ -230,11 +247,13 @@
return result;
};
+ // TODO should be abl to remove this, too
function lastWord(value) {
var words = trimWords(value);
return words[words.length - 1];
};
+ // TODO simplify, get rid of comments, remove multiple stuff
// fills in the input box w/the first match (assumed to be the best
match)
// q: the term entered
// sValue: the first matching result
@@ -249,6 +268,7 @@
}
};
+ // TODO refactor: move to instance method, verify usefulness
function hideResults() {
clearTimeout(timeout);
timeout = setTimeout(hideResultsNow, 200);
@@ -259,6 +279,7 @@
select.hide();
clearTimeout(timeout);
stopLoading();
+ // TODO reimplement mustMatch
if (options.mustMatch) {
// call search and run callback
$input.autocomplete("search", function (result){
@@ -274,11 +295,13 @@
}
);
}
+ // TODO implement focus-option
if (wasVisible)
// position cursor at end of input field
$.ui.autocomplete.selection(input, input.value.length,
input.value.length);
};
+ // TODO refactor, move to source-related method(s)
function receiveData(q, data) {
if ( data && data.length && hasFocus ) {
stopLoading();
@@ -290,6 +313,7 @@
}
};
+ // TODO refactor, move to source-related method(s)
function request(term, success, failure) {
if (!options.matchCase)
term = term.toLowerCase();
@@ -307,10 +331,10 @@
extraParams[key] = typeof param == "function" ? param(term) : param;
});
+ // TODO move to default implemention for remote source
$.ajax({
// try to leverage ajaxQueue plugin to abort previous requests
mode: "abort",
- // limit abortion to this input
port: "autocomplete" + input.name,
dataType: options.dataType,
url: options.url,
@@ -326,12 +350,14 @@
});
}
+ // TODO refactor, in this case, most likely remove, to be replaced with
custom source option
else if (options.source && typeof options.source == 'function') {
var resultData = options.source(term);
var parsed = (options.parse) ? options.parse(resultData) : resultData;
cache.add(term, parsed);
success(term, parsed);
+ // TODO verify
} else {
// if we have a failure, we need to empty the list -- this prevents
the the [TAB] key from selecting the last successful match
select.emptyList();
@@ -339,6 +365,7 @@
}
};
+ // TODO move to parse option; replace the default implementation and
move this one to demos
function parse(data) {
var parsed = [];
var rows = data.split("\n");
@@ -362,30 +389,36 @@
}, //End _init
+ // TODO update to latest ui.core, most likely using _trigger instead
_propagate: function(n, event) {
$.ui.plugin.call(this, n, [event, this.ui()]);
return this.element.triggerHandler(n == 'autocomplete' ?
n : 'autocomplete'+n, [event, this.ui()], this.options[n]);
},
- // Public methods
+ // TODO remove/replace
ui: function(event) {
return {
options: this.options,
element: this.element
};
},
+ // TODO remove/replace: this should be an option
result: function(handler) {
return this.element.bind("result.autocomplete", handler);
},
+ // TODO verify API
search: function(handler) {
return this.element.trigger("search.autocomplete", [handler]);
},
+ // TODO refactor to just call the method instead of triggering an event
flushCache: function() {
return this.element.trigger("flushCache.autocomplete");
},
+ // TODO remove delegation to setOptions, implement it here directly
instead
setData: function(key, value){
return this.element.trigger("setOptions.autocomplete", [{ key: value }]);
},
+ // TODO verify correctness, replace trigger("unautocomplete)
destroy: function() {
this.element
.removeAttr('disabled')
@@ -398,6 +431,7 @@
.removeClass('ui-autocomplete-disabled');
this.disabled = false;
},
+ // TODO verify that disabled flag is respected
disable: function() {
this.element
.attr('disabled', true)
@@ -408,27 +442,38 @@
$.extend($.ui.autocomplete, {
defaults: {
+ // TODO remove these options; css classes are hardcoded
inputClass: "ui-autocomplete-input",
resultsClass: "ui-widget ui-widget-content ui-autocomplete-results",
loadingClass: "ui-autocomplete-loading",
minChars: 1,
ajaxDelay: 400,
localDelay: 10,
+ // TODO replace these three with cacheMatch option
matchCase: false,
matchSubset: true,
matchContains: false,
+ // TODO verify usefulness
cacheLength: 10,
scrollMax: 150,
noScrollMax: 10,
mustMatch: false,
+ // TODO replace
extraParams: {},
+ // TODO remove
selectFirst: true,
+ // TODO remove, replaced by parse/source
formatItem: function(row) { return row[0]; },
+ // TODO remove, replaced by parse/source
formatMatch: null,
+ // TODO replace with focus-option: "fill"
autoFill: false,
width: 0,
+ // TODO remove
multiple: false,
+ // TODO remove
multipleSeparator: ", ",
+ // TODO verify usefulness of arguments
highlight: function(value, term) {
return value.replace(new RegExp("(?![^&;]+;)(?!<[^<>]*)(" +
term.replace(/([\^\$\(\)\[\]\{\}\*\.\+\?\|\\])/gi, "\\$1")
+ ")(?![^<>]*>)(?![^&;]+;)", "gi"), "<strong>$1</strong>");
},
@@ -437,6 +482,7 @@
}
});
+// TODO replace cache related options and matching with cacheMatch option
(false = no caching, otherwise implementation to match terms)
$.ui.autocomplete.cache = function(options) {
var data = {};
@@ -573,11 +619,9 @@
};
};
+// TODO refactor, probably move to autocomplete instance methods
$.ui.autocomplete.select = function (options, input, select, config) {
var CLASSES = {
- //ACTIVE: "ui-state-active" << From the Css Framework
Docs : .ui-state-active: Class to be applied on mousedown to clickable
button-like elements.
- //Since this isnt a button element, but a hybrid 'menu item', use a
custom class.
- //See: http://jqueryui.pbwiki.com/Menu 2- Visual Design
DEFAULT: 'ui-autocomplete-state-default',
ACTIVE: 'ui-autocomplete-state-active'
};
@@ -592,11 +636,12 @@
// Create results
function init() {
+ // TODO just check if element is set instead
if (!needsInit) return;
element = $("<div/>")
.hide()
.addClass(options.resultsClass)
- //.css("position", "absolute") set this up in the css in case users want
to do something wonky with the positioning.
+ // TODO is there a need to append somewhere else? eg. autocomplete in
dialog
.appendTo(document.body);
list = $("<ul/>").appendTo(element).mouseover( function(event) {
@@ -623,6 +668,7 @@
needsInit = false;
}
+ // TODO replace with closest("li")?
function target(event) {
var element = event.target;
while(element && element.tagName != "LI")
@@ -637,14 +683,14 @@
listItems.slice(active, active + 1).removeClass(CLASSES.ACTIVE);
movePosition(step);
var activeItem = listItems.slice(active, active +
1).addClass(CLASSES.ACTIVE);
- if(options.scroll) {
+ if (options.scroll) {
var offset = 0;
listItems.slice(0, active).each(function() {
offset += this.offsetHeight;
});
- if((offset + activeItem[0].offsetHeight - list.scrollTop()) >
list[0].clientHeight) {
+ if ((offset + activeItem[0].offsetHeight - list.scrollTop()) >
list[0].clientHeight) {
list.scrollTop(offset + activeItem[0].offsetHeight -
list.innerHeight());
- } else if(offset < list.scrollTop()) {
+ } else if (offset < list.scrollTop()) {
list.scrollTop(offset);
}
}
@@ -730,6 +776,7 @@
current: function() {
return this.visible() && (listItems.filter("." + CLASSES.ACTIVE)[0] ||
options.selectFirst && listItems[0]);
},
+ // TODO refactor, too much going on here
show: function() {
var offset = $(input).offset();
element.css({
@@ -776,6 +823,7 @@
};
};
+// TODO this should be a generic utility; where else is it of interest?
$.ui.autocomplete.selection = function(field, start, end) {
if( field.createTextRange ){
var selRange = field.createTextRange();