[webkit-reviews] review denied: [Bug 17374] Inspector should support tab completion while editing CSS : [Attachment 56082] Autocomplete CSS properties

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Nikita Vasilev
<me+webkit at elv1s.ru>'s request for review:
Bug 17374: Inspector should support tab completion while editing CSS
https://bugs.webkit.org/show_bug.cgi?id=17374

Attachment 56082: Autocomplete CSS properties
https://bugs.webkit.org/attachment.cgi?id=56082&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
WebCore/inspector/front-end/CSS.js:13
 +		    
Remove this empty line.

WebCore/inspector/front-end/CSS.js:22
 +	return this.filter(function(property) {
What is filter?

WebCore/inspector/front-end/StylesSidebarPane.js:1409
 +	    } else if (/[A-Z-]/.test(event.character)) {
Shouldn't this be /[a-zA-Z-]/ or /[a-z-]/i?

WebCore/inspector/front-end/StylesSidebarPane.js:1410
 +		setTimeout(function() {
You should name this function and not inline it in the setTimeout call since it
is so long.

There is another approch this could take, more like TextPrompt. TextPrompt
calls autoCompleteSoon after some key events (it dosen't care if it was a
character either), then autoCompleteSoon schedules a timeout that checks the
text content.

It is similar to what you are doing here, but you schedule a new timeout after
each keydown. TextPrompt coalesces them, so if you type fast it only
autocompletes once.

Why can't this share code with TextPrompt or just use it?

r- because you should cancel previous timeouts at a minimum, for fast typers.