Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

feat(typeahead): Added -auto-highlight & -prevent-submission options #1363

Closed
wants to merge 2 commits into from
Closed

feat(typeahead): Added -auto-highlight & -prevent-submission options #1363

wants to merge 2 commits into from

Conversation

troch
Copy link

@troch troch commented Dec 5, 2013

Proposition for issues #908 and #1298

  • Added typeahead-auto-highlight option (default to true, meaning the first matching item is automatically activated)
  • Added typeahead-prevent-submission option for preventing or allowing default enter event on input field (default to true meaning form won't be submitted if control is in a form)
  • Improved key events handling for better UX (Setting autoHighlight to false causes popup to have no item selected at first and changes the way some events are handled)

Unit tests will follow in the next few days

@troch
Copy link
Author

troch commented Dec 6, 2013

What needs to be discussed:

  • Should auto-highlight be forced to true if is-editable is set to false?
  • If prevent-submission is set to false, should enter default event be prevented if is-editable is false (and no match has been selected or input is no in sync with model)?

@jeffwhelpley
Copy link

Any idea when this PR will get merged?

@tomchentw
Copy link
Contributor

I would like to support merging prevent-submission into this. I'm using a normal form (which is with action attribute) with typeahead input, and it would be great if we already select the item, press enter should auto submit the form. 👍

@dk8996
Copy link

dk8996 commented Dec 19, 2013

One thing I noticed there is a bit of a race condition when you set typeaheadWaitMs, a promise will be created. This will cause the typeahead to popup after you hit enter. Try setting the wait ms to something like 500 or 1000.

@dk8996
Copy link

dk8996 commented Dec 19, 2013

Here a patch that will fix the common case. I noticed there are other promises being called after this so we need to take care of that as well.

http://sharesend.com/ohv1cnly

@troch
Copy link
Author

troch commented Dec 19, 2013

Thanks for the patch. I guess this also needs to be added when tab is pressed.

@dk8996
Copy link

dk8996 commented Dec 19, 2013

Yeah the tab will also need this. I am still not sure what to do about the following promises since they don't have cancel promise.

$q.when(parserResult.source(scope, locals)).then(function(matches) {
....

@ghost ghost assigned bekos Jan 19, 2014
@pkozlowski-opensource
Copy link
Member

@troch there are 2 separate topics in this PR which makes it harder to review and decide upon. It is always better to open small, focused PRs - those will be dealt with much quicker.

Now, taking those 2 issues separately:

  • preventing submission on ENTER - this was discussed at length in v0.7 problem with typehead + ng-submit + Enter #1298 and based on this discussion we are not going to introduce it. What we could do though, is to refactor this directive to move more logic to the controller and then people can extend it
  • auto-highlight - this is more complex that this, as we need to take into account the editable property - when something is not editable we need to highlight an option IMO. But more importantly, it shouldn't be yet-another option but (eventually) an extension point on the controller level.

Going to close this one, feel free to open smaller, focused PRs or issue(s) so we can discuss things in more details.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants