Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/lib/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class MdAutocompleteTrigger implements AfterContentInit, OnDestroy {
private _panelOpen: boolean = false;

/** The subscription to positioning changes in the autocomplete panel. */
private _panelPositionSub: Subscription;
private _panelPositionSubscription: Subscription;

/** Manages active item in option list based on key events. */
private _keyManager: ActiveDescendantKeyManager;
Expand All @@ -62,12 +62,12 @@ export class MdAutocompleteTrigger implements AfterContentInit, OnDestroy {
@Optional() private _controlDir: NgControl, @Optional() private _dir: Dir) {}

ngAfterContentInit() {
this._keyManager = new ActiveDescendantKeyManager(this.autocomplete.options);
this._keyManager = new ActiveDescendantKeyManager(this.autocomplete.options).withWrap();
}

ngOnDestroy() {
if (this._panelPositionSub) {
this._panelPositionSub.unsubscribe();
if (this._panelPositionSubscription) {
this._panelPositionSubscription.unsubscribe();
}

this._destroyPanel();
Expand Down Expand Up @@ -225,7 +225,7 @@ export class MdAutocompleteTrigger implements AfterContentInit, OnDestroy {
* y-offset can be adjusted to match the new position.
*/
private _subscribeToPositionChanges(strategy: ConnectedPositionStrategy) {
this._panelPositionSub = strategy.onPositionChange.subscribe(change => {
this._panelPositionSubscription = strategy.onPositionChange.subscribe(change => {
this.autocomplete.positionY = change.connectionPair.originY === 'top' ? 'above' : 'below';
});
}
Expand All @@ -235,9 +235,9 @@ export class MdAutocompleteTrigger implements AfterContentInit, OnDestroy {
return this._element.nativeElement.getBoundingClientRect().width;
}

/** Reset active item to -1 so DOWN_ARROW event will activate the first option.*/
/** Reset active item to null so arrow events will activate the correct options.*/
private _resetActiveItem(): void {
this._keyManager.setActiveItem(-1);
this._keyManager.setActiveItem(null);
}

/**
Expand Down
51 changes: 35 additions & 16 deletions src/lib/autocomplete/autocomplete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import {MdInputModule} from '../input/index';
import {Dir, LayoutDirection} from '../core/rtl/dir';
import {FormControl, ReactiveFormsModule} from '@angular/forms';
import {Subscription} from 'rxjs/Subscription';
import {ENTER, DOWN_ARROW, SPACE} from '../core/keyboard/keycodes';
import {ENTER, DOWN_ARROW, SPACE, UP_ARROW} from '../core/keyboard/keycodes';
import {MdOption} from '../core/option/option';
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler';


describe('MdAutocomplete', () => {
let overlayContainerElement: HTMLElement;
Expand Down Expand Up @@ -294,6 +296,36 @@ describe('MdAutocomplete', () => {
});
}));

it('should set the active item to the last option when UP key is pressed', async(() => {
fixture.componentInstance.trigger.openPanel();
fixture.detectChanges();

const optionEls =
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;

const UP_ARROW_EVENT = new FakeKeyboardEvent(UP_ARROW) as KeyboardEvent;
fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);

fixture.whenStable().then(() => {
fixture.detectChanges();
expect(fixture.componentInstance.trigger.activeOption)
.toBe(fixture.componentInstance.options.last, 'Expected last option to be active.');
expect(optionEls[10].classList).toContain('md-active');
expect(optionEls[0].classList).not.toContain('md-active');

fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);

fixture.whenStable().then(() => {
fixture.detectChanges();
expect(fixture.componentInstance.trigger.activeOption)
.toBe(fixture.componentInstance.options.first,
'Expected first option to be active.');
expect(optionEls[0].classList).toContain('md-active');
expect(optionEls[10].classList).not.toContain('md-active');
});
});
}));

it('should set the active item properly after filtering', async(() => {
fixture.componentInstance.trigger.openPanel();
fixture.detectChanges();
Expand Down Expand Up @@ -532,7 +564,7 @@ describe('MdAutocomplete', () => {

it('should fall back to above position if panel cannot fit below', () => {
// Push the autocomplete trigger down so it won't have room to open "below"
input.style.top = '400px';
input.style.top = '600px';
input.style.position = 'relative';

fixture.componentInstance.trigger.openPanel();
Expand All @@ -551,7 +583,7 @@ describe('MdAutocomplete', () => {

it('should align panel properly when filtering in "above" position', () => {
// Push the autocomplete trigger down so it won't have room to open "below"
input.style.top = '400px';
input.style.top = '600px';
input.style.position = 'relative';

fixture.componentInstance.trigger.openPanel();
Expand Down Expand Up @@ -645,16 +677,3 @@ class FakeKeyboardEvent {
constructor(public keyCode: number) {}
preventDefault() {}
}

class FakeViewportRuler {
getViewportRect() {
return {
left: 0, top: 0, width: 500, height: 500, bottom: 500, right: 500
};
}

getViewportScrollPosition() {
return {top: 0, left: 0};
}
}

38 changes: 38 additions & 0 deletions src/lib/core/a11y/list-key-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ describe('Key managers', () => {
expect(keyManager.setActiveItem).not.toHaveBeenCalledWith(0);
});

it('should set first item active when down arrow pressed if no active item', () => {
keyManager.setActiveItem(null);
keyManager.onKeydown(DOWN_ARROW_EVENT);

expect(keyManager.activeItemIndex)
.toBe(0, 'Expected active item to be 0 after down key if active item was null.');
expect(keyManager.setActiveItem).toHaveBeenCalledWith(0);
expect(keyManager.setActiveItem).not.toHaveBeenCalledWith(1);
expect(keyManager.setActiveItem).not.toHaveBeenCalledWith(2);
});

it('should set previous items as active when up arrow is pressed', () => {
keyManager.onKeydown(DOWN_ARROW_EVENT);

Expand All @@ -101,6 +112,17 @@ describe('Key managers', () => {
expect(keyManager.setActiveItem).toHaveBeenCalledWith(0);
});

it('should do nothing when up arrow is pressed if no active item and not wrap', () => {
keyManager.setActiveItem(null);
keyManager.onKeydown(UP_ARROW_EVENT);

expect(keyManager.activeItemIndex)
.toBe(null, 'Expected nothing to happen if up arrow occurs and no active item.');
expect(keyManager.setActiveItem).not.toHaveBeenCalledWith(0);
expect(keyManager.setActiveItem).not.toHaveBeenCalledWith(1);
expect(keyManager.setActiveItem).not.toHaveBeenCalledWith(2);
});

it('should skip disabled items using arrow keys', () => {
itemList.items[1].disabled = true;

Expand Down Expand Up @@ -347,6 +369,22 @@ describe('Key managers', () => {
expect(keyManager.activeItemIndex).toBe(2, 'Expected active item to wrap to end.');
});

it('should set last item active when up arrow is pressed if no active item', () => {
keyManager.withWrap();
keyManager.setActiveItem(null);
keyManager.onKeydown(UP_ARROW_EVENT);

expect(keyManager.activeItemIndex)
.toBe(2, 'Expected last item to be active on up arrow if no active item.');
expect(keyManager.setActiveItem).not.toHaveBeenCalledWith(0);
expect(keyManager.setActiveItem).toHaveBeenCalledWith(2);

keyManager.onKeydown(DOWN_ARROW_EVENT);
expect(keyManager.activeItemIndex)
.toBe(0, 'Expected active item to be 0 after wrapping back to beginning.');
expect(keyManager.setActiveItem).toHaveBeenCalledWith(0);
});

});

});
Expand Down
5 changes: 3 additions & 2 deletions src/lib/core/a11y/list-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,13 @@ export class ListKeyManager<T extends CanDisable> {

/** Sets the active item to the next enabled item in the list. */
setNextItemActive(): void {
this._setActiveItemByDelta(1);
this._activeItemIndex === null ? this.setFirstItemActive() : this._setActiveItemByDelta(1);
}

/** Sets the active item to a previous enabled item in the list. */
setPreviousItemActive(): void {
this._setActiveItemByDelta(-1);
this._activeItemIndex === null && this._wrap ? this.setLastItemActive()
: this._setActiveItemByDelta(-1);
Copy link
Member

Choose a reason for hiding this comment

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

Need to handle the case when activeItemIndex is null and wrap is false

Copy link
Member

Choose a reason for hiding this comment

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

(with a unit test)

}

/**
Expand Down