Summary

Issue 3444: Autocomplete is too agressive

Review Request #6333 — Created Sept. 19, 2014 and submitted Dec. 2, 2014, 1:59 p.m.

Information

Owner

adrw.hong

Repository

Review Board

Branch

master

Bugs

3444

Depends On

Commit

f56c94f...

Reviewers

Groups

students

People

Description

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

Testing Done

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)

Issues

Description	From	Last Updated
Can you restore this line?	david	Sept. 20, 2014, 11:17 a.m.
Blank line between these.	chipx86	Sept. 20, 2014, 11:16 a.m.
Please add a space after the // and capitalize "Return"	david	Sept. 20, 2014, 11:17 a.m.
Since we're taking ownership over this code, we should start to move to our coding style where possible. In this …	chipx86	Sept. 20, 2014, 11:17 a.m.

Tool: Pyflakes
Ignored Files:
    reviewboard/static/lib/js/ui.autocomplete.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/lib/js/ui.autocomplete.js

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?

Description:

~		- Set autocomplete suggestions dropdown delay to 0.
	~	- Set autocomplete suggestions dropdown delay to 0.
	+	- Also happens when ENTER key is pressed.

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.

chipx86 Sept. 19, 2014, 9:32 a.m.

I also want to know what fields you tested with and how the behavior of the dropdown has changed, since it's not obvious from the changes themselves.

Summary:

-	Issue 3444: Autocomplete is too agressive https://code.google.com/p/reviewboard/issues/detail?id=3444
+	Issue 3444: Autocomplete is too agressive

Bugs:

+	3444

Description:

~		- Set autocomplete suggestions dropdown delay to 0.
~		- Also happens when ENTER key is pressed.
	~	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.

Testing Done:

		No unit tests currently exist for ui.autocomplete.js
	+	Reverted delay values.
	+	Tests: (2 users, John and Johndoe)
	+	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.
	+	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)

Diff:

Revision 2 (+2 -10)

Show changes

reviewboard/static/lib/js/ui.autocomplete.js

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:

		No unit tests currently exist for ui.autocomplete.js
		Reverted delay values.
~		Tests: (2 users, John and Johndoe)
~		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.
~		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)
	~	Tests: (2 users and 2 groups, John and Johndoe)
	~	Tested on Windows 8.1 Chrome 38 and Firefox 32; 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.
	~	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)

Groups:

-	reviewboard

Change Summary:

Added testing for IE11 and grammar stuff.

Testing Done:

		No unit tests currently exist for ui.autocomplete.js
		Reverted delay values.
		Tests: (2 users and 2 groups, John and Johndoe)
~		Tested on Windows 8.1 Chrome 38 and Firefox 32; 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.
	~	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)

Ship it!

```
Excellent testing documentation!
```

reviewboard/static/lib/js/ui.autocomplete.js (Diff revision 2)
Show all issues
```
Can you restore this line?
```
reviewboard/static/lib/js/ui.autocomplete.js (Diff revision 2)
Show all issues
```
Please add a space after the // and capitalize "Return"
```

reviewboard/static/lib/js/ui.autocomplete.js (Diff revision 2)
Show all issues
```
Blank line between these.
```

reviewboard/static/lib/js/ui.autocomplete.js (Diff revision 2)

Show all issues

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:

-	1b12bf6c69290705abbf62a2e31dc45016b93122
+	f56c94fc021c9ad523f18fb6ab671c6811df58e9

Diff:

Revision 3 (+12 -12)

Show changes

reviewboard/static/lib/js/ui.autocomplete.js

Tool: Pyflakes
Ignored Files:
    reviewboard/static/lib/js/ui.autocomplete.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/lib/js/ui.autocomplete.js

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (00bc85d)

Status: Re-opened

Change Summary:

Re-implemented fix for agressive autocomplete.

Description:

	+	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.
	+
	+	Work In Progress: not properly checking if input is contained in matches.
	+	i.e. does not return any matches that contain a string when matchContains flag is true.
	+	e.g. "hil" matches "philjo" but currently doesn't
	+	return that result.
	+
	+	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

Testing Done:

		No unit tests currently exist for ui.autocomplete.js
~		Reverted delay values.
~		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)
	~	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)

Diff:

Revision 4 (+33 -7)

Show changes

reviewboard/static/lib/js/ui.autocomplete.js

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:

		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.

-		Work In Progress: not properly checking if input is contained in matches.
-		i.e. does not return any matches that contain a string when matchContains flag is true.
-		e.g. "hil" matches "philjo" but currently doesn't
-		return that result.
-
		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

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (12eafa0)