Skip to content

Commit

Permalink
feat(typeahead) stop selecting first result when displaying matches
Browse files Browse the repository at this point in the history
Switches from always setting the first result to be the active one, and
instead initially displays the result list with no active item.
After the pressing down/up keys the first/last item will be selected.

Also cleans up the keyboard interaction when alternating between
navigating between matches with arrow keys and typing additional
characters, by keeping the previous match selected if it continues to be
a match with the new results, instead of the previous behavior of
resetting to the first item selected.

Closes valor-software#3965
  • Loading branch information
maurizi committed Mar 13, 2018
1 parent a3de0a4 commit a2b1d7b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 30 deletions.
48 changes: 29 additions & 19 deletions src/spec/typeahead-container.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +82,28 @@ describe('Component: TypeaheadContainer', () => {
expect(ms[1].nativeElement.innerHTML).toBe('<strong>fo</strong>od');
});

it('should set the "active" class on the first match', () => {
expect(matches[0].classList.contains('active')).toBeTruthy();
});

it('should not set the "active" class on other matches', () => {
it('should not set the "active" class on any matches', () => {
expect(matches[0].classList.contains('active')).toBeFalsy();
expect(matches[1].classList.contains('active')).toBeFalsy();
});
});

describe('nextActiveMatch', () => {
it('should select the next match', () => {
it('should select the first match on first use', () => {
component.nextActiveMatch();

expect(component.isActive(component.matches[0])).toBeTruthy();
});

it('should select the next match after the first', () => {
component.nextActiveMatch();
component.nextActiveMatch();

expect(component.isActive(component.matches[1])).toBeTruthy();
});

it('should select the first match again, when triggered twice', () => {
it('should select the first match again, when triggered three times', () => {
component.nextActiveMatch();
component.nextActiveMatch();
component.nextActiveMatch();

Expand Down Expand Up @@ -162,8 +167,9 @@ describe('Component: TypeaheadContainer', () => {
expect(im[1].nativeElement.innerHTML).toBe('<strong>a</strong>pple');
});

it('should set the "active" class on the first item match', () => {
expect(itemMatches[0].classList.contains('active')).toBeTruthy();
it('should not set the "active" class on any matches', () => {
expect(itemMatches[0].classList.contains('active')).toBeFalsy();
expect(itemMatches[1].classList.contains('active')).toBeFalsy();
});

it('should not set the "active" class on the header match', () => {
Expand All @@ -176,13 +182,14 @@ describe('Component: TypeaheadContainer', () => {
});

describe('nextActiveMatch', () => {
it('should select the next item match', () => {
it('should select the first item match', () => {
component.nextActiveMatch();

expect(component.isActive(component.matches[2])).toBeTruthy();
expect(component.isActive(component.matches[1])).toBeTruthy();
});

it('should skip the header match, when triggered twice', () => {
it('should skip the header match, when triggered three times', () => {
component.nextActiveMatch();
component.nextActiveMatch();
component.nextActiveMatch();

Expand Down Expand Up @@ -300,17 +307,20 @@ describe('Component: TypeaheadContainer', () => {
expect(itemMatches[1].children[0].children[0].innerHTML).toBe('<strong>a</strong>pple');
});

it('should set the \"active\" class on the first item match', () => {
expect(itemMatches[0].classList.contains('active')).toBeTruthy();
it('should not set the \"active\" class on any matches', () => {
for (let i = 0; i < 9; i++) {
expect(itemMatches[i].classList.contains('active')).toBeFalsy();
}
});
});

describe('nextActiveMatch', () => {
it('should select the next item match', () => {
it('should select the first item match', () => {
component.nextActiveMatch();
expect(component.isActive(component.matches[1])).toBeTruthy();
expect(component.isActive(component.matches[0])).toBeTruthy();
});
it('should select the next item match and scroll', fakeAsync(() => {
component.nextActiveMatch();
component.nextActiveMatch();
component.nextActiveMatch();
fixture.detectChanges();
Expand All @@ -319,14 +329,14 @@ describe('Component: TypeaheadContainer', () => {
expect(containingElementScrollable[0].scrollTop).toBe(0);
}));
it('should select the last item match and scroll', () => {
for (let i = 0; i < 8; i++) {
for (let i = 0; i < 9; i++) {
component.nextActiveMatch();
}
expect(component.isActive(component.matches[10])).toBeTruthy();
});

it('should select the first item match and scroll to top', () => {
for (let i = 0; i < 9; i++) {
for (let i = 0; i < 10; i++) {
component.nextActiveMatch();
}
expect(component.isActive(component.matches[0])).toBeTruthy();
Expand All @@ -346,7 +356,7 @@ describe('Component: TypeaheadContainer', () => {
component.nextActiveMatch();
component.nextActiveMatch();
component.prevActiveMatch();
expect(component.isActive(component.matches[2])).toBeTruthy();
expect(component.isActive(component.matches[1])).toBeTruthy();
});
});

Expand Down
14 changes: 9 additions & 5 deletions src/typeahead/typeahead-container.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ export class TypeaheadContainerComponent {
});
}

if (this._matches.length > 0) {
this._active = this._matches[0];
if (this._active.isHeader()) {
this.nextActiveMatch();
if (this._active) {
const match = this._matches.find(m => m.value === this._active.value);
if (match) {
this.selectActive(match);
} else {
this._active = undefined;
}
}
}
Expand All @@ -101,7 +103,9 @@ export class TypeaheadContainerComponent {
}

selectActiveMatch(): void {
this.selectMatch(this._active);
if (this._active) {
this.selectMatch(this._active);
}
}

prevActiveMatch(): void {
Expand Down
15 changes: 9 additions & 6 deletions src/typeahead/typeahead.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,19 +251,22 @@ export class TypeaheadDirective implements OnInit, OnDestroy {
return;
}

// if an item is visible - prevent form submission
if (e.keyCode === 13) {
// if an item is active - prevent form submission
if (this._container.active && e.keyCode === 13) {
e.preventDefault();

return;
}

// if an item is visible - don't change focus
// if an item is active - don't change focus
if (e.keyCode === 9) {
e.preventDefault();
this._container.selectActiveMatch();
if (this._container.active) {
e.preventDefault();
this._container.selectActiveMatch();

return;
return;
}
this.hide();
}
}

Expand Down

0 comments on commit a2b1d7b

Please sign in to comment.