From f99af6d044c9412fba501aab5c6e11b16a52a09b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 5 Dec 2022 18:30:47 +0100 Subject: [PATCH] fix(cdk/listbox): set initial focus to selected option (#26174) Fixes that the listbox was setting the initial focus on a deselected option when a selected one was available. **Note:** this fix is a bit more convoluted that it needs to be. E.g. ideally we would just focus the selected option when the listbox receives focus. We can't do that, because the listbox supports two different focus management modes: focus and `aria-activedescendant`. The former doesn't allow tabbing to the listbox. Fixes #25833. --- src/cdk/listbox/listbox.spec.ts | 34 +++++++++++++++ src/cdk/listbox/listbox.ts | 63 +++++++++++++++++++++------ tools/public_api_guard/cdk/listbox.md | 2 + 3 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/cdk/listbox/listbox.spec.ts b/src/cdk/listbox/listbox.spec.ts index c9ac18836a01..3a75f90bdb2d 100644 --- a/src/cdk/listbox/listbox.spec.ts +++ b/src/cdk/listbox/listbox.spec.ts @@ -706,6 +706,38 @@ describe('CdkOption and CdkListbox', () => { expect(options[options.length - 1].isActive()).toBeTrue(); }); + + it('should focus the selected option when the listbox is focused', () => { + const {testComponent, fixture, listbox, listboxEl, options} = + setupComponent(ListboxWithOptions); + testComponent.selectedValue = 'peach'; + fixture.detectChanges(); + listbox.focus(); + fixture.detectChanges(); + + expect(options[3].isActive()).toBeTrue(); + + dispatchKeyboardEvent(listboxEl, 'keydown', UP_ARROW); + fixture.detectChanges(); + + expect(options[2].isActive()).toBeTrue(); + }); + + it('should not move focus to the selected option while the user is navigating', () => { + const {testComponent, fixture, listbox, listboxEl, options} = + setupComponent(ListboxWithOptions); + listbox.focus(); + fixture.detectChanges(); + expect(options[0].isActive()).toBeTrue(); + + dispatchKeyboardEvent(listboxEl, 'keydown', DOWN_ARROW); + fixture.detectChanges(); + expect(options[1].isActive()).toBeTrue(); + + testComponent.selectedValue = 'peach'; + fixture.detectChanges(); + expect(options[1].isActive()).toBeTrue(); + }); }); describe('with roving tabindex', () => { @@ -909,6 +941,7 @@ describe('CdkOption and CdkListbox', () => { [cdkListboxOrientation]="orientation" [cdkListboxNavigationWrapDisabled]="!navigationWraps" [cdkListboxNavigatesDisabledOptions]="!navigationSkipsDisabled" + [cdkListboxValue]="selectedValue" (cdkListboxValueChange)="onSelectionChange($event)">
) { this.changedOption = event.option; diff --git a/src/cdk/listbox/listbox.ts b/src/cdk/listbox/listbox.ts index 38575e067d5e..197f8e34f8c7 100644 --- a/src/cdk/listbox/listbox.ts +++ b/src/cdk/listbox/listbox.ts @@ -146,9 +146,6 @@ export class CdkOption implements ListKeyManagerOption, Highlightab /** Emits when the option is clicked. */ readonly _clicked = new Subject(); - /** Whether the option is currently active. */ - private _active = false; - ngOnDestroy() { this.destroyed.next(); this.destroyed.complete(); @@ -161,7 +158,7 @@ export class CdkOption implements ListKeyManagerOption, Highlightab /** Whether this option is active. */ isActive() { - return this._active; + return this.listbox.isActive(this); } /** Toggle the selected state of this option. */ @@ -190,20 +187,16 @@ export class CdkOption implements ListKeyManagerOption, Highlightab } /** - * Set the option as active. + * No-op implemented as a part of `Highlightable`. * @docs-private */ - setActiveStyles() { - this._active = true; - } + setActiveStyles() {} /** - * Set the option as inactive. + * No-op implemented as a part of `Highlightable`. * @docs-private */ - setInactiveStyles() { - this._active = false; - } + setInactiveStyles() {} /** Handle focus events on the option. */ protected _handleFocus() { @@ -240,6 +233,7 @@ export class CdkOption implements ListKeyManagerOption, Highlightab '(focus)': '_handleFocus()', '(keydown)': '_handleKeydown($event)', '(focusout)': '_handleFocusOut($event)', + '(focusin)': '_handleFocusIn()', }, providers: [ { @@ -419,6 +413,9 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con /** A predicate that does not skip any options. */ private readonly _skipNonePredicate = () => false; + /** Whether the listbox currently has focus. */ + private _hasFocus = false; + ngAfterContentInit() { if (typeof ngDevMode === 'undefined' || ngDevMode) { this._verifyNoOptionValueCollisions(); @@ -526,6 +523,14 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con return this.isValueSelected(option.value); } + /** + * Get whether the given option is active. + * @param option The option to get the active state of + */ + isActive(option: CdkOption): boolean { + return !!(this.listKeyManager?.activeItem === option); + } + /** * Get whether the given value is selected. * @param value The value to get the selected state of @@ -653,7 +658,12 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con /** Called when the listbox receives focus. */ protected _handleFocus() { if (!this.useActiveDescendant) { - this.listKeyManager.setNextItemActive(); + if (this.selectionModel.selected.length > 0) { + this._setNextFocusToSelectedOption(); + } else { + this.listKeyManager.setNextItemActive(); + } + this._focusActiveOption(); } } @@ -759,6 +769,13 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con } } + /** Called when a focus moves into the listbox. */ + protected _handleFocusIn() { + // Note that we use a `focusin` handler for this instead of the existing `focus` handler, + // because focus won't land on the listbox if `useActiveDescendant` is enabled. + this._hasFocus = true; + } + /** * Called when the focus leaves an element in the listbox. * @param event The focusout event @@ -767,6 +784,8 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con const otherElement = event.relatedTarget as Element; if (this.element !== otherElement && !this.element.contains(otherElement)) { this._onTouched(); + this._hasFocus = false; + this._setNextFocusToSelectedOption(); } } @@ -800,6 +819,10 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con this.listKeyManager.withHorizontalOrientation(this._dir?.value || 'ltr'); } + if (this.selectionModel.selected.length) { + Promise.resolve().then(() => this._setNextFocusToSelectedOption()); + } + this.listKeyManager.change.subscribe(() => this._focusActiveOption()); } @@ -820,6 +843,20 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con this.selectionModel.clear(false); } this.selectionModel.setSelection(...this._coerceValue(value)); + + if (!this._hasFocus) { + this._setNextFocusToSelectedOption(); + } + } + + /** Sets the first selected option as first in the keyboard focus order. */ + private _setNextFocusToSelectedOption() { + // Null check the options since they only get defined after `ngAfterContentInit`. + const selected = this.options?.find(option => option.isSelected()); + + if (selected) { + this.listKeyManager.updateActiveItem(selected); + } } /** Update the internal value of the listbox based on the selection model. */ diff --git a/tools/public_api_guard/cdk/listbox.md b/tools/public_api_guard/cdk/listbox.md index b143c78cf4a0..556f66df3f73 100644 --- a/tools/public_api_guard/cdk/listbox.md +++ b/tools/public_api_guard/cdk/listbox.md @@ -34,10 +34,12 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con protected _getAriaActiveDescendant(): string | null | undefined; protected _getTabIndex(): number | null; protected _handleFocus(): void; + protected _handleFocusIn(): void; protected _handleFocusOut(event: FocusEvent): void; protected _handleKeydown(event: KeyboardEvent): void; get id(): string; set id(value: string); + isActive(option: CdkOption): boolean; isSelected(option: CdkOption): boolean; isValueSelected(value: T): boolean; protected listKeyManager: ActiveDescendantKeyManager>;