Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Typeahead MinLength=0 #412

Merged
merged 4 commits into from
Apr 21, 2016
Merged

Typeahead MinLength=0 #412

merged 4 commits into from
Apr 21, 2016

Conversation

rbaarsma
Copy link
Contributor

Made it possible to use the TypeaheadMinLength feature with value=0. This will show the entire list of options as soon as the 'focus' event is fired and also when you remove all text.

Would add feature for #413

@rbaarsma rbaarsma mentioned this pull request Apr 14, 2016
Closed
@rbaarsma
Copy link
Contributor Author

rbaarsma commented Apr 15, 2016 •
edited
Loading

also fixes duplicate issue #187

valorkin
valorkin reviewed Apr 15, 2016
View reviewed changes
components/typeahead/typeahead.directive.ts
@@ -266,6 +282,13 @@ export class Typeahead implements OnInit {
return;
}

if (!this.cd.model) {
for (let i = 0; i < this.typeahead.length; i++) {

This comment was marked as off-topic.

Sign in to view

@valorkin
Copy link
Member

@rbaarsma looks good to me!
please rebase on latest and apply\validate my comments :)

@valorkin
Copy link
Member

@rbaarsma I have reread your issue about showing all options,
than special -1 value should be added to typeaheadOptionsLimit
treated in code and mentioned in doc

@rbaarsma
Copy link
Contributor Author

rbaarsma commented Apr 20, 2016 •
edited
Loading

@valorkin I have implemented your comments. I'm not sure what you mean with the special -1 value, but what I understand from #187 is that typeaheadMinLength 0 was a feature of the angular1 bootstrap as well. Makes sense to keep it the same and min-length of 0 makes sense. I'd leave it as it is.

As for rebasing it on latest. Unfortunately I'm pretty new to working with pull requests with open source projects and I don't want to do anything wrong. I did update my branch to the latest version. I thought you could simply merge it like this, but if I need to do something else can you please tell me what to do exactly to rebase :)?

@valorkin
Copy link
Member

ok, looks good to me!
please describe this option in doc
PS: I will squash on merge
PSS: God bless @github for this option :D

@rbaarsma
Copy link
Contributor Author

@valorkin updated readme. I think you can merge it now 😄

@valorkin valorkin merged commit f1c1909 into valor-software:master Apr 21, 2016
@valorkin
Copy link
Member

Great! Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rbaarsma @valorkin