From 18a7192e2eda12a391e3f26d33cf2db42511be52 Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Wed, 23 Apr 2025 10:47:47 -0400 Subject: [PATCH 1/2] fix(cdk-experimental/listbox): initial listbox focus state * The first option to receive focus in a listbox should be either the first focusable selected option or the first focusable option in the list if no focusable selected option exists. --- src/cdk-experimental/listbox/listbox.ts | 35 +++++++++-- .../behaviors/list-focus/list-focus.ts | 2 +- .../ui-patterns/listbox/listbox.spec.ts | 60 ++++++++++++++++--- .../ui-patterns/listbox/listbox.ts | 30 ++++++++++ .../cdk-listbox/cdk-listbox-example.html | 10 ++++ .../cdk-listbox/cdk-listbox-example.ts | 1 + 6 files changed, 126 insertions(+), 12 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index ecddfed9c224..03e1c6b48cf3 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -7,14 +7,17 @@ */ import { + AfterViewInit, booleanAttribute, computed, contentChildren, Directive, + effect, ElementRef, inject, input, model, + signal, } from '@angular/core'; import {ListboxPattern, OptionPattern} from '../ui-patterns'; import {Directionality} from '@angular/cdk/bidi'; @@ -29,9 +32,9 @@ import {_IdGenerator} from '@angular/cdk/a11y'; * * ```html * * ``` */ @@ -49,9 +52,10 @@ import {_IdGenerator} from '@angular/cdk/a11y'; '[attr.aria-activedescendant]': 'pattern.activedescendant()', '(keydown)': 'pattern.onKeydown($event)', '(pointerdown)': 'pattern.onPointerdown($event)', + '(focusin)': 'onFocus()', }, }) -export class CdkListbox { +export class CdkListbox implements AfterViewInit { /** The directionality (LTR / RTL) context for the application (or a subtree of it). */ private readonly _directionality = inject(Directionality); @@ -105,6 +109,28 @@ export class CdkListbox { items: this.items, textDirection: this.textDirection, }); + + /** Whether the listbox has received focus yet. */ + private _touched = signal(false); + + /** Whether the options in the listbox have been initialized. */ + private _isViewInitialized = signal(false); + + constructor() { + effect(() => { + if (this._isViewInitialized() && !this._touched()) { + this.pattern.setDefaultState(); + } + }); + } + + ngAfterViewInit() { + this._isViewInitialized.set(true); + } + + onFocus() { + this._touched.set(true); + } } /** A selectable option in a CdkListbox. */ @@ -133,6 +159,7 @@ export class CdkOption { /** A unique identifier for the option. */ protected id = computed(() => this._generatedId); + /** The value of the option. */ protected value = input.required(); // TODO(wagnermaciel): See if we want to change how we handle this since textContent is not diff --git a/src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.ts b/src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.ts index a327507d368e..7f9f3e3e042f 100644 --- a/src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.ts +++ b/src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.ts @@ -75,7 +75,7 @@ export class ListFocus { /** Returns the tabindex for the given item. */ getItemTabindex(item: T): -1 | 0 { - if (this.inputs.disabled()) { + if (this.isListDisabled()) { return -1; } if (this.inputs.focusMode() === 'activedescendant') { diff --git a/src/cdk-experimental/ui-patterns/listbox/listbox.spec.ts b/src/cdk-experimental/ui-patterns/listbox/listbox.spec.ts index a9c465dea200..615de0149069 100644 --- a/src/cdk-experimental/ui-patterns/listbox/listbox.spec.ts +++ b/src/cdk-experimental/ui-patterns/listbox/listbox.spec.ts @@ -67,7 +67,7 @@ describe('Listbox Pattern', () => { const options = signal([]); const listbox = getListbox({...inputs, items: options}); options.set(getOptions(listbox, values)); - return {listbox, options}; + return {listbox, options: options()}; } function getDefaultPatterns(inputs: Partial = {}) { @@ -266,7 +266,7 @@ describe('Listbox Pattern', () => { multi: signal(true), }); listbox = patterns.listbox; - options = patterns.options(); + options = patterns.options; }); it('should select an option on Space', () => { @@ -428,7 +428,7 @@ describe('Listbox Pattern', () => { selectionMode: signal('follow'), }); listbox = patterns.listbox; - options = patterns.options(); + options = patterns.options; }); it('should select an option on navigation', () => { @@ -563,9 +563,9 @@ describe('Listbox Pattern', () => { }); describe('Pointer Events', () => { - function click(options: WritableSignal, index: number, mods?: ModifierKeys) { + function click(options: TestOption[], index: number, mods?: ModifierKeys) { return { - target: options()[index].element(), + target: options[index].element(), shiftKey: mods?.shift, ctrlKey: mods?.control, } as unknown as PointerEvent; @@ -716,7 +716,7 @@ describe('Listbox Pattern', () => { skipDisabled: signal(false), selectionMode: signal('follow'), }); - options()[2].disabled.set(true); + options[2].disabled.set(true); listbox.onPointerdown(click(options, 0)); expect(listbox.inputs.value()).toEqual(['Apple']); @@ -732,7 +732,7 @@ describe('Listbox Pattern', () => { skipDisabled: signal(true), selectionMode: signal('follow'), }); - options()[2].disabled.set(true); + options[2].disabled.set(true); listbox.onPointerdown(click(options, 0)); expect(listbox.inputs.value()).toEqual(['Apple']); listbox.onKeydown(down({control: true})); @@ -785,4 +785,50 @@ describe('Listbox Pattern', () => { expect(listbox.inputs.value()).toEqual(['Apple', 'Banana', 'Blackberry', 'Blueberry']); }); }); + + describe('#setDefaultState', () => { + it('should set the active index to the first option', () => { + const {listbox} = getDefaultPatterns(); + listbox.setDefaultState(); + expect(listbox.inputs.activeIndex()).toBe(0); + }); + + it('should set the active index to the first focusable option', () => { + const {listbox, options} = getDefaultPatterns({ + skipDisabled: signal(true), + }); + options[0].disabled.set(true); + listbox.setDefaultState(); + expect(listbox.inputs.activeIndex()).toBe(1); + }); + + it('should set the active index to the first selected option', () => { + const {listbox} = getDefaultPatterns({ + value: signal(['Banana']), + skipDisabled: signal(true), + }); + listbox.setDefaultState(); + expect(listbox.inputs.activeIndex()).toBe(2); + }); + + it('should set the active index to the first focusable selected option', () => { + const {listbox, options} = getDefaultPatterns({ + value: signal(['Banana', 'Blackberry']), + skipDisabled: signal(true), + }); + options[2].disabled.set(true); + listbox.setDefaultState(); + expect(listbox.inputs.activeIndex()).toBe(3); + }); + + it('should set the active index to the first option if no selected option is focusable', () => { + const {listbox, options} = getDefaultPatterns({ + value: signal(['Banana']), + skipDisabled: signal(true), + }); + options[2].disabled.set(true); + listbox.setDefaultState(); + expect(listbox.inputs.activeIndex()).toBe(0); + }); + }); }); diff --git a/src/cdk-experimental/ui-patterns/listbox/listbox.ts b/src/cdk-experimental/ui-patterns/listbox/listbox.ts index 4a6f5491c3f8..4b85499ffc32 100644 --- a/src/cdk-experimental/ui-patterns/listbox/listbox.ts +++ b/src/cdk-experimental/ui-patterns/listbox/listbox.ts @@ -281,6 +281,36 @@ export class ListboxPattern { this._navigate(opts, () => this.typeahead.search(char)); } + /** + * Sets the listbox to it's default initial state. + * + * Sets the active index of the listbox to the first focusable selected + * item if one exists. Otherwise, sets focus to the first focusable item. + * + * This method should be called once the listbox and it's options are properly initialized, + * meaning the ListboxPattern and OptionPatterns should have references to each other before this + * is called. + */ + setDefaultState() { + let firstItem: OptionPattern | null = null; + + for (const item of this.inputs.items()) { + if (this.focusManager.isFocusable(item)) { + if (!firstItem) { + firstItem = item; + } + if (item.selected()) { + this.inputs.activeIndex.set(item.index()); + return; + } + } + } + + if (firstItem) { + this.inputs.activeIndex.set(firstItem.index()); + } + } + /** * Safely performs a navigation operation. * diff --git a/src/components-examples/cdk-experimental/listbox/cdk-listbox/cdk-listbox-example.html b/src/components-examples/cdk-experimental/listbox/cdk-listbox/cdk-listbox-example.html index 1bcb00409425..2e7b45026674 100644 --- a/src/components-examples/cdk-experimental/listbox/cdk-listbox/cdk-listbox-example.html +++ b/src/components-examples/cdk-experimental/listbox/cdk-listbox/cdk-listbox-example.html @@ -5,6 +5,15 @@ Readonly Skip Disabled + + Selection + + @for (fruit of fruits; track fruit) { + {{fruit}} + } + + + Disabled Options @@ -42,6 +51,7 @@
    Date: Wed, 23 Apr 2025 13:08:03 -0400 Subject: [PATCH 2/2] fixup! fix(cdk-experimental/listbox): initial listbox focus state --- src/cdk-experimental/listbox/listbox.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index 03e1c6b48cf3..4831727486d1 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -111,14 +111,14 @@ export class CdkListbox implements AfterViewInit { }); /** Whether the listbox has received focus yet. */ - private _touched = signal(false); + private _hasFocused = signal(false); /** Whether the options in the listbox have been initialized. */ private _isViewInitialized = signal(false); constructor() { effect(() => { - if (this._isViewInitialized() && !this._touched()) { + if (this._isViewInitialized() && !this._hasFocused()) { this.pattern.setDefaultState(); } }); @@ -129,7 +129,7 @@ export class CdkListbox implements AfterViewInit { } onFocus() { - this._touched.set(true); + this._hasFocused.set(true); } }