feat(typeahead): handles min-length of 0

[zacronos]

Correctly handles min-length of 0 (i.e. will keep typeahead popup open even when all input has been deleted).

Still uses a default value of 1 for backward compatibility (when min-length is falsy but not === 0).

[dmitriz]

Maybe renaming to minLength would make it easier to read - similar to the attribute?

[zacronos]

@dmitriz, I could change that if it is demanded, however note that I did not change that variable name -- minSearch was the name used in the original code.

Cleaning up pre-existing issues in the original code seems a rather high burden to place on a 5-line PR (not counting specs) from 1.5 years ago (this is a re-PR, the original was posted in Jan of 2014).

[dmitriz]

Well it was used only used in 2 places, now would be 5.
Would clean up a bit but no worries.

[rvanbaalen]

Please note that one community members' notes and reviews aren't 'demands' on how you should do things but rather suggestions/tips. There should always be an open discussion 👍

---

This email was sent from my iPhone and therefore subject to typos and other inaccuracies.

On 30 apr. 2015, at 02:02, Joe Ibershoff notifications@github.com wrote:

In src/typeahead/typeahead.js:

@@ -41,7 +41,10 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
//SUPPORTED ATTRIBUTES (OPTIONS)

   //minimal no of characters that needs to be entered before typeahead kicks-in

•  var minSearch = originalScope.$eval(attrs.typeaheadMinLength) || 1;
  

•  var minSearch = originalScope.$eval(attrs.typeaheadMinLength);
  

  @dmitriz, I could change that if it is demanded, however note that I did not change that variable name -- minSearch was the name used in the original code.

Cleaning up pre-existing issues in the original code seems a rather high burden to place on a 5-line PR (not counting specs) from 1.5 years ago (this is a re-PR, the original was posted in Jan of 2014).

—
Reply to this email directly or view it on GitHub.

[zacronos]

@dmitriz, fair enough, you're right that this variable isn't really getting used elsewhere. I'll add that in.

[mattvague]

What's the status on this?

[chrisirhc]

LGTM