Issue 3444: Autocomplete is too agressive
Review Request #6333 — Created Sept. 19, 2014 and submitted
Matches beginning of:
- in Group: matches beginning of group name or group alias.
- in People: matches beginning of username or first or last name.
- Autocomplete only replaces input with first dropdown item if input is a prefix for the item.
- COMMA key no longer triggers autocomplete; TAB and ENTER keys only work.
- Delete key (not backspace) no longer hides autocomplete dropdown.
- matches dropdown hide after a previous match has been made on the input box.
- Reverted delay values.
Issues reported:
https://code.google.com/p/reviewboard/issues/detail?id=3622
https://code.google.com/p/reviewboard/issues/detail?id=3629
https://code.google.com/p/reviewboard/issues/detail?id=3620
- No unit tests currently exist for ui.autocomplete.js
- Test input that matches beginning of group name or group alias. Making sure each group has a unique name and alias so it's only matching one field.
- Test input that matches beginning of username or first name or last name. Making sure each user has unique, username, first and last name so it's only matching one field.
- Header bar search tested and still works as expected. Matching one of group name, group alias, username, first name or last name.
- Tests: (2 users and 2 groups, John and Johndoe)
- Tested on Windows 8.1 Chrome 38, Firefox 32 and IE11; Linux Mint 17 (Ubuntu 14) Chromium 37 and Firefox 32
- type 'John' (wait for dropdown), type 'd', hit TAB/ENTER quickly, does not autocomplete since the dropdown hasn't updated due to delay.
- type 'Jo' (wait for dropdown), hit TAB/ENTER, autocompletes to John.
- type 'Johnd' (wait for dropdown', hit TAB/ENTER, autocompletes to Johndoe, remove 'ndoe', hit TAB/ENTER, autocompletes to John.
- Comma acts like regular input, does not trigger autocomplete. Starting to type another name after a comma behaves normally.
- TAB acts as normal when autocomplete dropdown not present (jumping through elements)
- ENTER act as normal when autocomplete dropdown not present (hitting OK below input field)
-
I realized afterwards that removing the delays is not a good idea so we discussed a few changes in person that might be good alternatives.
Referring to https://code.google.com/p/reviewboard/issues/detail?id=3444:
1. When user types "johnd", unselect the first option if it doesn't match, then when the server response comes back it will select "johndoe" (if such a user exists)
2. Don't have the autocomplete select the first match, make user select it. Results in a lot more keystrokes and not as nice. TAB key autocomplete will need fixing as well.
3. Any other options?
-
Make sure that the bug field is filled in. The summary itself shouldn't contain the bug information.
I'd like to see description filled out a bit more as well. Can you update it to be a series of sentences/paragraphs, rather than bullet point, that explains the reason for the change and what was done?
Testing Done must also be filled out. Even if there are no unit tests, there's a lot you can do to test that the functionality works (across browsers) and that nothing breaks.
Summary: |
|
||||
---|---|---|---|---|---|
Bugs: |
|
Description: |
|
|||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||
Diff: |
Revision 2 (+2 -10) |

-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/lib/js/ui.autocomplete.js Tool: Pyflakes Ignored Files: reviewboard/static/lib/js/ui.autocomplete.js
Change Summary:
Testing updates
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Groups: |
|
Change Summary:
Added testing for IE11 and grammar stuff.
Testing Done: |
|
---|
-
-
-
reviewboard/static/lib/js/ui.autocomplete.js (Diff revision 2) Please add a space after the
//
and capitalize "Return"
-
-
-
reviewboard/static/lib/js/ui.autocomplete.js (Diff revision 2) Since we're taking ownership over this code, we should start to move to our coding style where possible. In this case, that'd require that:
1) We place a space between
if
and(
2) Remove the spaces within the parens.
3) Use!==
for comparison (since it's stricter and faster).
4) Always use braces.So:
if (!selected || blahblah !== 0) { ... }
Change Summary:
Style fixes
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+12 -12) |

-
Tool: Pyflakes Ignored Files: reviewboard/static/lib/js/ui.autocomplete.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/lib/js/ui.autocomplete.js
Change Summary:
Re-implemented fix for agressive autocomplete.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+33 -7) |

-
Tool: Pyflakes Ignored Files: reviewboard/static/lib/js/ui.autocomplete.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/lib/js/ui.autocomplete.js
Description: |
|
---|