Skip to content

Commit

Permalink
feat(typeahead): add support for the focusFirst option
Browse files Browse the repository at this point in the history
Closes #748

Closes #856
  • Loading branch information
pkozlowski-opensource committed Oct 10, 2016
1 parent 9668832 commit e86277f
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 6 deletions.
4 changes: 3 additions & 1 deletion src/typeahead/typeahead-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ describe('ngb-typehead-config', () => {
it('should have sensible default values', () => {
const config = new NgbTypeaheadConfig();

expect(config.showHint).toBe(false);
expect(config.editable).toBeTruthy();
expect(config.focusFirst).toBeTruthy();
expect(config.showHint).toBeFalsy();
});
});
1 change: 1 addition & 0 deletions src/typeahead/typeahead-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ import {Injectable} from '@angular/core';
@Injectable()
export class NgbTypeaheadConfig {
editable = true;
focusFirst = true;
showHint = false;
}
7 changes: 5 additions & 2 deletions src/typeahead/typeahead-window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ export interface ResultTemplateContext {
`
})
export class NgbTypeaheadWindow {
activeIdx = 0;
/**
* An index of a match to be selected initially
*/
@Input() activeIdx = 0;

/**
* Typeahead match results to be displayed
Expand Down Expand Up @@ -73,7 +76,7 @@ export class NgbTypeaheadWindow {

next() { this.activeIdx = (this.activeIdx + 1) % this.results.length; }

prev() { this.activeIdx = (this.activeIdx === 0 ? this.results.length - 1 : this.activeIdx - 1); }
prev() { this.activeIdx = (this.activeIdx <= 0 ? this.results.length - 1 : this.activeIdx - 1); }

/**
* @internal
Expand Down
77 changes: 77 additions & 0 deletions src/typeahead/typeahead.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,83 @@ describe('ngb-typeahead', () => {
expectWindowResults(compiled, ['+ONE', 'ONE MORE']);
});

it('should not mark first result as active when focusFirst is false', () => {
const fixture = createTestComponent(`<input type="text" [ngbTypeahead]="find" [focusFirst]="false"/>`);
const compiled = fixture.nativeElement;

changeInput(compiled, 'o');
fixture.detectChanges();
expectWindowResults(compiled, ['one', 'one more']);
});

it('should properly make previous/next results active with down arrow keys when focusFirst is false', () => {
const fixture = createTestComponent(`<input type="text" [ngbTypeahead]="find" [focusFirst]="false"/>`);
const compiled = fixture.nativeElement;

changeInput(compiled, 'o');
fixture.detectChanges();
expectWindowResults(compiled, ['one', 'one more']);

// down
let event = createKeyDownEvent(Key.ArrowDown);
getDebugInput(fixture.debugElement).triggerEventHandler('keydown', event);
fixture.detectChanges();
expectWindowResults(compiled, ['+one', 'one more']);
expect(event.preventDefault).toHaveBeenCalled();

event = createKeyDownEvent(Key.ArrowDown);
getDebugInput(fixture.debugElement).triggerEventHandler('keydown', event);
fixture.detectChanges();
expectWindowResults(compiled, ['one', '+one more']);
expect(event.preventDefault).toHaveBeenCalled();

event = createKeyDownEvent(Key.ArrowDown);
getDebugInput(fixture.debugElement).triggerEventHandler('keydown', event);
fixture.detectChanges();
expectWindowResults(compiled, ['+one', 'one more']);
expect(event.preventDefault).toHaveBeenCalled();
});

it('should properly make previous/next results active with up arrow keys when focusFirst is false', () => {
const fixture = createTestComponent(`<input type="text" [ngbTypeahead]="find" [focusFirst]="false"/>`);
const compiled = fixture.nativeElement;

changeInput(compiled, 'o');
fixture.detectChanges();
expectWindowResults(compiled, ['one', 'one more']);

// up
let event = createKeyDownEvent(Key.ArrowUp);
getDebugInput(fixture.debugElement).triggerEventHandler('keydown', event);
fixture.detectChanges();
expectWindowResults(compiled, ['one', '+one more']);
expect(event.preventDefault).toHaveBeenCalled();

event = createKeyDownEvent(Key.ArrowUp);
getDebugInput(fixture.debugElement).triggerEventHandler('keydown', event);
fixture.detectChanges();
expectWindowResults(compiled, ['+one', 'one more']);
expect(event.preventDefault).toHaveBeenCalled();
});

it('should not select the result on TAB, close window and not write to the input when focusFirst is false', () => {
const fixture =
createTestComponent(`<input type="text" [(ngModel)]="model" [ngbTypeahead]="find" [focusFirst]="false"/>`);
const compiled = fixture.nativeElement;

changeInput(compiled, 'o');
fixture.detectChanges();
expect(getWindow(compiled)).not.toBeNull();

const event = createKeyDownEvent(Key.Tab);
getDebugInput(fixture.debugElement).triggerEventHandler('keydown', event);
fixture.detectChanges();
expect(getWindow(compiled)).toBeNull();
expectInputValue(compiled, 'o');
expect(fixture.componentInstance.model).toBe('o');
expect(event.preventDefault).toHaveBeenCalled();
});

it('should properly display results when an owning components using OnPush strategy', fakeAsync(() => {
const fixture = createOnPushTestComponent(`<input type="text" [(ngModel)]="model" [ngbTypeahead]="find"/>`);
const compiled = fixture.nativeElement;
Expand Down
19 changes: 16 additions & 3 deletions src/typeahead/typeahead.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import 'rxjs/add/operator/let';
import {positionElements} from '../util/positioning';
import {NgbTypeaheadWindow, ResultTemplateContext} from './typeahead-window';
import {PopupService} from '../util/popup';
import {toString} from '../util/util';
import {toString, isDefined} from '../util/util';
import {NgbTypeaheadConfig} from './typeahead-config';

enum Key {
Expand Down Expand Up @@ -87,6 +87,11 @@ export class NgbTypeahead implements ControlValueAccessor,
*/
@Input() editable: boolean;

/**
* A flag indicating if the first match should automatically be focused as you type.
*/
@Input() focusFirst: boolean;

/**
* A function to convert a given value into string to display in the input field
*/
Expand Down Expand Up @@ -126,6 +131,7 @@ export class NgbTypeahead implements ControlValueAccessor,
private _injector: Injector, componentFactoryResolver: ComponentFactoryResolver, config: NgbTypeaheadConfig,
ngZone: NgZone) {
this.editable = config.editable;
this.focusFirst = config.focusFirst;
this.showHint = config.showHint;

this._valueChanges = Observable.fromEvent(_elementRef.nativeElement, 'input', ($event) => $event.target.value);
Expand Down Expand Up @@ -198,7 +204,10 @@ export class NgbTypeahead implements ControlValueAccessor,
case Key.Enter:
case Key.Tab:
const result = this._windowRef.instance.getActive();
this._selectResult(result);
if (isDefined(result)) {
this._selectResult(result);
}
this._closePopup();
break;
case Key.Escape:
this.dismissPopup();
Expand All @@ -210,7 +219,7 @@ export class NgbTypeahead implements ControlValueAccessor,
private _openPopup() {
if (!this._windowRef) {
this._windowRef = this._popupService.open();
this._windowRef.instance.selectEvent.subscribe((result: any) => this._selectResult(result));
this._windowRef.instance.selectEvent.subscribe((result: any) => this._selectResultClosePopup(result));
}
}

Expand All @@ -227,7 +236,10 @@ export class NgbTypeahead implements ControlValueAccessor,
this.writeValue(result);
this._onChange(result);
}
}

private _selectResultClosePopup(result: any) {
this._selectResult(result);
this._closePopup();
}

Expand Down Expand Up @@ -260,6 +272,7 @@ export class NgbTypeahead implements ControlValueAccessor,
this._closePopup();
} else {
this._openPopup();
this._windowRef.instance.activeIdx = this.focusFirst ? 0 : -1;
this._windowRef.instance.results = results;
this._windowRef.instance.term = this._elementRef.nativeElement.value;
if (this.resultFormatter) {
Expand Down

3 comments on commit e86277f

@kjartanvalur
Copy link

Choose a reason for hiding this comment

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

Thanks for this @pkozlowski-opensource

But it would be really nice when you press arrow up on first item the focus would disappear instead of going to last item. It´s really the default behaviour in most cases I think. E.g. search box on http://www.imdb.com

What do you think?

@pkozlowski-opensource
Copy link
Member Author

Choose a reason for hiding this comment

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

But it would be really nice when you press arrow up on first item the focus would disappear instead of going to last item. It´s really the default behaviour in most cases I think. E.g. search box on http://www.imdb.com

What do you think?

Yeh, I considered it and was torn. I'm fine with both ways but it is true that many sites skip focus as you describe. Would you mind filling a new issue for this so I don't forget. I'm fine with changing it as you suggest.

@kjartanvalur
Copy link

Choose a reason for hiding this comment

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

Thanks man! #876

Please sign in to comment.