Issue 3444: Autocomplete is too agressive

Review Request #6333 — Created Sept. 19, 2014 and submitted

Information

Review Board
master
f56c94f...

Reviewers

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)
Description From Last Updated

Can you restore this line?

daviddavid

Blank line between these.

chipx86chipx86

Please add a space after the // and capitalize "Return"

daviddavid

Since we're taking ownership over this code, we should start to move to our coding style where possible. In this …

chipx86chipx86
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
  2. 
      
AD
  1. 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?

  2. 
      
AD
chipx86
  1. 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.

    1. 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.

  2. 
      
AD
AD
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
  2. 
      
AD
AD
dkus
  1. 
      
  2. 
      
brennie
  1. Excellent testing documentation!

  2. 
      
david
  1. 
      
  2. Show all issues

    Can you restore this line?

  3. Show all issues

    Please add a space after the // and capitalize "Return"

  4. 
      
chipx86
  1. 
      
  2. Show all issues

    Blank line between these.

  3. 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) {
        ...
    }
    
  4. 
      
AD
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
  2. 
      
david
  1. Ship It!

  2. 
      
AD
AD
AD
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
  2. 
      
AD
david
  1. Ship It!
  2. 
      
AD
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.0.x (12eafa0)