From 92ed9c8c6357225dfc412eae013e176332101e6a Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 1 Mar 2018 01:05:39 +0100 Subject: [PATCH] feat(list-key-manager): accept item references in setActiveItem (#10029) * Adds an overload to `setActiveItem` to allow for an item to be set as active, rather than its index. This avoids awkward conversions in certain cases (see the chips and select changes). * Deprecates the `updateActiveItemIndex` method in favor of `updateActiveItem` which accepts both an index and an item. * Fixes the active item not being updated when it is set via `updateActiveItemIndex`. --- .../activedescendant-key-manager.ts | 19 +++++++-- src/cdk/a11y/key-manager/focus-key-manager.ts | 17 ++++++-- .../a11y/key-manager/list-key-manager.spec.ts | 23 +++++++++++ src/cdk/a11y/key-manager/list-key-manager.ts | 41 ++++++++++++++++--- src/lib/chips/chip-list.ts | 7 +--- src/lib/select/select.ts | 6 +-- 6 files changed, 91 insertions(+), 22 deletions(-) diff --git a/src/cdk/a11y/key-manager/activedescendant-key-manager.ts b/src/cdk/a11y/key-manager/activedescendant-key-manager.ts index b02bf6fbbd8..58bfcd7466a 100644 --- a/src/cdk/a11y/key-manager/activedescendant-key-manager.ts +++ b/src/cdk/a11y/key-manager/activedescendant-key-manager.ts @@ -24,11 +24,22 @@ export interface Highlightable extends ListKeyManagerOption { export class ActiveDescendantKeyManager extends ListKeyManager { /** - * This method sets the active item to the item at the specified index. - * It also adds active styles to the newly active item and removes active - * styles from the previously active item. + * Sets the active item to the item at the specified index and adds the + * active styles to the newly active item. Also removes active styles + * from the previously active item. + * @param index Index of the item to be set as active. */ - setActiveItem(index: number): void { + setActiveItem(index: number): void; + + /** + * Sets the active item to the item to the specified one and adds the + * active styles to the it. Also removes active styles from the + * previously active item. + * @param item Item to be set as active. + */ + setActiveItem(item: T): void; + + setActiveItem(index: any): void { if (this.activeItem) { this.activeItem.setInactiveStyles(); } diff --git a/src/cdk/a11y/key-manager/focus-key-manager.ts b/src/cdk/a11y/key-manager/focus-key-manager.ts index 84f52753b7b..5a44e0e353c 100644 --- a/src/cdk/a11y/key-manager/focus-key-manager.ts +++ b/src/cdk/a11y/key-manager/focus-key-manager.ts @@ -32,11 +32,20 @@ export class FocusKeyManager extends ListKeyManager { } /** - * This method sets the active item to the item at the specified index. - * It also adds focuses the newly active item. + * Sets the active item to the item at the specified + * index and focuses the newly active item. + * @param index Index of the item to be set as active. */ - setActiveItem(index: number): void { - super.setActiveItem(index); + setActiveItem(index: number): void; + + /** + * Sets the active item to the item that is specified and focuses it. + * @param item Item to be set as active. + */ + setActiveItem(item: T): void; + + setActiveItem(item: any): void { + super.setActiveItem(item); if (this.activeItem) { this.activeItem.focus(this._origin); diff --git a/src/cdk/a11y/key-manager/list-key-manager.spec.ts b/src/cdk/a11y/key-manager/list-key-manager.spec.ts index e2de0f5adb7..1188467fe95 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.spec.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.spec.ts @@ -349,6 +349,29 @@ describe('Key managers', () => { .toBe(1, `Expected activeItemIndex to be updated when setActiveItem() was called.`); }); + it('should be able to set the active item by reference', () => { + expect(keyManager.activeItemIndex) + .toBe(0, `Expected first item of the list to be active.`); + + keyManager.setActiveItem(itemList.items[2]); + expect(keyManager.activeItemIndex) + .toBe(2, `Expected activeItemIndex to be updated.`); + }); + + it('should be able to set the active item without emitting an event', () => { + const spy = jasmine.createSpy('change spy'); + const subscription = keyManager.change.subscribe(spy); + + expect(keyManager.activeItemIndex).toBe(0); + + keyManager.updateActiveItem(2); + + expect(keyManager.activeItemIndex).toBe(2); + expect(spy).not.toHaveBeenCalled(); + + subscription.unsubscribe(); + }); + it('should expose the active item correctly', () => { keyManager.onKeydown(fakeKeyEvents.downArrow); diff --git a/src/cdk/a11y/key-manager/list-key-manager.ts b/src/cdk/a11y/key-manager/list-key-manager.ts index b4dd1ac5c56..20d5333e7c9 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.ts @@ -162,14 +162,21 @@ export class ListKeyManager { * Sets the active item to the item at the index specified. * @param index The index of the item to be set as active. */ - setActiveItem(index: number): void { + setActiveItem(index: number): void; + + /** + * Sets the active item to the specified item. + * @param item The item to be set as active. + */ + setActiveItem(item: T): void; + + setActiveItem(item: any): void { const previousIndex = this._activeItemIndex; - this._activeItemIndex = index; - this._activeItem = this._items.toArray()[index]; + this.updateActiveItem(item); if (this._activeItemIndex !== previousIndex) { - this.change.next(index); + this.change.next(this._activeItemIndex); } } @@ -272,12 +279,34 @@ export class ListKeyManager { : this._setActiveItemByDelta(-1); } + /** + * Allows setting the active without any other effects. + * @param index Index of the item to be set as active. + */ + updateActiveItem(index: number): void; + + /** + * Allows setting the active item without any other effects. + * @param item Item to be set as active. + */ + updateActiveItem(item: T): void; + + updateActiveItem(item: any): void { + const itemArray = this._items.toArray(); + const index = typeof item === 'number' ? item : itemArray.indexOf(item); + + this._activeItemIndex = index; + this._activeItem = itemArray[index]; + } + /** * Allows setting of the activeItemIndex without any other effects. * @param index The new activeItemIndex. + * @deprecated Use `updateActiveItem` instead. + * @deletion-target 7.0.0 */ - updateActiveItemIndex(index: number) { - this._activeItemIndex = index; + updateActiveItemIndex(index: number): void { + this.updateActiveItem(index); } /** diff --git a/src/lib/chips/chip-list.ts b/src/lib/chips/chip-list.ts index 480e712deb9..35a7a61ad5f 100644 --- a/src/lib/chips/chip-list.ts +++ b/src/lib/chips/chip-list.ts @@ -567,14 +567,11 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo // Shift focus to the active item. Note that we shouldn't do this in multiple // mode, because we don't know what chip the user interacted with last. if (correspondingChip) { - const correspondingChipIndex = this.chips.toArray().indexOf(correspondingChip); - if (isUserInput) { - this._keyManager.setActiveItem(correspondingChipIndex); + this._keyManager.setActiveItem(correspondingChip); } else { - this._keyManager.updateActiveItemIndex(correspondingChipIndex); + this._keyManager.updateActiveItem(correspondingChip); } - } } } diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index ac42bbebb41..0a5a291af16 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -808,7 +808,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, // Shift focus to the active item. Note that we shouldn't do this in multiple // mode, because we don't know what option the user interacted with last. if (correspondingOption) { - this._keyManager.setActiveItem(this.options.toArray().indexOf(correspondingOption)); + this._keyManager.setActiveItem(correspondingOption); } } @@ -910,7 +910,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, this._selectionModel.toggle(option); this.stateChanges.next(); wasSelected ? option.deselect() : option.select(); - this._keyManager.setActiveItem(this._getOptionIndex(option)!); + this._keyManager.setActiveItem(option); this._sortValues(); } else { this._clearSelection(option.value == null ? undefined : option); @@ -976,7 +976,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, if (this.empty) { this._keyManager.setFirstItemActive(); } else { - this._keyManager.setActiveItem(this._getOptionIndex(this._selectionModel.selected[0])!); + this._keyManager.setActiveItem(this._selectionModel.selected[0]); } } }