Merge lp:~huwshimi/lazr-js/autocomplete-enter-580404 into lp:lazr-js

[Huw Wilkins]

Pressing enter in an autocomplete field now submits the form when there are no suggestions.

Pressing enter in an autocomplete field now submits the form when there are no suggestions.

TO TEST:

View a bug page. Click the edit button or "Add tags". Type in a tag name. Press ENTER.

The form should submit and the tags will be saved. You used to have to press ENTER twice to get this to happen.

[Martin Pool (mbp)]

s/wat/want

What happens if this.get(QUERY) is actually null? Won't this error? Or maybe that can never happen if ._last_input_completed is false?

I see this code was already in the keydown method and I'm just a bit curious why it's there. Does it need to happen on keydown to stop the default behaviour happening?

[lp:~huwshimi/lazr-js/autocomplete-enter-580404]

204. By Huw Wilkins on 2011-02-11

Fixed comment grammar.

[Martin Pool (mbp)]

So if I've typed the start of a unique string, the first enter will complete the string and the second will submit?

Does this make it impossible to enter a prefix of an official tag as a new tag?

(I guess I should just run up the branch at this point.)

Thanks for addressing this, the current situation does grate.

[Benji York (benji)]

In testing the branch I couldn't trigger the autocomplete behavior at all, but the change did appear to work as advertised (i.e., a single enter caused the new tag value to be saved). Upon inspection I can't see any problem with the patch as given.

I would like to see a test for this behavior to prevent a regression, but I don't know what the state of our tests of browser behavior is currently so, unfortunately, that may not be practical.

review: Approve (code*)

[Brad Crittenden (bac)]

After setting up some official tags for a project I tested autocomplete and it worked pretty well. I did discover an issue while using a Webkit browser (Safari) that does not appear under Firefox:

Create an offical tag "reallylong".

Try to add a tag "long". When "long" is entered, autocomplete suggests "reallylong". To *not* accept the suggestion enter a space. Upon doing so in Safari the 'accept' and 'cancel' icons disappear and pressing return has no affect. The only way to recover is to press ESC and the entry is cancelled.

I don't know if this problem is introduced by your branch. Please either fix as part of your branch or open a new bug. I'm marking the branch Approved so you can do either at your discretion.

review: Approve (code)

[Huw Wilkins (huwshimi)]

@Martin
> I see this code was already in the keydown method and I'm just a bit curious why it's there. Does it need to happen on keydown to stop the default behaviour happening?

Where else would this code live? On keyup? I don't know why this code lives in keydown as opposed to keyup.

> Does this make it impossible to enter a prefix of an official tag as a new tag?

Yes, in a way. You can not press enter to save the form if you have any suggestions. In the prefix case you would have to enter a space in those circumstances. However, this is not a regression introduced by this branch.

@Brad
Will try and test that. I suspect it is a different issue.

[Huw Wilkins (huwshimi)]

@Brad
Actually I think I'm wrong it looks like it is an issue with my branch. Will fix.

[lp:~huwshimi/lazr-js/autocomplete-enter-580404]

205. By Huw Wilkins on 2011-03-01

Fixed error when the field contained a trailing space.

[Huw Wilkins (huwshimi)]

@Brad that is fixed. There was an error when there was a trailing space. My guess is that safari handles errors differently and was killing the javascript after the error.

=== modified file 'src-js/lazrjs/autocomplete/autocomplete.js'
--- src-js/lazrjs/autocomplete/autocomplete.js	2011-01-14 14:52:37 +0000
+++ src-js/lazrjs/autocomplete/autocomplete.js	2011-03-01 01:07:20 +0000
@@ -723,7 +723,10 @@
     _onInputKeydown: function(e) {
         // Is this one of our completion keys; Tab, or Enter?
         if (e.keyCode === TAB || e.keyCode === RETURN) {
-            if (this.get(QUERY) !== null && !this._last_input_was_completed) {
+            /* Check that the last string was not completed and that there are  
+               matching queries (we don't want to try and complete the input if 
+               there are no matches). */
+            if (this.get(QUERY) !== null && !this._last_input_was_completed && this.findMatches(this.get(QUERY).text).length !== 0) {
                 // The user has an active query in the input box.
                 this.completeInput();
                 // Keep the tab key from switching focus away from the input