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.

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

Add option changing TAB-key behaviour: Focus on the next tabindex, instead of selecting the top-suggestion #181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Darnokk
Copy link

@Darnokk Darnokk commented Feb 20, 2018

  • The default behaviour of the TAB-key is to select the top suggestion
  • We introduced a new option called 'tabAutocomplete' which is NOT mandatory and defaults to true.
  • If the option 'tabAutocomplete' is true, the TAB-key will select the top suggestion as usual
  • If the option 'tabAutocomplete' is NOT set, the TAB-key will select the top suggestion as usual
  • Only if you set the option 'tabAutocomplete' to false pressing the TAB key will change the focus on the html-element with the next tabindex.
  • Added option 'tabAutocomplete' to doc
  • Added two methods for tabAutocomplete = true resp. false to the unit-test typeahead_spec.js

@Darnokk
Copy link
Author

Darnokk commented Feb 20, 2018

This is a new version of the Pullreqeust: #77
Therefore i closed the old one #77

@drinu276
Copy link

Is there a set date when this will be merged into master?
Thanks a lot!

@jlbooker
Copy link
Contributor

There's no set date. We'll need some reviews and consensus from other contributors before we merge. Any @corejavascript/collaborators available to review this?

spyOn(this.view, 'autocomplete');
this.menu.getTopSelectable.andReturn($el);
this.input.trigger(eventName, payload);
expect(this.view.autocomplete).toHaveBeenCalledWith($el);
Copy link
Contributor

@jcrben jcrben Mar 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test would be more durable if we could read the resulting HTML and find that the text is there, as I'm doing with tests using enzyme... just because the view was called with the $el does not mean it actually autocompleted right?

Particularly with a loosely-maintained library like this, it's important to have reliable tests - I recommend writing an integration test https://github.com/corejavascript/typeahead.js/tree/master/test/integration

I think @jesperronn tried to revive integration tests but ran into a travis issue

@jcrben
Copy link
Contributor

jcrben commented Mar 28, 2018

Apparently my message didn't get posted - I got the @corejavascript/collaborators message - this library is used at work but it's legacy - I'm focused on react these days. Is there anyone else out there?

Might be time to open this up to more collaborators? I think @lenovouser is the owner? (https://github.com/orgs/corejavascript/teams/collaborators/members confirms maintainer role)

Cross-posted to https://github.com/orgs/corejavascript/teams/collaborators/discussions/1

@jlbooker
Copy link
Contributor

This is still in use at my previous employer, but is also legacy. I'm focused on React now too (thought about recreating this in React, but haven't had the time).

I own the NPM package and can still handle build/release tasks, just no time to code review PRs. I'd be happy to have bring in more collaborators.

@rdlugosz
Copy link

Thanks for this PR - works great.

@cjolly
Copy link

cjolly commented Jul 26, 2018

We would definitely use this feature. Our app is heavy on data entry, and has a bunch of fields on a single form, a few of which we use typeahead. Our users often want to type something into a field that's a close match, but they want to only keep the data they've typed in. The ability to tab to the next field without selecting the first match would be great!

@jlbooker
Copy link
Contributor

HI @Darnokk or anyone else still interested in this PR.. I was looking at merging this tonight, but it looks like it has a couple of merge conflicts. Could anyone still interested in this take the time to resolve those so this can be merged cleanly?

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

Successfully merging this pull request may close these issues.

6 participants