Skip to content

Commit

Permalink
fix(cdk/listbox): set initial focus to selected option (#26174)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto committed Dec 5, 2022
1 parent 30c6c8f commit f99af6d
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 13 deletions.
34 changes: 34 additions & 0 deletions src/cdk/listbox/listbox.spec.ts
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -909,6 +941,7 @@ describe('CdkOption and CdkListbox', () => {
[cdkListboxOrientation]="orientation"
[cdkListboxNavigationWrapDisabled]="!navigationWraps"
[cdkListboxNavigatesDisabledOptions]="!navigationSkipsDisabled"
[cdkListboxValue]="selectedValue"
(cdkListboxValueChange)="onSelectionChange($event)">
<div cdkOption="apple"
[cdkOptionDisabled]="isAppleDisabled"
Expand Down Expand Up @@ -937,6 +970,7 @@ class ListboxWithOptions {
appleId: string;
appleTabindex: number;
orientation: 'horizontal' | 'vertical' = 'vertical';
selectedValue: string;

onSelectionChange(event: ListboxValueChangeEvent<unknown>) {
this.changedOption = event.option;
Expand Down
63 changes: 50 additions & 13 deletions src/cdk/listbox/listbox.ts
Expand Up @@ -146,9 +146,6 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab
/** Emits when the option is clicked. */
readonly _clicked = new Subject<MouseEvent>();

/** Whether the option is currently active. */
private _active = false;

ngOnDestroy() {
this.destroyed.next();
this.destroyed.complete();
Expand All @@ -161,7 +158,7 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab

/** Whether this option is active. */
isActive() {
return this._active;
return this.listbox.isActive(this);
}

/** Toggle the selected state of this option. */
Expand Down Expand Up @@ -190,20 +187,16 @@ export class CdkOption<T = unknown> 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() {
Expand Down Expand Up @@ -240,6 +233,7 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab
'(focus)': '_handleFocus()',
'(keydown)': '_handleKeydown($event)',
'(focusout)': '_handleFocusOut($event)',
'(focusin)': '_handleFocusIn()',
},
providers: [
{
Expand Down Expand Up @@ -419,6 +413,9 @@ export class CdkListbox<T = unknown> 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();
Expand Down Expand Up @@ -526,6 +523,14 @@ export class CdkListbox<T = unknown> 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<T>): boolean {
return !!(this.listKeyManager?.activeItem === option);
}

/**
* Get whether the given value is selected.
* @param value The value to get the selected state of
Expand Down Expand Up @@ -653,7 +658,12 @@ export class CdkListbox<T = unknown> 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();
}
}
Expand Down Expand Up @@ -759,6 +769,13 @@ export class CdkListbox<T = unknown> 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
Expand All @@ -767,6 +784,8 @@ export class CdkListbox<T = unknown> 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();
}
}

Expand Down Expand Up @@ -800,6 +819,10 @@ export class CdkListbox<T = unknown> 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());
}

Expand All @@ -820,6 +843,20 @@ export class CdkListbox<T = unknown> 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. */
Expand Down
2 changes: 2 additions & 0 deletions tools/public_api_guard/cdk/listbox.md
Expand Up @@ -34,10 +34,12 @@ export class CdkListbox<T = unknown> 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<T>): boolean;
isSelected(option: CdkOption<T>): boolean;
isValueSelected(value: T): boolean;
protected listKeyManager: ActiveDescendantKeyManager<CdkOption<T>>;
Expand Down

0 comments on commit f99af6d

Please sign in to comment.