From 9260fb5d769fefbb0e7d4cf9ff02877a93768ca0 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 12 May 2022 23:46:38 +0000 Subject: [PATCH 01/10] fix(cdk-experimental/listbox): clean up the listbox API and make it work with forms --- src/cdk-experimental/listbox/listbox.spec.ts | 58 +- src/cdk-experimental/listbox/listbox.ts | 692 +++++++++--------- src/cdk/collections/selection-model.ts | 11 + .../cdk-experimental-listbox/BUILD.bazel | 1 + .../cdk-listbox-demo.html | 44 +- .../cdk-listbox-demo.ts | 23 +- 6 files changed, 434 insertions(+), 395 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.spec.ts b/src/cdk-experimental/listbox/listbox.spec.ts index fbe784ca2891..6a541bfa6d44 100644 --- a/src/cdk-experimental/listbox/listbox.spec.ts +++ b/src/cdk-experimental/listbox/listbox.spec.ts @@ -1,7 +1,7 @@ import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing'; import {Component, DebugElement, ViewChild} from '@angular/core'; import {By} from '@angular/platform-browser'; -import {CdkListbox, CdkListboxModule, CdkOption, ListboxSelectionChangeEvent} from './index'; +import {CdkListbox, CdkListboxModule, CdkOption, ListboxValueChangeEvent} from './index'; import { createKeyboardEvent, dispatchKeyboardEvent, @@ -114,17 +114,17 @@ describe('CdkOption and CdkListbox', () => { listboxInstance.setActiveOption(optionInstances[1]); fixture.detectChanges(); - expect(listboxInstance._listKeyManager.activeItem).toBe(optionInstances[1]); + expect(listboxInstance.listKeyManager.activeItem).toBe(optionInstances[1]); dispatchKeyboardEvent(listboxElement, 'keydown', HOME); fixture.detectChanges(); - expect(listboxInstance._listKeyManager.activeItem).toBe(optionInstances[0]); + expect(listboxInstance.listKeyManager.activeItem).toBe(optionInstances[0]); dispatchKeyboardEvent(listboxElement, 'keydown', END); fixture.detectChanges(); - expect(listboxInstance._listKeyManager.activeItem).toBe(optionInstances[3]); + expect(listboxInstance.listKeyManager.activeItem).toBe(optionInstances[3]); }); it('should be able to toggle listbox disabled state', () => { @@ -210,15 +210,15 @@ describe('CdkOption and CdkListbox', () => { }); it('should change active item using type ahead', fakeAsync(() => { - expect(listboxInstance._listKeyManager.activeItem).toBeNull(); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(-1); + expect(listboxInstance.listKeyManager.activeItem).toBeNull(); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); dispatchKeyboardEvent(listboxElement, 'keydown', A); fixture.detectChanges(); tick(200); - expect(listboxInstance._listKeyManager.activeItem).toEqual(optionInstances[2]); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(2); + expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[2]); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(2); })); it('should not handle space or enter on a disabled listbox', () => { @@ -240,8 +240,8 @@ describe('CdkOption and CdkListbox', () => { }); it('should not handle type ahead on a disabled listbox', fakeAsync(() => { - expect(listboxInstance._listKeyManager.activeItem).toBeNull(); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(-1); + expect(listboxInstance.listKeyManager.activeItem).toBeNull(); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); testComponent.isListboxDisabled = true; fixture.detectChanges(); @@ -250,8 +250,8 @@ describe('CdkOption and CdkListbox', () => { fixture.detectChanges(); tick(200); - expect(listboxInstance._listKeyManager.activeItem).toBeNull(); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(-1); + expect(listboxInstance.listKeyManager.activeItem).toBeNull(); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); })); it('should not select a disabled option using space or enter', () => { @@ -273,38 +273,38 @@ describe('CdkOption and CdkListbox', () => { }); it('should update active item upon arrow key presses', () => { - expect(listboxInstance._listKeyManager.activeItem).toBeNull(); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(-1); + expect(listboxInstance.listKeyManager.activeItem).toBeNull(); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); fixture.detectChanges(); - expect(listboxInstance._listKeyManager.activeItem).toEqual(optionInstances[0]); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(0); + expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[0]); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(0); dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); fixture.detectChanges(); - expect(listboxInstance._listKeyManager.activeItem).toEqual(optionInstances[1]); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(1); + expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[1]); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(1); }); it('should skip disabled options when navigating with arrow keys', () => { - expect(listboxInstance._listKeyManager.activeItem).toBeNull(); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(-1); + expect(listboxInstance.listKeyManager.activeItem).toBeNull(); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); testComponent.isSolarDisabled = true; dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); fixture.detectChanges(); - expect(listboxInstance._listKeyManager.activeItem).toEqual(optionInstances[0]); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(0); + expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[0]); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(0); dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); fixture.detectChanges(); - expect(listboxInstance._listKeyManager.activeItem).toEqual(optionInstances[2]); - expect(listboxInstance._listKeyManager.activeItemIndex).toBe(2); + expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[2]); + expect(listboxInstance.listKeyManager.activeItemIndex).toBe(2); }); it('should update selected option on click event', () => { @@ -903,7 +903,7 @@ class ListboxWithOptions { isPurpleDisabled: boolean = false; isSolarDisabled: boolean = false; - onSelectionChange(event: ListboxSelectionChangeEvent) { + onSelectionChange(event: ListboxValueChangeEvent) { this.changedOption = event.option; } } @@ -923,7 +923,7 @@ class ListboxMultiselect { changedOption: CdkOption; isMultiselectable: boolean = false; - onSelectionChange(event: ListboxSelectionChangeEvent) { + onSelectionChange(event: ListboxValueChangeEvent) { this.changedOption = event.option; } } @@ -943,7 +943,7 @@ class ListboxActiveDescendant { isActiveDescendant: boolean = true; focusedOption: string; - onSelectionChange(event: ListboxSelectionChangeEvent) { + onSelectionChange(event: ListboxValueChangeEvent) { this.changedOption = event.option; } @@ -974,7 +974,7 @@ class ListboxControlValueAccessor { showListbox: boolean = true; @ViewChild(CdkListbox) listbox: CdkListbox; - onSelectionChange(event: ListboxSelectionChangeEvent) { + onSelectionChange(event: ListboxValueChangeEvent) { this.changedOption = event.option; } } @@ -1007,7 +1007,7 @@ class ListboxInsideCombobox { @ViewChild(CdkListbox) listbox: CdkListbox; @ViewChild(CdkCombobox) combobox: CdkCombobox; - onSelectionChange(event: ListboxSelectionChangeEvent) { + onSelectionChange(event: ListboxValueChangeEvent) { this.changedOption = event.option; } } diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index cdbf1c445b0d..fd4fbd579abb 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -8,15 +8,14 @@ import { AfterContentInit, + ChangeDetectorRef, ContentChildren, Directive, ElementRef, - EventEmitter, forwardRef, Inject, Input, OnDestroy, - OnInit, Optional, Output, QueryList, @@ -24,21 +23,18 @@ import { import {ActiveDescendantKeyManager, Highlightable, ListKeyManagerOption} from '@angular/cdk/a11y'; import {DOWN_ARROW, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, UP_ARROW} from '@angular/cdk/keycodes'; import {BooleanInput, coerceArray, coerceBooleanProperty} from '@angular/cdk/coercion'; -import {SelectionChange, SelectionModel} from '@angular/cdk/collections'; -import {defer, merge, Observable, Subject} from 'rxjs'; -import {startWith, switchMap, takeUntil} from 'rxjs/operators'; +import {SelectionModel} from '@angular/cdk/collections'; +import {BehaviorSubject, defer, merge, Observable, Subject} from 'rxjs'; +import {filter, mapTo, startWith, switchMap, takeUntil} from 'rxjs/operators'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {Directionality} from '@angular/cdk/bidi'; import {CDK_COMBOBOX, CdkCombobox} from '@angular/cdk-experimental/combobox'; -let nextId = 0; -let listboxId = 0; +/** The next id to use for the CdkOption directive. */ +let nextOptionId = 0; -export const CDK_LISTBOX_VALUE_ACCESSOR = { - provide: NG_VALUE_ACCESSOR, - useExisting: forwardRef(() => CdkListbox), - multi: true, -}; +/** The next id to use for the CdkListbox directive. */ +let nextListboxId = 0; @Directive({ selector: '[cdkOption]', @@ -46,144 +42,112 @@ export const CDK_LISTBOX_VALUE_ACCESSOR = { host: { 'role': 'option', 'class': 'cdk-option', - '(click)': 'toggle()', - '(focus)': 'activate()', - '(blur)': 'deactivate()', '[id]': 'id', - '[attr.aria-selected]': 'selected || null', - '[attr.tabindex]': '_getTabIndex()', - '[attr.aria-disabled]': '_isInteractionDisabled()', - '[class.cdk-option-disabled]': '_isInteractionDisabled()', - '[class.cdk-option-active]': '_active', - '[class.cdk-option-selected]': 'selected', + '[attr.aria-selected]': 'isSelected() || null', + '[attr.tabindex]': '!listbox.useActiveDescendant && !disabled ? -1 : null', + '[attr.aria-disabled]': 'disabled', + '[class.cdk-option-disabled]': 'disabled', + '[class.cdk-option-active]': 'isActive()', + '[class.cdk-option-selected]': 'isSelected()', + '(click)': '_clicked.next()', }, }) -export class CdkOption implements ListKeyManagerOption, Highlightable { - private _selected: boolean = false; - private _disabled: boolean = false; - private _value: T; - _active: boolean = false; +export class CdkOption implements ListKeyManagerOption, Highlightable, OnDestroy { + /** The id of the option's host element. */ + @Input() id = `cdk-option-${nextOptionId++}`; - /** The id of the option, set to a uniqueid if the user does not provide one. */ - @Input() id = `cdk-option-${nextId++}`; + // TODO(mmalerba): What do we do if the user doesn't specify a value? + /** The value of this option. */ + @Input('cdkOptionValue') value: T; - @Input() - get selected(): boolean { - return this._selected; - } - set selected(value: BooleanInput) { - if (!this._disabled) { - this._selected = coerceBooleanProperty(value); - } - } + /** + * The text used to locate this item during listbox typeahead. If not specified, + * the `textContent` of the item will be used. + */ + @Input('cdkOptionTypeaheadLabel') typeaheadLabel: string; - @Input() + /** Whether this option is disabled. */ + @Input('cdkOptionDisabled') get disabled(): boolean { - return this._disabled; + return this.listbox.disabled || this._disabled; } set disabled(value: BooleanInput) { this._disabled = coerceBooleanProperty(value); } + private _disabled: boolean = false; - /** The form value of the option. */ - @Input() - get value(): T { - return this._value; - } - set value(value: T) { - if (this.selected && value !== this._value) { - this.deselect(); - } - this._value = value; - } + /** Emits when the option is destroyed. */ + protected destroyed = new Subject(); - /** - * The text used to locate this item during menu typeahead. If not specified, - * the `textContent` of the item will be used. - */ - @Input() typeahead: string; + /** Emits when the option is clicked. */ + readonly _clicked = new Subject(); - @Output() readonly selectionChange = new EventEmitter>(); + /** Whether the option is currently active. */ + private _active = false; constructor( - private readonly _elementRef: ElementRef, - @Inject(forwardRef(() => CdkListbox)) readonly listbox: CdkListbox, - ) {} - - /** Toggles the selected state, emits a change event through the injected listbox. */ - toggle() { - if (!this._isInteractionDisabled()) { - this.selected = !this.selected; - this._emitSelectionChange(true); - } - } - - /** Sets the active property true if the option and listbox aren't disabled. */ - activate() { - if (!this._isInteractionDisabled()) { - this._active = true; - } + /** The option's host element */ + protected readonly elementRef: ElementRef, + /** The change detector ref for this option. */ + protected readonly changeDetectorRef: ChangeDetectorRef, + /** The parent listbox this option belongs to. */ + @Inject(forwardRef(() => CdkListbox)) protected readonly listbox: CdkListbox, + ) { + this.listbox._internalValueChange.pipe(takeUntil(this.destroyed)).subscribe(() => { + this.changeDetectorRef.markForCheck(); + }); } - /** Sets the active property false. */ - deactivate() { - if (!this._isInteractionDisabled()) { - this._active = false; - } + ngOnDestroy() { + this.destroyed.next(); + this.destroyed.complete(); } - /** Sets the selected property true if it was false. */ - select() { - if (!this.selected) { - this.selected = true; - this._emitSelectionChange(); - } + /** Whether this option is selected. */ + isSelected() { + return this.listbox.isSelected(this.value); } - /** Sets the selected property false if it was true. */ - deselect() { - if (this.selected) { - this.selected = false; - this._emitSelectionChange(); - } + /** Whether this option is active. */ + isActive() { + return this._active; } - /** Applies focus to the option. */ - focus() { - this._elementRef.nativeElement.focus(); + /** Toggle the selected state of this option. */ + toggle() { + this.listbox.toggle(this); } - /** Returns true if the option or listbox are disabled, and false otherwise. */ - _isInteractionDisabled(): boolean { - return this.listbox.disabled || this._disabled; + /** Select this option if it is not selected. */ + select() { + this.listbox.select(this); } - /** Emits a change event extending the Option Selection Change Event interface. */ - private _emitSelectionChange(isUserInput = false) { - this.selectionChange.emit({ - source: this, - isUserInput: isUserInput, - }); + /** Deselect this option if it is selected. */ + deselect() { + this.listbox.deselect(this); } - /** Returns the tab index which depends on the disabled property. */ - _getTabIndex(): string | null { - return this._isInteractionDisabled() ? null : '-1'; + /** Focus this option. */ + focus() { + this.elementRef.nativeElement.focus(); } /** Get the label for this element which is required by the FocusableOption interface. */ getLabel() { - return (this.typeahead ?? this._elementRef.nativeElement.textContent?.trim()) || ''; + return (this.typeaheadLabel ?? this.elementRef.nativeElement.textContent?.trim()) || ''; } - /** Sets the active property to true to enable the active css class. */ + /** Set the option as active. */ setActiveStyles() { this._active = true; + this.changeDetectorRef.markForCheck(); } - /** Sets the active property to false to disable the active css class. */ + /** Set the option as inactive. */ setInactiveStyles() { this._active = false; + this.changeDetectorRef.markForCheck(); } } @@ -194,360 +158,382 @@ export class CdkOption implements ListKeyManagerOption, Highlightab 'role': 'listbox', 'class': 'cdk-listbox', '[id]': 'id', - '(focus)': '_focusActiveOption()', - '(keydown)': '_keydown($event)', - '[attr.tabindex]': '_tabIndex', + '[attr.tabindex]': 'disabled ? null : 0', '[attr.aria-disabled]': 'disabled', '[attr.aria-multiselectable]': 'multiple', '[attr.aria-activedescendant]': '_getAriaActiveDescendant()', '[attr.aria-orientation]': 'orientation', + '(focus)': '_handleFocus()', + '(keydown)': '_handleKeydown($event)', + '(focusout)': '_handleFocusOut($event)', }, - providers: [CDK_LISTBOX_VALUE_ACCESSOR], + providers: [ + { + provide: NG_VALUE_ACCESSOR, + useExisting: forwardRef(() => CdkListbox), + multi: true, + }, + ], }) -export class CdkListbox implements AfterContentInit, OnDestroy, OnInit, ControlValueAccessor { - _listKeyManager: ActiveDescendantKeyManager>; - _selectionModel: SelectionModel>; - _tabIndex = 0; - - /** `View -> model callback called when select has been touched` */ - _onTouched: () => void = () => {}; - - /** `View -> model callback called when value changes` */ - _onChange: (value: T) => void = () => {}; - - readonly optionSelectionChanges: Observable> = defer(() => { - const options = this._options; - - return options.changes.pipe( - startWith(options), - switchMap(() => merge(...options.map(option => option.selectionChange))), - ); - }) as Observable>; - - private _disabled: boolean = false; - private _multiple: boolean = false; - private _useActiveDescendant: boolean = false; - private _autoFocus: boolean = true; - private _activeOption: CdkOption; - private readonly _destroyed = new Subject(); - - @ContentChildren(CdkOption, {descendants: true}) _options: QueryList>; - - @Output() readonly selectionChange = new EventEmitter>(); +export class CdkListbox implements AfterContentInit, OnDestroy, ControlValueAccessor { + /** The id of the option's host element. */ + @Input() id = `cdk-listbox-${nextListboxId++}`; - @Input() id = `cdk-listbox-${listboxId++}`; + // TODO(mmalerba): Should we normalize the order? Do we care about duplicates? + /** The value selected in the listbox, represented as an array of option values. */ + @Input('cdkListboxValue') + get value(): T[] { + return this.selectionModel().selected; + } + set value(value: T[]) { + this.selectionModel().setSelection(...coerceArray(value == null ? [] : value)); + } /** - * Whether the listbox allows multiple options to be selected. - * If `multiple` switches from `true` to `false`, all options are deselected. + * Whether the listbox allows multiple options to be selected. If the value switches from `true` + * to `false`, and more than one option is selected, all options are deselected. */ - @Input() + @Input('cdkListboxMultiple') get multiple(): boolean { return this._multiple; } set multiple(value: BooleanInput) { - const coercedValue = coerceBooleanProperty(value); - this._updateSelectionOnMultiSelectionChange(coercedValue); - this._multiple = coercedValue; + this._multiple = coerceBooleanProperty(value); + this._updateSelectionMode(); } + private _multiple: boolean = false; - @Input() + /** Whether the listbox is disabled. */ + @Input('cdkListboxDisabled') get disabled(): boolean { return this._disabled; } set disabled(value: BooleanInput) { this._disabled = coerceBooleanProperty(value); } + private _disabled: boolean = false; /** Whether the listbox will use active descendant or will move focus onto the options. */ - @Input() + @Input('cdkListboxUseActiveDescendant') get useActiveDescendant(): boolean { return this._useActiveDescendant; } set useActiveDescendant(shouldUseActiveDescendant: BooleanInput) { this._useActiveDescendant = coerceBooleanProperty(shouldUseActiveDescendant); } + private _useActiveDescendant: boolean = false; + // TODO(mmalerba): Why do we have this? Shouldn't it depend on whether we're using active descendant? /** Whether on focus the listbox will focus its active option, default to true. */ - @Input() + @Input('cdkListboxAutoFocus') get autoFocus(): boolean { return this._autoFocus; } set autoFocus(shouldAutoFocus: BooleanInput) { this._autoFocus = coerceBooleanProperty(shouldAutoFocus); } + private _autoFocus: boolean = true; + + /** The orientation of the listbox. Only affects keyboard interaction, not visual layout. */ + @Input('cdkListboxOrientation') orientation: 'horizontal' | 'vertical' = 'vertical'; + + // TODO(mmalerba): This is currently unused, do we need to do something with it? + /** The function used to compare option values. */ + @Input('cdkListboxCompareWith') compareWith: (o1: T, o2: T) => boolean = (a1, a2) => a1 === a2; + + /** Emits when the selected value(s) in the listbox change. */ + @Output('cdkListboxValueChange') readonly valueChange = new Subject>(); + + /** The child options in this listbox. */ + @ContentChildren(CdkOption, {descendants: true}) protected options: QueryList>; + + /** The selection model used by the listbox. */ + protected selectionModelSubject = new BehaviorSubject(new SelectionModel(this.multiple)); + + /** The key manager that manages keyboard navigation for this listbox. */ + protected listKeyManager: ActiveDescendantKeyManager>; - /** Determines the orientation for the list key manager. Affects keyboard interaction. */ - @Input('listboxOrientation') orientation: 'horizontal' | 'vertical' = 'vertical'; + /** Emits when the listbox is destroyed. */ + protected readonly destroyed = new Subject(); - @Input() compareWith: (o1: T, o2: T) => boolean = (a1, a2) => a1 === a2; + /** Emits when the internal value of the listbox changes for any reason. */ + _internalValueChange = this.selectionModelSubject.pipe( + switchMap(selectionModel => selectionModel.changed), + takeUntil(this.destroyed), + ); + + /** Callback called when the listbox has been touched */ + private _onTouched: () => void = () => {}; + + /** Callback called when the listbox value changes */ + private _onChange: (value: T[]) => void = () => {}; + + /** Emits when an option has been clicked. */ + private _optionClicked = defer(() => + (this.options.changes as Observable[]>).pipe( + startWith(this.options), + switchMap(options => merge(...options.map(option => option._clicked.pipe(mapTo(option))))), + ), + ); constructor( + /** The host element of the listbox. */ + protected elementRef: ElementRef, + // TODO(mmalerba): Should not depend on combobox @Optional() @Inject(CDK_COMBOBOX) private readonly _combobox: CdkCombobox, + /** The directionality of the page. */ @Optional() private readonly _dir?: Directionality, ) {} - ngOnInit() { - this._selectionModel = new SelectionModel>(this.multiple); - } - ngAfterContentInit() { this._initKeyManager(); - this._initSelectionModel(); this._combobox?._registerContent(this.id, 'listbox'); - - this.optionSelectionChanges.subscribe(event => { - this._emitChangeEvent(event.source); - this._updateSelectionModel(event.source); - this.setActiveOption(event.source); - this._updatePanelForSelection(event.source); - }); + this._optionClicked + .pipe( + filter(option => !option.disabled), + takeUntil(this.destroyed), + ) + .subscribe(option => { + this.listKeyManager.setActiveItem(option); + this.triggerOption(option); + this._updatePanelForSelection(option); + }); } ngOnDestroy() { - this._listKeyManager.change.complete(); - this._destroyed.next(); - this._destroyed.complete(); + this.listKeyManager.change.complete(); + this.destroyed.next(); + this.destroyed.complete(); } - private _initKeyManager() { - this._listKeyManager = new ActiveDescendantKeyManager(this._options) - .withWrap() - .withTypeAhead() - .withHomeAndEnd() - .withAllowedModifierKeys(['shiftKey']); - - if (this.orientation === 'vertical') { - this._listKeyManager.withVerticalOrientation(); - } else { - this._listKeyManager.withHorizontalOrientation(this._dir?.value || 'ltr'); - } - - this._listKeyManager.change.pipe(takeUntil(this._destroyed)).subscribe(() => { - this._updateActiveOption(); - }); + /** + * Toggle the selected state of the given option. + * @param option The option to toggle + */ + toggle(option: CdkOption | T) { + this.selectionModel().toggle(option instanceof CdkOption ? option.value : option); } - private _initSelectionModel() { - this._selectionModel.changed - .pipe(takeUntil(this._destroyed)) - .subscribe((event: SelectionChange>) => { - for (const option of event.added) { - option.selected = true; - } - - for (const option of event.removed) { - option.selected = false; - } - }); + /** + * Select the given option. + * @param option The option to select + */ + select(option: CdkOption | T) { + this.selectionModel().select(option instanceof CdkOption ? option.value : option); } - _keydown(event: KeyboardEvent) { - if (this._disabled) { - return; - } - - const manager = this._listKeyManager; - const {keyCode} = event; - const previousActiveIndex = manager.activeItemIndex; + /** + * Deselect the given option. + * @param option The option to deselect + */ + deselect(option: CdkOption | T) { + this.selectionModel().deselect(option instanceof CdkOption ? option.value : option); + } - if (keyCode === SPACE || keyCode === ENTER) { - if (manager.activeItem && !manager.isTyping()) { - this._toggleActiveOption(); - } - event.preventDefault(); + /** + * Set the selected state of all options. + * @param isSelected The new selected state to set + */ + setAllSelected(isSelected: boolean) { + if (!isSelected) { + this.selectionModel().clear(); } else { - manager.onKeydown(event); - } - - /** Will select an option if shift was pressed while navigating to the option */ - const isArrow = - keyCode === UP_ARROW || - keyCode === DOWN_ARROW || - keyCode === LEFT_ARROW || - keyCode === RIGHT_ARROW; - if (isArrow && event.shiftKey && previousActiveIndex !== this._listKeyManager.activeItemIndex) { - this._toggleActiveOption(); + this.selectionModel().select(...this.options.toArray().map(option => option.value)); } } - /** Emits a selection change event, called when an option has its selected state changed. */ - _emitChangeEvent(option: CdkOption) { - this.selectionChange.emit({ - source: this, - option: option, - }); + /** + * Get whether the given option is selected. + * @param option The option to get the selected state of + */ + isSelected(option: CdkOption | T) { + return this.selectionModel().isSelected(option instanceof CdkOption ? option.value : option); } - /** Updates the selection model after a toggle. */ - _updateSelectionModel(option: CdkOption) { - if (!this.multiple && this._selectionModel.selected.length !== 0) { - const previouslySelected = this._selectionModel.selected[0]; - this.deselect(previouslySelected); - } - - option.selected ? this._selectionModel.select(option) : this._selectionModel.deselect(option); + /** + * Registers a callback to be invoked when the listbox's value changes from user input. + * @param fn The callback to register + * @docs-private + */ + registerOnChange(fn: (value: T[]) => void): void { + this._onChange = fn; } - _updatePanelForSelection(option: CdkOption) { - if (this._combobox) { - if (!this.multiple) { - this._combobox.updateAndClose(option.selected ? option.value : []); - } else { - this._combobox.updateAndClose(this.getSelectedValues()); - } - } + /** + * Registers a callback to be invoked when the listbox is blurred by the user. + * @param fn The callback to register + * @docs-private + */ + registerOnTouched(fn: () => {}): void { + this._onTouched = fn; } - /** Toggles the selected state of the active option if not disabled. */ - private _toggleActiveOption() { - const activeOption = this._listKeyManager.activeItem; - if (activeOption && !activeOption.disabled) { - activeOption.toggle(); + /** + * Sets the listbox's value. + * @param value The new value of the listbox + * @docs-private + */ + writeValue(value: T[]): void { + if (this.options) { + this.selectionModel().setSelection(...(value == null ? [] : coerceArray(value))); } } - /** Returns the id of the active option if active descendant is being used. */ - _getAriaActiveDescendant(): string | null | undefined { - return this._useActiveDescendant ? this._listKeyManager?.activeItem?.id : null; + /** + * Sets the disabled state of the listbox. + * @param isDisabled The new disabled state + * @docs-private + */ + setDisabledState(isDisabled: boolean): void { + this.disabled = isDisabled; } - /** Updates the activeOption and the active and focus properties of the option. */ - private _updateActiveOption() { - if (!this._listKeyManager.activeItem) { - return; - } - - this._activeOption?.deactivate(); - this._activeOption = this._listKeyManager.activeItem; - this._activeOption.activate(); + /** The selection model used to track the listbox's value. */ + protected selectionModel() { + return this.selectionModelSubject.value; + } - if (!this.useActiveDescendant) { - this._activeOption.focus(); + /** + * Triggers the given option in response to user interaction. + * - In single selection mode: selects the option and deselects any other selected option. + * - In multi selection mode: toggles the selected state of the option. + * @param option The option to trigger + */ + protected triggerOption(option: CdkOption | null) { + if (option && !option.disabled) { + let changed = false; + const subscription = this.selectionModel().changed.subscribe(() => (changed = true)); + if (this.multiple) { + this.toggle(option); + } else { + this.select(option); + } + subscription.unsubscribe(); + if (changed) { + this._onChange(this.value); + this.valueChange.next({ + value: this.value, + listbox: this, + option: option, + }); + } } } - /** Updates selection states of options when the 'multiple' property changes. */ - private _updateSelectionOnMultiSelectionChange(value: boolean) { - if (this.multiple && !value) { - // Deselect all options instead of arbitrarily keeping one of the selected options. - this.setAllSelected(false); - } else if (!this.multiple && value) { - this._selectionModel = new SelectionModel>( - value, - this._selectionModel?.selected, - ); + /** Called when the listbox receives focus. */ + protected _handleFocus() { + if (this.autoFocus) { + this.listKeyManager.setActiveItem(this.listKeyManager.activeItem ?? this.options.first); + this._focusActiveOption(); } } - _focusActiveOption() { - if (!this.autoFocus) { + /** Called when the user presses keydown on the listbox. */ + protected _handleKeydown(event: KeyboardEvent) { + if (this._disabled) { return; } - if (this._listKeyManager.activeItem) { - this.setActiveOption(this._listKeyManager.activeItem); - } else if (this._options.first) { - this.setActiveOption(this._options.first); - } - } - - /** Selects the given option if the option and listbox aren't disabled. */ - select(option: CdkOption) { - if (!this.disabled && !option.disabled) { - option.select(); - } - } + const {keyCode} = event; + const previousActiveIndex = this.listKeyManager.activeItemIndex; - /** Deselects the given option if the option and listbox aren't disabled. */ - deselect(option: CdkOption) { - if (!this.disabled && !option.disabled) { - option.deselect(); + if (keyCode === SPACE || keyCode === ENTER) { + this.triggerOption(this.listKeyManager.activeItem); + event.preventDefault(); + } else { + this.listKeyManager.onKeydown(event); } - } - /** Sets the selected state of all options to be the given value. */ - setAllSelected(isSelected: boolean) { - for (const option of this._options.toArray()) { - isSelected ? this.select(option) : this.deselect(option); + /** Will select an option if shift was pressed while navigating to the option */ + const isArrow = + keyCode === UP_ARROW || + keyCode === DOWN_ARROW || + keyCode === LEFT_ARROW || + keyCode === RIGHT_ARROW; + if (isArrow && event.shiftKey && previousActiveIndex !== this.listKeyManager.activeItemIndex) { + this.triggerOption(this.listKeyManager.activeItem); } } - /** Updates the key manager's active item to the given option. */ - setActiveOption(option: CdkOption) { - this._listKeyManager.updateActiveItem(option); - this._updateActiveOption(); - } - /** - * Saves a callback function to be invoked when the select's value - * changes from user input. Required to implement ControlValueAccessor. + * Called when the focus leaves an element in the listbox. + * @param event The focusout event */ - registerOnChange(fn: (value: T) => void): void { - this._onChange = fn; + protected _handleFocusOut(event: FocusEvent) { + const hostElement = this.elementRef.nativeElement; + const otherElement = event.relatedTarget as Element; + if (hostElement !== otherElement && !hostElement.contains(otherElement)) { + this._onTouched(); + } } - /** - * Saves a callback function to be invoked when the select is blurred - * by the user. Required to implement ControlValueAccessor. - */ - registerOnTouched(fn: () => {}): void { - this._onTouched = fn; + /** Get the id of the active option if active descendant is being used. */ + protected _getAriaActiveDescendant(): string | null | undefined { + return this._useActiveDescendant ? this.listKeyManager?.activeItem?.id : null; } - /** Sets the select's value. Required to implement ControlValueAccessor. */ - writeValue(values: T | T[]): void { - if (this._options) { - this._setSelectionByValue(values); + /** Initialize the key manager. */ + private _initKeyManager() { + this.listKeyManager = new ActiveDescendantKeyManager(this.options) + .withWrap() + .withTypeAhead() + .withHomeAndEnd() + .withAllowedModifierKeys(['shiftKey']); + + if (this.orientation === 'vertical') { + this.listKeyManager.withVerticalOrientation(); + } else { + this.listKeyManager.withHorizontalOrientation(this._dir?.value || 'ltr'); } - } - /** Disables the select. Required to implement ControlValueAccessor. */ - setDisabledState(isDisabled: boolean): void { - this.disabled = isDisabled; + this.listKeyManager.change + .pipe(takeUntil(this.destroyed)) + .subscribe(() => this._focusActiveOption()); } - /** Returns the values of the currently selected options. */ - getSelectedValues(): T[] { - return this._options.filter(option => option.selected).map(option => option.value); + // TODO(mmalerba): Should not depend on combobox. + private _updatePanelForSelection(option: CdkOption) { + if (this._combobox) { + if (!this.multiple) { + this._combobox.updateAndClose(option.isSelected() ? option.value : []); + } else { + this._combobox.updateAndClose(this.value); + } + } } - /** Selects an option that has the corresponding given value. */ - private _setSelectionByValue(values: T | T[]) { - for (const option of this._options.toArray()) { - this.deselect(option); + /** Update the selection mode when the 'multiple' property changes. */ + private _updateSelectionMode() { + if (this.multiple !== this.selectionModel().isMultipleSelection()) { + let newSelection = this.value; + // If we're changing to a single selection model and there are multiple items selected, + // clear the selection rather than arbitrarily keeping one of the items selected. + // TODO(mmalerba): Is this the right behavior? + // Maybe we should just leave an invalid value until the user does something to change it? + if (!this.multiple && newSelection.length > 1) { + newSelection = []; + this._onChange(newSelection); + } + this.selectionModelSubject.next(new SelectionModel(this.multiple, newSelection)); } + } - const valuesArray = coerceArray(values); - for (const value of valuesArray) { - const correspondingOption = this._options.find((option: CdkOption) => { - return option.value != null && this.compareWith(option.value, value); - }); - - if (correspondingOption) { - this.select(correspondingOption); - if (!this.multiple) { - return; - } - } + /** Focus the active option. */ + private _focusActiveOption() { + if (!this.useActiveDescendant) { + this.listKeyManager.activeItem?.focus(); } } } -/** Change event that is being fired whenever the selected state of an option changes. */ -export interface ListboxSelectionChangeEvent { +/** Change event that is fired whenever the value of the listbox changes. */ +export interface ListboxValueChangeEvent { + /** The new value of the listbox. */ + readonly value: T[]; + /** Reference to the listbox that emitted the event. */ - readonly source: CdkListbox; + readonly listbox: CdkListbox; - /** Reference to the option that has been changed. */ + /** Reference to the option that was triggered. */ readonly option: CdkOption; } - -/** Event object emitted by MatOption when selected or deselected. */ -export interface OptionSelectionChangeEvent { - /** Reference to the option that emitted the event. */ - source: CdkOption; - - /** Whether the change in the option's value was a result of a user action. */ - isUserInput: boolean; -} diff --git a/src/cdk/collections/selection-model.ts b/src/cdk/collections/selection-model.ts index 58dd32b4e799..6f7b0e73e330 100644 --- a/src/cdk/collections/selection-model.ts +++ b/src/cdk/collections/selection-model.ts @@ -71,6 +71,17 @@ export class SelectionModel { this._emitChangeEvent(); } + setSelection(...values: T[]): void { + this._verifyValueAssignment(values); + const oldValues = this.selected; + const newSelectedSet = new Set(values); + values.forEach(value => this._markSelected(value)); + oldValues + .filter(value => !newSelectedSet.has(value)) + .forEach(value => this._unmarkSelected(value)); + this._emitChangeEvent(); + } + /** * Toggles a value between selected and deselected. */ diff --git a/src/dev-app/cdk-experimental-listbox/BUILD.bazel b/src/dev-app/cdk-experimental-listbox/BUILD.bazel index 0dbad6c9dd90..78b49e276487 100644 --- a/src/dev-app/cdk-experimental-listbox/BUILD.bazel +++ b/src/dev-app/cdk-experimental-listbox/BUILD.bazel @@ -11,5 +11,6 @@ ng_module( ], deps = [ "//src/cdk-experimental/listbox", + "//src/material/select", ], ) diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html index 27ea35c5f897..f16c89a0069c 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html @@ -1,14 +1,42 @@ +

formControl

    -
  • Apple
  • -
  • Orange
  • -
  • Grapefruit
  • -
  • Peach
  • + [cdkListboxMultiple]="multiSelectable" + [cdkListboxUseActiveDescendant]="activeDescendant" + [formControl]="fruitControl"> +
  • Apple
  • +
  • Orange
  • +
  • Grapefruit
  • +
  • Peach
- +

ngModel

+
    +
  • Apple
  • +
  • Orange
  • +
  • Grapefruit
  • +
  • Peach
  • +
+ +

value binding

+
    +
  • Apple
  • +
  • Orange
  • +
  • Grapefruit
  • +
  • Peach
  • +
+ +

+ Form Control Value: {{fruitControl.value | json}} +

+ +

diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts index 9b90fba5a853..71c00cb4a7bb 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts @@ -6,24 +6,37 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component} from '@angular/core'; +import {ChangeDetectionStrategy, Component, ViewEncapsulation} from '@angular/core'; import {CdkListboxModule} from '@angular/cdk-experimental/listbox'; import {CommonModule} from '@angular/common'; import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; +import {MatSelectModule} from '@angular/material/select'; @Component({ templateUrl: 'cdk-listbox-demo.html', styleUrls: ['cdk-listbox-demo.css'], standalone: true, - imports: [CdkListboxModule, CommonModule, FormsModule, ReactiveFormsModule], + imports: [CdkListboxModule, CommonModule, FormsModule, MatSelectModule, ReactiveFormsModule], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class CdkListboxDemo { multiSelectable = false; activeDescendant = true; - formControl = new FormControl(''); + fruitControl = new FormControl(); - disableForm() { - this.formControl.disable(); + get fruit() { + return this.fruitControl.value; + } + set fruit(value) { + this.fruitControl.patchValue(value); + } + + toggleFormDisabled() { + if (this.fruitControl.disabled) { + this.fruitControl.enable(); + } else { + this.fruitControl.disable(); + } } toggleMultiple() { From b4cd266b1fa457662d193eba9787508016f4cde4 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 19 May 2022 22:36:38 +0000 Subject: [PATCH 02/10] fix(cdk-experimental/listbox): address feedback --- src/cdk-experimental/listbox/listbox.ts | 278 ++++++++++++------ src/cdk/collections/selection-model.ts | 8 +- .../cdk-listbox-demo.css | 3 + .../cdk-listbox-demo.html | 73 +++-- .../cdk-listbox-demo.ts | 39 ++- 5 files changed, 286 insertions(+), 115 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index fd4fbd579abb..8adaaac9fce6 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -13,7 +13,8 @@ import { Directive, ElementRef, forwardRef, - Inject, + inject, + InjectFlags, Input, OnDestroy, Optional, @@ -24,18 +25,16 @@ import {ActiveDescendantKeyManager, Highlightable, ListKeyManagerOption} from '@ import {DOWN_ARROW, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, UP_ARROW} from '@angular/cdk/keycodes'; import {BooleanInput, coerceArray, coerceBooleanProperty} from '@angular/cdk/coercion'; import {SelectionModel} from '@angular/cdk/collections'; -import {BehaviorSubject, defer, merge, Observable, Subject} from 'rxjs'; -import {filter, mapTo, startWith, switchMap, takeUntil} from 'rxjs/operators'; +import {BehaviorSubject, combineLatest, defer, merge, Observable, Subject} from 'rxjs'; +import {filter, mapTo, startWith, switchMap, take, takeUntil} from 'rxjs/operators'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {Directionality} from '@angular/cdk/bidi'; -import {CDK_COMBOBOX, CdkCombobox} from '@angular/cdk-experimental/combobox'; +import {CdkCombobox} from '@angular/cdk-experimental/combobox'; -/** The next id to use for the CdkOption directive. */ -let nextOptionId = 0; - -/** The next id to use for the CdkListbox directive. */ -let nextListboxId = 0; +/** The next id to use for creating unique DOM IDs. */ +let nextId = 0; +/** A selectable option in a listbox. */ @Directive({ selector: '[cdkOption]', exportAs: 'cdkOption', @@ -44,21 +43,21 @@ let nextListboxId = 0; 'class': 'cdk-option', '[id]': 'id', '[attr.aria-selected]': 'isSelected() || null', - '[attr.tabindex]': '!listbox.useActiveDescendant && !disabled ? -1 : null', + '[attr.tabindex]': '_getTabIndex()', '[attr.aria-disabled]': 'disabled', '[class.cdk-option-disabled]': 'disabled', '[class.cdk-option-active]': 'isActive()', '[class.cdk-option-selected]': 'isSelected()', '(click)': '_clicked.next()', + '(focus)': '_handleFocus()', }, }) export class CdkOption implements ListKeyManagerOption, Highlightable, OnDestroy { /** The id of the option's host element. */ - @Input() id = `cdk-option-${nextOptionId++}`; + @Input() id = `cdk-option-${nextId++}`; - // TODO(mmalerba): What do we do if the user doesn't specify a value? /** The value of this option. */ - @Input('cdkOptionValue') value: T; + @Input('cdkOption') value: T; /** * The text used to locate this item during listbox typeahead. If not specified, @@ -76,6 +75,12 @@ export class CdkOption implements ListKeyManagerOption, Highlightab } private _disabled: boolean = false; + /** The option's host element */ + readonly element: HTMLElement = inject(ElementRef).nativeElement; + + /** The parent listbox this option belongs to. */ + protected readonly listbox: CdkListbox = inject(CdkListbox); + /** Emits when the option is destroyed. */ protected destroyed = new Subject(); @@ -85,19 +90,6 @@ export class CdkOption implements ListKeyManagerOption, Highlightab /** Whether the option is currently active. */ private _active = false; - constructor( - /** The option's host element */ - protected readonly elementRef: ElementRef, - /** The change detector ref for this option. */ - protected readonly changeDetectorRef: ChangeDetectorRef, - /** The parent listbox this option belongs to. */ - @Inject(forwardRef(() => CdkListbox)) protected readonly listbox: CdkListbox, - ) { - this.listbox._internalValueChange.pipe(takeUntil(this.destroyed)).subscribe(() => { - this.changeDetectorRef.markForCheck(); - }); - } - ngOnDestroy() { this.destroyed.next(); this.destroyed.complete(); @@ -130,24 +122,46 @@ export class CdkOption implements ListKeyManagerOption, Highlightab /** Focus this option. */ focus() { - this.elementRef.nativeElement.focus(); + this.element.focus(); } /** Get the label for this element which is required by the FocusableOption interface. */ getLabel() { - return (this.typeaheadLabel ?? this.elementRef.nativeElement.textContent?.trim()) || ''; + return (this.typeaheadLabel ?? this.element.textContent?.trim()) || ''; } - /** Set the option as active. */ + /** + * Set the option as active. + * @docs-private + */ setActiveStyles() { this._active = true; - this.changeDetectorRef.markForCheck(); } - /** Set the option as inactive. */ + /** + * Set the option as inactive. + * @docs-private + */ setInactiveStyles() { this._active = false; - this.changeDetectorRef.markForCheck(); + } + + /** Handle focus events on the option. */ + protected _handleFocus() { + // Options can wind up getting focused in active descendant mode if the user clicks on them. + // In this case, we push focus back to the parent listbox to prevent an extra tab stop when + // the user performs a shift+tab. + if (this.listbox.useActiveDescendant) { + this.listbox.focus(); + } + } + + /** Get the tabindex for this option. */ + protected _getTabIndex() { + if (this.listbox.useActiveDescendant || this.disabled) { + return -1; + } + return this.isActive() ? 0 : -1; } } @@ -158,7 +172,7 @@ export class CdkOption implements ListKeyManagerOption, Highlightab 'role': 'listbox', 'class': 'cdk-listbox', '[id]': 'id', - '[attr.tabindex]': 'disabled ? null : 0', + '[attr.tabindex]': '_getTabIndex()', '[attr.aria-disabled]': 'disabled', '[attr.aria-multiselectable]': 'multiple', '[attr.aria-activedescendant]': '_getAriaActiveDescendant()', @@ -177,17 +191,18 @@ export class CdkOption implements ListKeyManagerOption, Highlightab }) export class CdkListbox implements AfterContentInit, OnDestroy, ControlValueAccessor { /** The id of the option's host element. */ - @Input() id = `cdk-listbox-${nextListboxId++}`; + @Input() id = `cdk-listbox-${nextId++}`; - // TODO(mmalerba): Should we normalize the order? Do we care about duplicates? + // TODO(mmalerba): Add forms validation support. /** The value selected in the listbox, represented as an array of option values. */ @Input('cdkListboxValue') get value(): T[] { - return this.selectionModel().selected; + return this._value; } set value(value: T[]) { - this.selectionModel().setSelection(...coerceArray(value == null ? [] : value)); + this._setSelection(value); } + private _value: T[] = []; /** * Whether the listbox allows multiple options to be selected. If the value switches from `true` @@ -199,7 +214,7 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con } set multiple(value: BooleanInput) { this._multiple = coerceBooleanProperty(value); - this._updateSelectionMode(); + this._updateSelectionModel(); } private _multiple: boolean = false; @@ -223,23 +238,19 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con } private _useActiveDescendant: boolean = false; - // TODO(mmalerba): Why do we have this? Shouldn't it depend on whether we're using active descendant? - /** Whether on focus the listbox will focus its active option, default to true. */ - @Input('cdkListboxAutoFocus') - get autoFocus(): boolean { - return this._autoFocus; - } - set autoFocus(shouldAutoFocus: BooleanInput) { - this._autoFocus = coerceBooleanProperty(shouldAutoFocus); - } - private _autoFocus: boolean = true; - /** The orientation of the listbox. Only affects keyboard interaction, not visual layout. */ @Input('cdkListboxOrientation') orientation: 'horizontal' | 'vertical' = 'vertical'; - // TODO(mmalerba): This is currently unused, do we need to do something with it? /** The function used to compare option values. */ - @Input('cdkListboxCompareWith') compareWith: (o1: T, o2: T) => boolean = (a1, a2) => a1 === a2; + @Input('cdkListboxCompareWith') + get compareWith(): undefined | ((o1: T, o2: T) => boolean) { + return this._compareWith; + } + set compareWith(fn: undefined | ((o1: T, o2: T) => boolean)) { + this._compareWith = fn; + this._updateSelectionModel(); + } + private _compareWith?: (o1: T, o2: T) => boolean; /** Emits when the selected value(s) in the listbox change. */ @Output('cdkListboxValueChange') readonly valueChange = new Subject>(); @@ -248,7 +259,9 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con @ContentChildren(CdkOption, {descendants: true}) protected options: QueryList>; /** The selection model used by the listbox. */ - protected selectionModelSubject = new BehaviorSubject(new SelectionModel(this.multiple)); + protected selectionModelSubject = new BehaviorSubject( + new SelectionModel(this.multiple, [], true, this._compareWith), + ); /** The key manager that manages keyboard navigation for this listbox. */ protected listKeyManager: ActiveDescendantKeyManager>; @@ -256,11 +269,11 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con /** Emits when the listbox is destroyed. */ protected readonly destroyed = new Subject(); - /** Emits when the internal value of the listbox changes for any reason. */ - _internalValueChange = this.selectionModelSubject.pipe( - switchMap(selectionModel => selectionModel.changed), - takeUntil(this.destroyed), - ); + /** The host element of the listbox. */ + protected readonly element: HTMLElement = inject(ElementRef).nativeElement; + + /** The change detector for this listbox. */ + protected readonly changeDetectorRef = inject(ChangeDetectorRef); /** Callback called when the listbox has been touched */ private _onTouched: () => void = () => {}; @@ -276,16 +289,27 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con ), ); - constructor( - /** The host element of the listbox. */ - protected elementRef: ElementRef, - // TODO(mmalerba): Should not depend on combobox - @Optional() @Inject(CDK_COMBOBOX) private readonly _combobox: CdkCombobox, - /** The directionality of the page. */ - @Optional() private readonly _dir?: Directionality, - ) {} + /** The directionality of the page. */ + private readonly _dir = inject(Directionality, InjectFlags.Optional); + + // TODO(mmalerba): Should not depend on combobox + private readonly _combobox = inject(CdkCombobox, InjectFlags.Optional); + + constructor() { + this.selectionModelSubject + .pipe( + switchMap(selectionModel => selectionModel.changed), + takeUntil(this.destroyed), + ) + .subscribe(() => { + this._updateInternalValue(); + }); + } ngAfterContentInit() { + if (typeof ngDevMode === 'undefined' || ngDevMode) { + this._verifyNoOptionValueCollisions(); + } this._initKeyManager(); this._combobox?._registerContent(this.id, 'listbox'); this._optionClicked @@ -293,11 +317,7 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con filter(option => !option.disabled), takeUntil(this.destroyed), ) - .subscribe(option => { - this.listKeyManager.setActiveItem(option); - this.triggerOption(option); - this._updatePanelForSelection(option); - }); + .subscribe(option => this._handleOptionClicked(option)); } ngOnDestroy() { @@ -374,9 +394,7 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con * @docs-private */ writeValue(value: T[]): void { - if (this.options) { - this.selectionModel().setSelection(...(value == null ? [] : coerceArray(value))); - } + this._setSelection(value); } /** @@ -388,6 +406,11 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con this.disabled = isDisabled; } + /** Focus the listbox's host element. */ + focus() { + this.element.focus(); + } + /** The selection model used to track the listbox's value. */ protected selectionModel() { return this.selectionModelSubject.value; @@ -402,13 +425,14 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con protected triggerOption(option: CdkOption | null) { if (option && !option.disabled) { let changed = false; - const subscription = this.selectionModel().changed.subscribe(() => (changed = true)); + this.selectionModel() + .changed.pipe(take(1), takeUntil(this.destroyed)) + .subscribe(() => (changed = true)); if (this.multiple) { this.toggle(option); } else { this.select(option); } - subscription.unsubscribe(); if (changed) { this._onChange(this.value); this.valueChange.next({ @@ -422,7 +446,7 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con /** Called when the listbox receives focus. */ protected _handleFocus() { - if (this.autoFocus) { + if (!this.useActiveDescendant) { this.listKeyManager.setActiveItem(this.listKeyManager.activeItem ?? this.options.first); this._focusActiveOption(); } @@ -460,9 +484,8 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con * @param event The focusout event */ protected _handleFocusOut(event: FocusEvent) { - const hostElement = this.elementRef.nativeElement; const otherElement = event.relatedTarget as Element; - if (hostElement !== otherElement && !hostElement.contains(otherElement)) { + if (this.element !== otherElement && !this.element.contains(otherElement)) { this._onTouched(); } } @@ -472,6 +495,14 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con return this._useActiveDescendant ? this.listKeyManager?.activeItem?.id : null; } + /** Get the tabindex for the listbox. */ + protected _getTabIndex() { + if (this.disabled) { + return -1; + } + return this.useActiveDescendant || !this.listKeyManager.activeItem ? 0 : -1; + } + /** Initialize the key manager. */ private _initKeyManager() { this.listKeyManager = new ActiveDescendantKeyManager(this.options) @@ -503,19 +534,15 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con } /** Update the selection mode when the 'multiple' property changes. */ - private _updateSelectionMode() { - if (this.multiple !== this.selectionModel().isMultipleSelection()) { - let newSelection = this.value; - // If we're changing to a single selection model and there are multiple items selected, - // clear the selection rather than arbitrarily keeping one of the items selected. - // TODO(mmalerba): Is this the right behavior? - // Maybe we should just leave an invalid value until the user does something to change it? - if (!this.multiple && newSelection.length > 1) { - newSelection = []; - this._onChange(newSelection); - } - this.selectionModelSubject.next(new SelectionModel(this.multiple, newSelection)); - } + private _updateSelectionModel() { + this.selectionModelSubject.next( + new SelectionModel( + this.multiple, + !this.multiple && this.value.length > 1 ? [] : this.value, + true, + this._compareWith, + ), + ); } /** Focus the active option. */ @@ -523,6 +550,75 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con if (!this.useActiveDescendant) { this.listKeyManager.activeItem?.focus(); } + this.changeDetectorRef.markForCheck(); + } + + /** + * Set the selected values. + * @param value The list of new selected values. + */ + private _setSelection(value: T[]) { + this.selectionModel().setSelection(...(value == null ? [] : coerceArray(value))); + } + + /** Update the internal value of the listbox based on the selection model. */ + private _updateInternalValue() { + const selectionSet = new Set(this.selectionModel().selected); + // Reduce the options list to just the selected values, maintaining their order, + // but removing any duplicate values. + this._value = (this.options ?? []).reduce((result, option) => { + if (selectionSet.has(option.value)) { + result.push(option.value); + selectionSet.delete(option.value); + } + return result; + }, []); + this.changeDetectorRef.markForCheck(); + } + + /** + * Handle the user clicking an option. + * @param option The option that was clicked. + */ + private _handleOptionClicked(option: CdkOption) { + this.listKeyManager.setActiveItem(option); + this.triggerOption(option); + this._updatePanelForSelection(option); + } + + /** Verifies that no two options represent the same value under the compareWith function. */ + private _verifyNoOptionValueCollisions() { + combineLatest([ + this.selectionModelSubject, + this.options.changes.pipe(startWith(this.options)), + ]).subscribe(() => { + const isEqual = this.compareWith ?? Object.is; + const options = [...this.options]; + for (const option of options) { + let duplicate = options.find( + other => option !== other && isEqual(option.value, other.value), + ); + if (duplicate) { + // TODO(mmalerba): Link to docs about this. + if (this.compareWith) { + console.warn( + `Found multiple CdkOption representing the same value under the given compareWith function`, + { + option1: option.element, + option2: duplicate.element, + compareWith: this.compareWith, + }, + ); + } else { + console.warn(`Found multiple CdkOption with the same value`, { + option1: option.element, + option2: duplicate.element, + }); + } + return; + } + } + }); } } diff --git a/src/cdk/collections/selection-model.ts b/src/cdk/collections/selection-model.ts index 6f7b0e73e330..93a25650ba1d 100644 --- a/src/cdk/collections/selection-model.ts +++ b/src/cdk/collections/selection-model.ts @@ -40,6 +40,7 @@ export class SelectionModel { private _multiple = false, initiallySelectedValues?: T[], private _emitChanges = true, + private _compareWith?: (o1: T, o2: T) => boolean, ) { if (initiallySelectedValues && initiallySelectedValues.length) { if (_multiple) { @@ -101,6 +102,9 @@ export class SelectionModel { * Determines whether a value is selected. */ isSelected(value: T): boolean { + if (this._compareWith) { + return [...this._selection].some(v => this._compareWith!(v, value)); + } return this._selection.has(value); } @@ -158,7 +162,9 @@ export class SelectionModel { this._unmarkAll(); } - this._selection.add(value); + if (!this.isSelected(value)) { + this._selection.add(value); + } if (this._emitChanges) { this._selectedToEmit.push(value); diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css index e3175380a68d..8c1c5097c453 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css @@ -1,4 +1,7 @@ .demo-listbox { + display: inline-block; + width: 200px; + margin-right: 10px; list-style-type: none; border: 1px solid black; cursor: default; diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html index f16c89a0069c..c5626b74e1ca 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html @@ -1,43 +1,76 @@

formControl

-
    -
  • Apple
  • -
  • Orange
  • -
  • Grapefruit
  • -
  • Peach
  • +
  • Apple
  • +
  • Orange
  • +
  • Grapefruit
  • +
  • Peach
+

ngModel

-
    -
  • Apple
  • -
  • Orange
  • -
  • Grapefruit
  • -
  • Peach
  • +
  • Apple
  • +
  • Orange
  • +
  • Grapefruit
  • +
  • Peach
+

value binding

-
    -
  • Apple
  • -
  • Orange
  • -
  • Grapefruit
  • -
  • Peach
  • + (cdkListboxValueChange)="fruit = $event.value"> +
  • Apple
  • +
  • Orange
  • +
  • Grapefruit
  • +
  • Peach
+

- Form Control Value: {{fruitControl.value | json}} + Listbox Control Value: {{fruitControl.value | json}} +

+

+ Native Select Control Value: {{nativeFruitControl.value | json}}

- + +
+
- +
- + diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts index 71c00cb4a7bb..f035d27c4472 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts @@ -6,12 +6,23 @@ * found in the LICENSE file at https://angular.io/license */ -import {ChangeDetectionStrategy, Component, ViewEncapsulation} from '@angular/core'; +import { + AfterViewInit, + ChangeDetectionStrategy, + Component, + ViewChild, + ViewEncapsulation, +} from '@angular/core'; import {CdkListboxModule} from '@angular/cdk-experimental/listbox'; import {CommonModule} from '@angular/common'; -import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; +import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms'; import {MatSelectModule} from '@angular/material/select'; +function dumbCompare(o1: string, o2: string) { + const equiv = new Set(['apple', 'orange']); + return o1 === o2 || (equiv.has(o1) && equiv.has(o2)); +} + @Component({ templateUrl: 'cdk-listbox-demo.html', styleUrls: ['cdk-listbox-demo.css'], @@ -22,20 +33,31 @@ import {MatSelectModule} from '@angular/material/select'; export class CdkListboxDemo { multiSelectable = false; activeDescendant = true; + compare?: (o1: string, o2: string) => boolean; fruitControl = new FormControl(); + nativeFruitControl = new FormControl(); get fruit() { return this.fruitControl.value; } set fruit(value) { - this.fruitControl.patchValue(value); + this.fruitControl.setValue(value); + } + + get nativeFruit() { + return this.nativeFruitControl.value; + } + set nativeFruit(value) { + this.nativeFruitControl.setValue(value); } toggleFormDisabled() { if (this.fruitControl.disabled) { this.fruitControl.enable(); + this.nativeFruitControl.enable(); } else { this.fruitControl.disable(); + this.nativeFruitControl.disable(); } } @@ -46,4 +68,15 @@ export class CdkListboxDemo { toggleActiveDescendant() { this.activeDescendant = !this.activeDescendant; } + + toggleDumbCompare() { + this.compare = this.compare ? undefined : dumbCompare; + } + + onNativeFruitChange(event: Event) { + this.nativeFruit = Array.from( + (event.target as HTMLSelectElement).selectedOptions, + option => option.value, + ); + } } From b88f91b4ce640b383ed3cb8e29c69ed0c6144535 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 20 May 2022 02:55:59 +0000 Subject: [PATCH 03/10] fix(cdk-experimental/listbox): add forms validator support --- src/cdk-experimental/listbox/listbox.ts | 96 ++++++++++++++++++- .../cdk-listbox-demo.css | 4 + .../cdk-listbox-demo.html | 13 ++- 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index 8adaaac9fce6..864aa4eab650 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -27,7 +27,16 @@ import {BooleanInput, coerceArray, coerceBooleanProperty} from '@angular/cdk/coe import {SelectionModel} from '@angular/cdk/collections'; import {BehaviorSubject, combineLatest, defer, merge, Observable, Subject} from 'rxjs'; import {filter, mapTo, startWith, switchMap, take, takeUntil} from 'rxjs/operators'; -import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; +import { + AbstractControl, + ControlValueAccessor, + NG_VALIDATORS, + NG_VALUE_ACCESSOR, + ValidationErrors, + Validator, + ValidatorFn, + Validators, +} from '@angular/forms'; import {Directionality} from '@angular/cdk/bidi'; import {CdkCombobox} from '@angular/cdk-experimental/combobox'; @@ -187,13 +196,19 @@ export class CdkOption implements ListKeyManagerOption, Highlightab useExisting: forwardRef(() => CdkListbox), multi: true, }, + { + provide: NG_VALIDATORS, + useExisting: forwardRef(() => CdkListbox), + multi: true, + }, ], }) -export class CdkListbox implements AfterContentInit, OnDestroy, ControlValueAccessor { +export class CdkListbox + implements AfterContentInit, OnDestroy, ControlValueAccessor, Validator +{ /** The id of the option's host element. */ @Input() id = `cdk-listbox-${nextId++}`; - // TODO(mmalerba): Add forms validation support. /** The value selected in the listbox, represented as an array of option values. */ @Input('cdkListboxValue') get value(): T[] { @@ -215,6 +230,7 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con set multiple(value: BooleanInput) { this._multiple = coerceBooleanProperty(value); this._updateSelectionModel(); + this._onValidatorChange(); } private _multiple: boolean = false; @@ -276,11 +292,14 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con protected readonly changeDetectorRef = inject(ChangeDetectorRef); /** Callback called when the listbox has been touched */ - private _onTouched: () => void = () => {}; + private _onTouched: () => {}; /** Callback called when the listbox value changes */ private _onChange: (value: T[]) => void = () => {}; + /** Callback called when the form validator changes. */ + private _onValidatorChange = () => {}; + /** Emits when an option has been clicked. */ private _optionClicked = defer(() => (this.options.changes as Observable[]>).pipe( @@ -295,6 +314,44 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con // TODO(mmalerba): Should not depend on combobox private readonly _combobox = inject(CdkCombobox, InjectFlags.Optional); + /** + * Validator that produces an error if multiple values are selected in a single selection + * listbox. + * @param control The control to validate + * @return A validation error or null + */ + private _validateMultipleValues: ValidatorFn = (control: AbstractControl) => { + const controlValue = this._coerceValue(control.value); + if (!this.multiple && controlValue.length > 1) { + return {'cdkListboxMultipleValues': true}; + } + return null; + }; + + /** + * Validator that produces an error if any selected values are not valid options for this listbox. + * @param control The control to validate + * @return A validation error or null + */ + private _validateInvalidValues: ValidatorFn = (control: AbstractControl) => { + const validValues = (this.options ?? []).map(option => option.value); + const controlValue = this._coerceValue(control.value); + const isEqual = this.compareWith ?? Object.is; + const invalidValues = controlValue.filter( + value => !validValues.some(validValue => isEqual(value, validValue)), + ); + if (invalidValues.length) { + return {'cdkListboxInvalidValues': {'values': invalidValues}}; + } + return null; + }; + + /** The combined set of validators for this listbox. */ + private _validators = Validators.compose([ + this._validateMultipleValues, + this._validateInvalidValues, + ])!; + constructor() { this.selectionModelSubject .pipe( @@ -312,6 +369,9 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con } this._initKeyManager(); this._combobox?._registerContent(this.id, 'listbox'); + this.options.changes.pipe(takeUntil(this.destroyed)).subscribe(() => { + this._onValidatorChange(); + }); this._optionClicked .pipe( filter(option => !option.disabled), @@ -406,6 +466,23 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con this.disabled = isDisabled; } + /** + * Validate the given control + * @docs-private + */ + validate(control: AbstractControl): ValidationErrors | null { + return this._validators(control); + } + + /** + * Registers a callback to be called when the form validator changes. + * @param fn The callback to call + * @docs-private + */ + registerOnValidatorChange(fn: () => void) { + this._onValidatorChange = fn; + } + /** Focus the listbox's host element. */ focus() { this.element.focus(); @@ -558,7 +635,7 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con * @param value The list of new selected values. */ private _setSelection(value: T[]) { - this.selectionModel().setSelection(...(value == null ? [] : coerceArray(value))); + this.selectionModel().setSelection(...this._coerceValue(value)); } /** Update the internal value of the listbox based on the selection model. */ @@ -620,6 +697,15 @@ export class CdkListbox implements AfterContentInit, OnDestroy, Con } }); } + + /** + * Coerces a value into an array representing a listbox selection. + * @param value The value to coerce + * @return An array + */ + private _coerceValue(value: T[]) { + return value == null ? [] : coerceArray(value); + } } /** Change event that is fired whenever the value of the listbox changes. */ diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css index 8c1c5097c453..45d2bc8d5ec3 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.css @@ -15,3 +15,7 @@ .demo-listbox .cdk-option-selected { background: cornflowerblue; } + +.demo-listbox.ng-invalid { + box-shadow: 0 0 0 4px red; +} diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html index c5626b74e1ca..ece38d0bf626 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.html @@ -1,5 +1,6 @@

formControl

-
    formControl
  • Orange
  • Grapefruit
  • Peach
  • +
  • Kiwi
+
+ Errors: {{fruitForm.errors | json}} +

ngModel

-
    ngModel +
    + Errors: {{fruitModel.errors | json}} +

    value binding

      Date: Fri, 20 May 2022 21:28:32 +0000 Subject: [PATCH 04/10] fix(cdk-experimental/listbox): address feedback --- src/cdk-experimental/listbox/listbox.ts | 92 +++++++++++++++++-------- 1 file changed, 63 insertions(+), 29 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index 864aa4eab650..add8c52813ae 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -17,7 +17,6 @@ import { InjectFlags, Input, OnDestroy, - Optional, Output, QueryList, } from '@angular/core'; @@ -211,13 +210,12 @@ export class CdkListbox /** The value selected in the listbox, represented as an array of option values. */ @Input('cdkListboxValue') - get value(): T[] { - return this._value; + get value(): readonly T[] { + return this.selectionModel().selected; } - set value(value: T[]) { + set value(value: readonly T[]) { this._setSelection(value); } - private _value: T[] = []; /** * Whether the listbox allows multiple options to be selected. If the value switches from `true` @@ -274,6 +272,7 @@ export class CdkListbox /** The child options in this listbox. */ @ContentChildren(CdkOption, {descendants: true}) protected options: QueryList>; + // TODO(mmalerba): Refactor SelectionModel so that its not necessary to create new instances /** The selection model used by the listbox. */ protected selectionModelSubject = new BehaviorSubject( new SelectionModel(this.multiple, [], true, this._compareWith), @@ -292,10 +291,10 @@ export class CdkListbox protected readonly changeDetectorRef = inject(ChangeDetectorRef); /** Callback called when the listbox has been touched */ - private _onTouched: () => {}; + private _onTouched = () => {}; /** Callback called when the listbox value changes */ - private _onChange: (value: T[]) => void = () => {}; + private _onChange: (value: readonly T[]) => void = () => {}; /** Callback called when the form validator changes. */ private _onValidatorChange = () => {}; @@ -334,12 +333,8 @@ export class CdkListbox * @return A validation error or null */ private _validateInvalidValues: ValidatorFn = (control: AbstractControl) => { - const validValues = (this.options ?? []).map(option => option.value); const controlValue = this._coerceValue(control.value); - const isEqual = this.compareWith ?? Object.is; - const invalidValues = controlValue.filter( - value => !validValues.some(validValue => isEqual(value, validValue)), - ); + const invalidValues = this._getValuesWithValidity(controlValue, false); if (invalidValues.length) { return {'cdkListboxInvalidValues': {'values': invalidValues}}; } @@ -370,6 +365,7 @@ export class CdkListbox this._initKeyManager(); this._combobox?._registerContent(this.id, 'listbox'); this.options.changes.pipe(takeUntil(this.destroyed)).subscribe(() => { + this._updateInternalValue(); this._onValidatorChange(); }); this._optionClicked @@ -435,7 +431,7 @@ export class CdkListbox * @param fn The callback to register * @docs-private */ - registerOnChange(fn: (value: T[]) => void): void { + registerOnChange(fn: (value: readonly T[]) => void): void { this._onChange = fn; } @@ -453,7 +449,7 @@ export class CdkListbox * @param value The new value of the listbox * @docs-private */ - writeValue(value: T[]): void { + writeValue(value: readonly T[]): void { this._setSelection(value); } @@ -615,7 +611,7 @@ export class CdkListbox this.selectionModelSubject.next( new SelectionModel( this.multiple, - !this.multiple && this.value.length > 1 ? [] : this.value, + !this.multiple && this.value.length > 1 ? [] : this.value.slice(), true, this._compareWith, ), @@ -634,25 +630,49 @@ export class CdkListbox * Set the selected values. * @param value The list of new selected values. */ - private _setSelection(value: T[]) { - this.selectionModel().setSelection(...this._coerceValue(value)); + private _setSelection(value: readonly T[]) { + this.selectionModel().setSelection( + ...this._getValuesWithValidity(this._coerceValue(value), true), + ); } /** Update the internal value of the listbox based on the selection model. */ private _updateInternalValue() { - const selectionSet = new Set(this.selectionModel().selected); - // Reduce the options list to just the selected values, maintaining their order, - // but removing any duplicate values. - this._value = (this.options ?? []).reduce((result, option) => { - if (selectionSet.has(option.value)) { - result.push(option.value); - selectionSet.delete(option.value); - } - return result; - }, []); + const options = [...this.options]; + const indexCache = new Map(); + // Check if we need to remove any values due to them becoming invalid + // (e.g. if the option was removed from the DOM.) + const selected = this.selectionModel().selected; + const validSelected = this._getValuesWithValidity(selected, true); + if (validSelected.length != selected.length) { + this.selectionModel().setSelection(...validSelected); + } + this.selectionModel().sort((a: T, b: T) => { + const aIndex = this._getIndexForValue(indexCache, options, a); + const bIndex = this._getIndexForValue(indexCache, options, b); + return aIndex - bIndex; + }); this.changeDetectorRef.markForCheck(); } + /** + * Gets the index of the given value in the given list of options. + * @param cache The cache of indices found so far + * @param options The list of options to search in + * @param value The value to find + * @return The index of the value in the options list + */ + private _getIndexForValue(cache: Map, options: CdkOption[], value: T) { + const isEqual = this.compareWith || Object.is; + if (!cache.has(value)) { + cache.set( + value, + options.findIndex(option => isEqual(value, option.value)), + ); + } + return cache.get(value)!; + } + /** * Handle the user clicking an option. * @param option The option that was clicked. @@ -703,15 +723,29 @@ export class CdkListbox * @param value The value to coerce * @return An array */ - private _coerceValue(value: T[]) { + private _coerceValue(value: readonly T[]) { return value == null ? [] : coerceArray(value); } + + /** + * Get the sublist of values with the given validity. + * @param values The list of values + * @param valid Whether to get valid values + * @return The sublist of values with the requested validity + */ + private _getValuesWithValidity(values: readonly T[], valid: boolean) { + const isEqual = this.compareWith || Object.is; + const validValues = (this.options || []).map(option => option.value); + return values.filter( + value => valid === validValues.some(validValue => isEqual(value, validValue)), + ); + } } /** Change event that is fired whenever the value of the listbox changes. */ export interface ListboxValueChangeEvent { /** The new value of the listbox. */ - readonly value: T[]; + readonly value: readonly T[]; /** Reference to the listbox that emitted the event. */ readonly listbox: CdkListbox; From 5bab4c2f4aba296f01f1e1b0647f7fc07329fc37 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 20 May 2022 22:31:57 +0000 Subject: [PATCH 05/10] fix(cdk-experimental/listbox): address feedback --- src/cdk-experimental/listbox/listbox.ts | 36 ++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index add8c52813ae..6e73f65b06ff 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -386,24 +386,48 @@ export class CdkListbox * Toggle the selected state of the given option. * @param option The option to toggle */ - toggle(option: CdkOption | T) { - this.selectionModel().toggle(option instanceof CdkOption ? option.value : option); + toggle(option: CdkOption) { + this.toggleValue(option.value); + } + + /** + * Toggle the selected state of the given value. + * @param value The value to toggle + */ + toggleValue(value: T) { + this.selectionModel().toggle(value); } /** * Select the given option. * @param option The option to select */ - select(option: CdkOption | T) { - this.selectionModel().select(option instanceof CdkOption ? option.value : option); + select(option: CdkOption) { + this.selectValue(option.value); + } + + /** + * Select the given value. + * @param value The value to select + */ + selectValue(value: T) { + this.selectionModel().select(value); } /** * Deselect the given option. * @param option The option to deselect */ - deselect(option: CdkOption | T) { - this.selectionModel().deselect(option instanceof CdkOption ? option.value : option); + deselect(option: CdkOption) { + this.deselectValue(option.value); + } + + /** + * Deselect the given value. + * @param value The value to deselect + */ + deselectValue(value: T) { + this.selectionModel().deselect(value); } /** From 53e36f79a6f19e1dcad0f853f93607857293597b Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Mon, 23 May 2022 16:47:02 +0000 Subject: [PATCH 06/10] fix(cdk-experimental/listbox): address feedback --- src/cdk-experimental/listbox/listbox.ts | 33 ++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index 6e73f65b06ff..10397fed1176 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -22,7 +22,12 @@ import { } from '@angular/core'; import {ActiveDescendantKeyManager, Highlightable, ListKeyManagerOption} from '@angular/cdk/a11y'; import {DOWN_ARROW, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, UP_ARROW} from '@angular/cdk/keycodes'; -import {BooleanInput, coerceArray, coerceBooleanProperty} from '@angular/cdk/coercion'; +import { + BooleanInput, + coerceArray, + coerceBooleanProperty, + coerceNumberProperty, +} from '@angular/cdk/coercion'; import {SelectionModel} from '@angular/cdk/collections'; import {BehaviorSubject, combineLatest, defer, merge, Observable, Subject} from 'rxjs'; import {filter, mapTo, startWith, switchMap, take, takeUntil} from 'rxjs/operators'; @@ -83,6 +88,18 @@ export class CdkOption implements ListKeyManagerOption, Highlightab } private _disabled: boolean = false; + /** The tabindex of the option when it is enabled. */ + @Input('tabindex') + get enabledTabIndex() { + return this._enabledTabIndex === undefined + ? this.listbox.enabledTabIndex + : this._enabledTabIndex; + } + set enabledTabIndex(value) { + this._enabledTabIndex = value; + } + private _enabledTabIndex?: number | null; + /** The option's host element */ readonly element: HTMLElement = inject(ElementRef).nativeElement; @@ -169,7 +186,7 @@ export class CdkOption implements ListKeyManagerOption, Highlightab if (this.listbox.useActiveDescendant || this.disabled) { return -1; } - return this.isActive() ? 0 : -1; + return this.isActive() ? this.enabledTabIndex : -1; } } @@ -208,6 +225,16 @@ export class CdkListbox /** The id of the option's host element. */ @Input() id = `cdk-listbox-${nextId++}`; + /** The tabindex to use when the listbox is enabled. */ + @Input('tabindex') + get enabledTabIndex() { + return this._enabledTabIndex === undefined ? 0 : this._enabledTabIndex; + } + set enabledTabIndex(value) { + this._enabledTabIndex = value; + } + private _enabledTabIndex?: number | null; + /** The value selected in the listbox, represented as an array of option values. */ @Input('cdkListboxValue') get value(): readonly T[] { @@ -597,7 +624,7 @@ export class CdkListbox if (this.disabled) { return -1; } - return this.useActiveDescendant || !this.listKeyManager.activeItem ? 0 : -1; + return this.useActiveDescendant || !this.listKeyManager.activeItem ? this.enabledTabIndex : -1; } /** Initialize the key manager. */ From f5e1f869540a55a9e7dda0f0ff3df86d38dfb1a2 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 24 May 2022 21:16:53 +0000 Subject: [PATCH 07/10] fix(cdk-experimental/listbox): fix tests --- src/cdk-experimental/listbox/BUILD.bazel | 1 + src/cdk-experimental/listbox/listbox.spec.ts | 1249 +++++++----------- src/cdk-experimental/listbox/listbox.ts | 47 +- 3 files changed, 546 insertions(+), 751 deletions(-) diff --git a/src/cdk-experimental/listbox/BUILD.bazel b/src/cdk-experimental/listbox/BUILD.bazel index 8a3678e929f1..52498a8b7f3d 100644 --- a/src/cdk-experimental/listbox/BUILD.bazel +++ b/src/cdk-experimental/listbox/BUILD.bazel @@ -28,6 +28,7 @@ ng_test_library( "//src/cdk-experimental/combobox", "//src/cdk/keycodes", "//src/cdk/testing/private", + "@npm//@angular/common", "@npm//@angular/forms", "@npm//@angular/platform-browser", ], diff --git a/src/cdk-experimental/listbox/listbox.spec.ts b/src/cdk-experimental/listbox/listbox.spec.ts index 6a541bfa6d44..a9694f49a4ea 100644 --- a/src/cdk-experimental/listbox/listbox.spec.ts +++ b/src/cdk-experimental/listbox/listbox.spec.ts @@ -1,881 +1,678 @@ -import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing'; -import {Component, DebugElement, ViewChild} from '@angular/core'; +import {fakeAsync, TestBed, tick} from '@angular/core/testing'; +import {Component, Type} from '@angular/core'; import {By} from '@angular/platform-browser'; import {CdkListbox, CdkListboxModule, CdkOption, ListboxValueChangeEvent} from './index'; -import { - createKeyboardEvent, - dispatchKeyboardEvent, - dispatchMouseEvent, -} from '../../cdk/testing/private'; -import {A, DOWN_ARROW, END, HOME, SPACE} from '@angular/cdk/keycodes'; -import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; -import {CdkCombobox, CdkComboboxModule} from '@angular/cdk-experimental/combobox'; +import {dispatchKeyboardEvent, dispatchMouseEvent} from '../../cdk/testing/private'; +import {B, DOWN_ARROW, END, HOME, SPACE, UP_ARROW} from '@angular/cdk/keycodes'; +import {FormControl, ReactiveFormsModule} from '@angular/forms'; +import {CommonModule} from '@angular/common'; + +async function setupComponent(component: Type, imports: any[] = []) { + await TestBed.configureTestingModule({ + imports: [CdkListboxModule, ...imports], + declarations: [component], + }).compileComponents(); + const fixture = TestBed.createComponent(component); + fixture.detectChanges(); + + const listboxDebugEl = fixture.debugElement.query(By.directive(CdkListbox)); + const optionDebugEls = fixture.debugElement.queryAll(By.directive(CdkOption)); + + return { + fixture, + testComponent: fixture.componentInstance, + listbox: listboxDebugEl.injector.get>(CdkListbox), + listboxEl: listboxDebugEl.nativeElement as HTMLElement, + options: optionDebugEls.map(el => el.injector.get>(CdkOption)), + optionEls: optionDebugEls.map(el => el.nativeElement as HTMLElement), + }; +} describe('CdkOption and CdkListbox', () => { - describe('selection state change', () => { - let fixture: ComponentFixture; - - let testComponent: ListboxWithOptions; - - let listbox: DebugElement; - let listboxInstance: CdkListbox; - let listboxElement: HTMLElement; - - let options: DebugElement[]; - let optionInstances: CdkOption[]; - let optionElements: HTMLElement[]; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkListboxModule], - declarations: [ListboxWithOptions], - }).compileComponents(); - })); - - beforeEach(waitForAsync(() => { - fixture = TestBed.createComponent(ListboxWithOptions); - fixture.detectChanges(); - - testComponent = fixture.debugElement.componentInstance; - - listbox = fixture.debugElement.query(By.directive(CdkListbox)); - listboxInstance = listbox.injector.get>(CdkListbox); - listboxElement = listbox.nativeElement; - - options = fixture.debugElement.queryAll(By.directive(CdkOption)); - optionInstances = options.map(o => o.injector.get(CdkOption)); - optionElements = options.map(o => o.nativeElement); - })); - - it('should generate a unique optionId for each option', () => { - let optionIds: string[] = []; - for (const instance of optionInstances) { - expect(optionIds.indexOf(instance.id)).toBe(-1); - optionIds.push(instance.id); - - expect(instance.id).toMatch(/cdk-option-\d+/); + describe('id', () => { + it('should generate unique ids', async () => { + const {listbox, listboxEl, options, optionEls} = await setupComponent(ListboxWithOptions); + const optionIds = new Set(optionEls.map(option => option.id)); + expect(optionIds.size).toBe(options.length); + for (let i = 0; i < options.length; i++) { + expect(options[i].id).toBe(optionEls[i].id); + expect(options[i].id).toMatch(/cdk-option-\d+/); } + expect(listbox.id).toEqual(listboxEl.id); + expect(listbox.id).toMatch(/cdk-listbox-\d+/); }); - it('should have set the selected input of the options to null by default', () => { - for (const option of optionElements) { - expect(option.hasAttribute('aria-selected')).toBeFalse(); - } - }); - - it('should update aria-selected when selected is changed programmatically', () => { - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - optionInstances[1].selected = true; + it('should not overwrite user given ids', async () => { + const {testComponent, fixture, listboxEl, optionEls} = await setupComponent( + ListboxWithOptions, + ); + testComponent.listboxId = 'my-listbox'; + testComponent.appleId = 'my-apple'; fixture.detectChanges(); - - expect(optionElements[1].getAttribute('aria-selected')).toBe('true'); + expect(listboxEl.id).toBe('my-listbox'); + expect(optionEls[0].id).toBe('my-apple'); }); + }); - it('should update selected option on click event', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + describe('tabindex', () => { + it('should use tabindex=0 for focusable elements, tabindex=-1 for non-focusable elements', async () => { + const {fixture, listbox, listboxEl, optionEls} = await setupComponent(ListboxWithOptions); + expect(listboxEl.getAttribute('tabindex')).toBe('0'); + expect(optionEls[0].getAttribute('tabindex')).toBe('-1'); - dispatchMouseEvent(optionElements[0], 'click'); + listbox.focus(); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption).toBeDefined(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); + expect(listboxEl.getAttribute('tabindex')).toBe('-1'); + expect(optionEls[0].getAttribute('tabindex')).toBe('0'); }); - it('should update selected option on space or enter key press', () => { - let selectedOptions = optionInstances.filter(option => option.selected); + it('should respect user given tabindex for focusable elements', async () => { + const {testComponent, fixture, listbox, listboxEl, optionEls} = await setupComponent( + ListboxWithOptions, + ); + testComponent.listboxTabindex = 10; + testComponent.appleTabindex = 20; + fixture.detectChanges(); - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + expect(listboxEl.getAttribute('tabindex')).toBe('10'); + expect(optionEls[0].getAttribute('tabindex')).toBe('-1'); - listboxInstance.setActiveOption(optionInstances[0]); - dispatchKeyboardEvent(listboxElement, 'keydown', SPACE); + listbox.focus(); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption).toBeDefined(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); + expect(listboxEl.getAttribute('tabindex')).toBe('-1'); + expect(optionEls[0].getAttribute('tabindex')).toBe('20'); }); - it('should update active option on home and end key press', () => { - listboxInstance.setActiveOption(optionInstances[1]); + it('should use listbox tabindex for focusable options', async () => { + const {testComponent, fixture, listbox, optionEls} = await setupComponent(ListboxWithOptions); + testComponent.listboxTabindex = 10; fixture.detectChanges(); - expect(listboxInstance.listKeyManager.activeItem).toBe(optionInstances[1]); + expect(optionEls[0].getAttribute('tabindex')).toBe('-1'); - dispatchKeyboardEvent(listboxElement, 'keydown', HOME); + listbox.focus(); fixture.detectChanges(); - expect(listboxInstance.listKeyManager.activeItem).toBe(optionInstances[0]); + expect(optionEls[0].getAttribute('tabindex')).toBe('10'); + }); + }); + + describe('selection', () => { + it('should be empty initially', async () => { + const {fixture, listbox, options, optionEls} = await setupComponent(ListboxWithOptions); + expect(listbox.value).toEqual([]); + for (let i = 0; i < options.length; i++) { + expect(options[i].isSelected()).toBeFalse(); + expect(optionEls[i].hasAttribute('aria-selected')).toBeFalse(); + } + expect(fixture.componentInstance.changedOption).toBeUndefined(); + }); - dispatchKeyboardEvent(listboxElement, 'keydown', END); + it('should update when selection is changed programmatically', async () => { + const {fixture, listbox, options, optionEls} = await setupComponent(ListboxWithOptions); + options[1].select(); fixture.detectChanges(); - expect(listboxInstance.listKeyManager.activeItem).toBe(optionInstances[3]); + expect(listbox.value).toEqual(['orange']); + expect(options[1].isSelected()).toBeTrue(); + expect(optionEls[1].getAttribute('aria-selected')).toBe('true'); + expect(fixture.componentInstance.changedOption).toBeUndefined(); }); - it('should be able to toggle listbox disabled state', () => { - expect(listboxInstance.disabled).toBeFalse(); - expect(listboxElement.getAttribute('aria-disabled')).toBe('false'); - - testComponent.isListboxDisabled = true; + it('should update on option clicked', async () => { + const {fixture, listbox, options, optionEls} = await setupComponent(ListboxWithOptions); + optionEls[0].click(); fixture.detectChanges(); - expect(listboxInstance.disabled).toBeTrue(); - expect(listboxElement.getAttribute('aria-disabled')).toBe('true'); + expect(listbox.value).toEqual(['apple']); + expect(options[0].isSelected()).toBeTrue(); + expect(optionEls[0].getAttribute('aria-selected')).toBe('true'); + expect(fixture.componentInstance.changedOption?.id).toBe(options[0].id); }); - it('should toggle option disabled state', () => { - expect(optionInstances[0].disabled).toBeFalse(); - expect(optionElements[0].getAttribute('aria-disabled')).toBe('false'); - - testComponent.isPurpleDisabled = true; + it('should update on option activated via keyboard', async () => { + const {fixture, listbox, listboxEl, options, optionEls} = await setupComponent( + ListboxWithOptions, + ); + listbox.focus(); + dispatchKeyboardEvent(listboxEl, 'keydown', SPACE); fixture.detectChanges(); - expect(optionInstances[0].disabled).toBeTrue(); - expect(optionElements[0].getAttribute('aria-disabled')).toBe('true'); + expect(listbox.value).toEqual(['apple']); + expect(options[0].isSelected()).toBeTrue(); + expect(optionEls[0].getAttribute('aria-selected')).toBe('true'); + expect(fixture.componentInstance.changedOption?.id).toBe(options[0].id); }); - it('should toggle option aria-disabled state on listbox disabled state change', () => { - optionInstances[0].disabled = true; + it('should deselect previously selected option in single-select listbox', async () => { + const {fixture, listbox, options, optionEls} = await setupComponent(ListboxWithOptions); + dispatchMouseEvent(optionEls[0], 'click'); fixture.detectChanges(); - expect(listboxInstance.disabled).toBeFalse(); - expect(optionInstances[0].disabled).toBeTrue(); - expect(optionElements[0].hasAttribute('tabindex')).toBeFalse(); - expect(optionElements[1].getAttribute('aria-disabled')).toBe('false'); - expect(optionElements[1].getAttribute('tabindex')).toBe('-1'); + expect(listbox.value).toEqual(['apple']); + expect(options[0].isSelected()).toBeTrue(); - testComponent.isListboxDisabled = true; + dispatchMouseEvent(optionEls[2], 'click'); fixture.detectChanges(); - expect(listboxInstance.disabled).toBeTrue(); - expect(optionInstances[0].disabled).toBeTrue(); - expect(optionElements[0].hasAttribute('tabindex')).toBeFalse(); - expect(optionElements[1].getAttribute('aria-disabled')).toBe('true'); - expect(optionElements[1].hasAttribute('tabindex')).toBeFalse(); + expect(listbox.value).toEqual(['banana']); + expect(options[0].isSelected()).toBeFalse(); }); - it('should not toggle selected on click of a disabled option', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); - - testComponent.isPurpleDisabled = true; + it('should select all options programmatically in multi-select listbox', async () => { + const {testComponent, fixture, listbox} = await setupComponent(ListboxWithOptions); + testComponent.isMultiselectable = true; fixture.detectChanges(); - dispatchMouseEvent(optionElements[0], 'click'); + + listbox.setAllSelected(true); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + expect(listbox.value).toEqual(['apple', 'orange', 'banana', 'peach']); }); - it('should not toggle selected on click in a disabled listbox', () => { - let selectedOptions = optionInstances.filter(option => option.selected); + it('should add to selection in multi-select listbox', async () => { + const {testComponent, fixture, listbox, options, optionEls} = await setupComponent( + ListboxWithOptions, + ); + testComponent.isMultiselectable = true; + optionEls[0].click(); + fixture.detectChanges(); - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + expect(listbox.value).toEqual(['apple']); + expect(options[0].isSelected()).toBeTrue(); - testComponent.isListboxDisabled = true; - fixture.detectChanges(); - dispatchMouseEvent(optionElements[0], 'click'); + optionEls[2].click(); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + expect(listbox.value).toEqual(['apple', 'banana']); + expect(options[0].isSelected()).toBeTrue(); }); - it('should change active item using type ahead', fakeAsync(() => { - expect(listboxInstance.listKeyManager.activeItem).toBeNull(); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); - - dispatchKeyboardEvent(listboxElement, 'keydown', A); + it('should deselect all options when switching to single-selection with invalid selection', async () => { + const {testComponent, fixture, listbox} = await setupComponent(ListboxWithOptions); + testComponent.isMultiselectable = true; fixture.detectChanges(); - tick(200); - - expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[2]); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(2); - })); - - it('should not handle space or enter on a disabled listbox', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - - expect(selectedOptions.length).toBe(0); - expect(fixture.componentInstance.changedOption).toBeUndefined(); - - listboxInstance.setActiveOption(optionInstances[0]); - testComponent.isListboxDisabled = true; + listbox.setAllSelected(true); fixture.detectChanges(); - dispatchKeyboardEvent(listboxElement, 'keydown', SPACE); + expect(listbox.value).toEqual(['apple', 'orange', 'banana', 'peach']); + + testComponent.isMultiselectable = false; fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(0); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + expect(listbox.value).toEqual([]); }); - it('should not handle type ahead on a disabled listbox', fakeAsync(() => { - expect(listboxInstance.listKeyManager.activeItem).toBeNull(); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); - - testComponent.isListboxDisabled = true; + it('should preserve selection when switching to single-selection with valid selection', async () => { + const {testComponent, fixture, listbox, optionEls} = await setupComponent(ListboxWithOptions); + testComponent.isMultiselectable = true; fixture.detectChanges(); - - dispatchKeyboardEvent(listboxElement, 'keydown', A); + optionEls[0].click(); fixture.detectChanges(); - tick(200); - - expect(listboxInstance.listKeyManager.activeItem).toBeNull(); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); - })); - it('should not select a disabled option using space or enter', () => { - let selectedOptions = optionInstances.filter(option => option.selected); + expect(listbox.value).toEqual(['apple']); - expect(selectedOptions.length).toBe(0); - expect(fixture.componentInstance.changedOption).toBeUndefined(); - - listboxInstance.setActiveOption(optionInstances[0]); - testComponent.isPurpleDisabled = true; - fixture.detectChanges(); - - dispatchKeyboardEvent(listboxElement, 'keydown', SPACE); + testComponent.isMultiselectable = false; fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(0); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + expect(listbox.value).toEqual(['apple']); }); - it('should update active item upon arrow key presses', () => { - expect(listboxInstance.listKeyManager.activeItem).toBeNull(); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); + it('should allow programmatically toggling options', async () => { + const {testComponent, fixture, listbox, options} = await setupComponent(ListboxWithOptions); + testComponent.isMultiselectable = true; + fixture.detectChanges(); - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); + options[0].toggle(); + listbox.toggle(options[1]); fixture.detectChanges(); - expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[0]); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(0); + expect(options[0].isSelected()).toBeTrue(); + expect(options[1].isSelected()).toBeTrue(); - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); + options[0].toggle(); + listbox.toggle(options[1]); fixture.detectChanges(); - expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[1]); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(1); + expect(options[0].isSelected()).toBeFalse(); + expect(options[1].isSelected()).toBeFalse(); }); - it('should skip disabled options when navigating with arrow keys', () => { - expect(listboxInstance.listKeyManager.activeItem).toBeNull(); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(-1); + it('should allow programmatically selecting and deselecting options', async () => { + const {testComponent, fixture, listbox, options} = await setupComponent(ListboxWithOptions); + testComponent.isMultiselectable = true; + fixture.detectChanges(); - testComponent.isSolarDisabled = true; - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); + options[0].select(); + listbox.select(options[1]); fixture.detectChanges(); - expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[0]); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(0); + expect(options[0].isSelected()).toBeTrue(); + expect(options[1].isSelected()).toBeTrue(); - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); + options[0].deselect(); + listbox.deselect(options[1]); fixture.detectChanges(); - expect(listboxInstance.listKeyManager.activeItem).toEqual(optionInstances[2]); - expect(listboxInstance.listKeyManager.activeItemIndex).toBe(2); + expect(options[0].isSelected()).toBeFalse(); + expect(options[1].isSelected()).toBeFalse(); }); - it('should update selected option on click event', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + // TODO(mmalerba): Fix this case. + // Currently banana gets booted because the option isn't loaded yet, + // but then when the option loads the value is already lost. + // it('should allow binding to listbox value', async () => { + // const {testComponent, fixture, listbox, options} = await setupComponent(ListboxWithBoundValue); + // expect(listbox.value).toEqual(['banana']); + // expect(options[2].isSelected()).toBeTrue(); + // + // testComponent.value = ['orange']; + // fixture.detectChanges(); + // + // expect(listbox.value).toEqual(['orange']); + // expect(options[1].isSelected()).toBeTrue(); + // }); + }); - dispatchMouseEvent(optionElements[0], 'click'); + describe('disabled state', () => { + it('should be able to toggle listbox disabled state', async () => { + const {fixture, testComponent, listbox, listboxEl, options, optionEls} = await setupComponent( + ListboxWithOptions, + ); + testComponent.isListboxDisabled = true; fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption).toBeDefined(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); - }); - - it('should focus and toggle the next item when pressing SHIFT + DOWN_ARROW', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - const downKeyEvent = createKeyboardEvent('keydown', DOWN_ARROW, undefined, {shift: true}); + expect(listbox.disabled).toBeTrue(); + expect(listboxEl.getAttribute('aria-disabled')).toBe('true'); - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); + for (let i = 0; i < options.length; i++) { + expect(options[i].disabled).toBeTrue(); + expect(optionEls[i].getAttribute('aria-disabled')).toBe('true'); + } + }); - listboxInstance.setActiveOption(optionInstances[0]); - listboxInstance._keydown(downKeyEvent); + it('should toggle option disabled state', async () => { + const {fixture, testComponent, options, optionEls} = await setupComponent(ListboxWithOptions); + testComponent.isAppleDisabled = true; fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[1].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[1].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption).toBeDefined(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[1].id); + expect(options[0].disabled).toBeTrue(); + expect(optionEls[0].getAttribute('aria-disabled')).toBe('true'); }); - }); - - describe('with multiple selection', () => { - let fixture: ComponentFixture; - let testComponent: ListboxMultiselect; - let listbox: DebugElement; - let listboxInstance: CdkListbox; - - let options: DebugElement[]; - let optionInstances: CdkOption[]; - let optionElements: HTMLElement[]; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkListboxModule], - declarations: [ListboxMultiselect], - }).compileComponents(); - })); - - beforeEach(waitForAsync(() => { - fixture = TestBed.createComponent(ListboxMultiselect); + it('should not change selection on click of a disabled option', async () => { + const {fixture, testComponent, listbox, optionEls} = await setupComponent(ListboxWithOptions); + testComponent.isAppleDisabled = true; fixture.detectChanges(); - testComponent = fixture.debugElement.componentInstance; - listbox = fixture.debugElement.query(By.directive(CdkListbox)); - listboxInstance = listbox.injector.get>(CdkListbox); - - options = fixture.debugElement.queryAll(By.directive(CdkOption)); - optionInstances = options.map(o => o.injector.get(CdkOption)); - optionElements = options.map(o => o.nativeElement); - })); - - it('should select all options using the select all method', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - testComponent.isMultiselectable = true; + optionEls[0].click(); fixture.detectChanges(); - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); + expect(listbox.value).toEqual([]); expect(fixture.componentInstance.changedOption).toBeUndefined(); - - listboxInstance.setAllSelected(true); - fixture.detectChanges(); - - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(4); - - for (const option of optionElements) { - expect(option.getAttribute('aria-selected')).toBe('true'); - } - - expect(fixture.componentInstance.changedOption).toBeDefined(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[3].id); }); - it('should deselect previously selected when multiple is false', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption).toBeUndefined(); - - dispatchMouseEvent(optionElements[0], 'click'); + it('should not change selection on click in a disabled listbox', async () => { + const {fixture, testComponent, listbox, optionEls} = await setupComponent(ListboxWithOptions); + testComponent.isListboxDisabled = true; fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); - - dispatchMouseEvent(optionElements[2], 'click'); + optionEls[0].click(); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(optionElements[2].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[2].selected).toBeTrue(); - - /** Expect first option to be most recently changed because it was deselected. */ - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); - }); - - it('should allow multiple selection when multiple is true', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - testComponent.isMultiselectable = true; - - expect(selectedOptions.length).toBe(0); + expect(listbox.value).toEqual([]); expect(fixture.componentInstance.changedOption).toBeUndefined(); + }); - dispatchMouseEvent(optionElements[0], 'click'); + it('should not change selection on keyboard activation in a disabled listbox', async () => { + const {fixture, testComponent, listbox, listboxEl} = await setupComponent(ListboxWithOptions); + listbox.focus(); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); - - dispatchMouseEvent(optionElements[2], 'click'); + testComponent.isListboxDisabled = true; fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(2); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(optionElements[2].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[2].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[2].id); - }); - - it('should deselect all options when multiple switches to false', () => { - let selectedOptions = optionInstances.filter(option => option.selected); - testComponent.isMultiselectable = true; + dispatchKeyboardEvent(listboxEl, 'keydown', SPACE); + fixture.detectChanges(); - expect(selectedOptions.length).toBe(0); + expect(listbox.value).toEqual([]); expect(fixture.componentInstance.changedOption).toBeUndefined(); + }); - dispatchMouseEvent(optionElements[0], 'click'); + it('should not change selection on click of a disabled option', async () => { + const {fixture, testComponent, listbox, listboxEl, options} = await setupComponent( + ListboxWithOptions, + ); + listbox.focus(); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(1); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); + testComponent.isAppleDisabled = true; + fixture.detectChanges(); - testComponent.isMultiselectable = false; + dispatchKeyboardEvent(listboxEl, 'keydown', SPACE); fixture.detectChanges(); - selectedOptions = optionInstances.filter(option => option.selected); - expect(selectedOptions.length).toBe(0); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[0].selected).toBeFalse(); - expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id); + expect(listbox.value).toEqual([]); + expect(fixture.componentInstance.changedOption).toBeUndefined(); }); - }); - describe('with aria active descendant', () => { - let fixture: ComponentFixture; - - let testComponent: ListboxActiveDescendant; - - let listbox: DebugElement; - let listboxInstance: CdkListbox; - let listboxElement: HTMLElement; + it('should not handle type ahead on a disabled listbox', fakeAsync(async () => { + const {fixture, testComponent, listboxEl, options} = await setupComponent(ListboxWithOptions); + testComponent.isListboxDisabled = true; + fixture.detectChanges(); - let options: DebugElement[]; - let optionInstances: CdkOption[]; - let optionElements: HTMLElement[]; + dispatchKeyboardEvent(listboxEl, 'keydown', B); + fixture.detectChanges(); + tick(200); - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkListboxModule], - declarations: [ListboxActiveDescendant], - }).compileComponents(); + for (let option of options) { + expect(option.isActive()).toBeFalse(); + } })); - beforeEach(waitForAsync(() => { - fixture = TestBed.createComponent(ListboxActiveDescendant); + it('should skip disabled options when navigating with arrow keys', async () => { + const {testComponent, fixture, listbox, listboxEl, options} = await setupComponent( + ListboxWithOptions, + ); + testComponent.isOrangeDisabled = true; + listbox.focus(); fixture.detectChanges(); - testComponent = fixture.debugElement.componentInstance; + expect(options[0].isActive()).toBeTrue(); - listbox = fixture.debugElement.query(By.directive(CdkListbox)); - listboxInstance = listbox.injector.get>(CdkListbox); - listboxElement = listbox.nativeElement; - - options = fixture.debugElement.queryAll(By.directive(CdkOption)); - optionInstances = options.map(o => o.injector.get(CdkOption)); - optionElements = options.map(o => o.nativeElement); - })); + dispatchKeyboardEvent(listboxEl, 'keydown', DOWN_ARROW); + fixture.detectChanges(); - it('should update aria active descendant when enabled', () => { - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse(); + expect(options[2].isActive()).toBeTrue(); + }); + }); - listboxInstance.setActiveOption(optionInstances[0]); + describe('compare with', () => { + it('should allow custom function to compare option values', async () => { + const {fixture, listbox, options} = await setupComponent< + ListboxWithObjectValues, + {name: string} + >(ListboxWithObjectValues, [CommonModule]); + listbox.value = [{name: 'Banana'}]; fixture.detectChanges(); - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue(); - expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[0].id); + expect(options[2].isSelected()).toBeTrue(); - listboxInstance.setActiveOption(optionInstances[2]); + listbox.value = [{name: 'Orange', extraStuff: true} as any]; fixture.detectChanges(); - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue(); - expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[2].id); + expect(options[1].isSelected()).toBeTrue(); }); + }); - it('should update aria active descendant via arrow keys', () => { - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse(); - - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); + describe('keyboard navigation', () => { + it('should update active item on arrow key presses', async () => { + const {fixture, listbox, listboxEl, options} = await setupComponent(ListboxWithOptions); + listbox.focus(); + dispatchKeyboardEvent(listboxEl, 'keydown', DOWN_ARROW); fixture.detectChanges(); - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue(); - expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[0].id); + expect(options[1].isActive()).toBeTrue(); - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); + dispatchKeyboardEvent(listboxEl, 'keydown', UP_ARROW); fixture.detectChanges(); - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue(); - expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[1].id); + expect(options[0].isActive()).toBeTrue(); }); - it('should place focus on options and not set active descendant', () => { - testComponent.isActiveDescendant = false; + it('should update active option on home and end key press', async () => { + const {fixture, listbox, listboxEl, options, optionEls} = await setupComponent( + ListboxWithOptions, + ); + listbox.focus(); + dispatchKeyboardEvent(listboxEl, 'keydown', END); fixture.detectChanges(); - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse(); + expect(options[options.length - 1].isActive()).toBeTrue(); + expect(optionEls[options.length - 1].classList).toContain('cdk-option-active'); - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); + dispatchKeyboardEvent(listboxEl, 'keydown', HOME); fixture.detectChanges(); - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse(); - expect(document.activeElement).toEqual(optionElements[0]); - dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW); - fixture.detectChanges(); - - expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse(); - expect(document.activeElement).toEqual(optionElements[1]); + expect(options[0].isActive()).toBeTrue(); + expect(optionEls[0].classList).toContain('cdk-option-active'); }); - }); - - describe('with control value accessor implemented', () => { - let fixture: ComponentFixture; - let testComponent: ListboxControlValueAccessor; - let listbox: DebugElement; - let listboxInstance: CdkListbox; + it('should change active item using type ahead', fakeAsync(async () => { + const {fixture, listbox, listboxEl, options} = await setupComponent(ListboxWithOptions); + listbox.focus(); + fixture.detectChanges(); - let options: DebugElement[]; - let optionInstances: CdkOption[]; - let optionElements: HTMLElement[]; + dispatchKeyboardEvent(listboxEl, 'keydown', B); + fixture.detectChanges(); + tick(200); - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkListboxModule, FormsModule, ReactiveFormsModule], - declarations: [ListboxControlValueAccessor], - }).compileComponents(); + expect(options[2].isActive()).toBeTrue(); })); - beforeEach(() => { - fixture = TestBed.createComponent(ListboxControlValueAccessor); + it('should allow custom type ahead label', fakeAsync(async () => { + const {fixture, listbox, listboxEl, options} = await setupComponent( + ListboxWithCustomTypeahead, + ); + listbox.focus(); fixture.detectChanges(); - testComponent = fixture.debugElement.componentInstance; - - listbox = fixture.debugElement.query(By.directive(CdkListbox)); - listboxInstance = listbox.injector.get>(CdkListbox); - - options = fixture.debugElement.queryAll(By.directive(CdkOption)); - optionInstances = options.map(o => o.injector.get(CdkOption)); - optionElements = options.map(o => o.nativeElement); - }); - - it('should be able to set the disabled state via setDisabledState', () => { - expect(listboxInstance.disabled) - .withContext('Expected the selection list to be enabled.') - .toBe(false); - expect(optionInstances.every(option => !option.disabled)) - .withContext('Expected every list option to be enabled.') - .toBe(true); - - listboxInstance.setDisabledState(true); + dispatchKeyboardEvent(listboxEl, 'keydown', B); fixture.detectChanges(); + tick(200); - expect(listboxInstance.disabled) - .withContext('Expected the selection list to be disabled.') - .toBe(true); - for (const option of optionElements) { - expect(option.getAttribute('aria-disabled')).toBe('true'); - } - }); - - it('should be able to select options via writeValue', () => { - expect(optionInstances.every(option => !option.disabled)) - .withContext('Expected every list option to be enabled.') - .toBe(true); + expect(options[2].isActive()).toBeTrue(); + })); - listboxInstance.writeValue('arc'); + it('should focus and toggle the next item when pressing SHIFT + DOWN_ARROW', async () => { + const {fixture, listbox, listboxEl, options} = await setupComponent(ListboxWithOptions); + listbox.focus(); fixture.detectChanges(); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionElements[1].hasAttribute('aria-selected')).toBeFalse(); - expect(optionElements[3].hasAttribute('aria-selected')).toBeFalse(); + dispatchKeyboardEvent(listboxEl, 'keydown', DOWN_ARROW, undefined, {shift: true}); + fixture.detectChanges(); - expect(optionInstances[2].selected).toBeTrue(); - expect(optionElements[2].getAttribute('aria-selected')).toBe('true'); + expect(listbox.value).toEqual(['orange']); + expect(fixture.componentInstance.changedOption?.id).toBe(options[1].id); }); - it('should be select multiple options by their values', () => { - expect(optionInstances.every(option => !option.disabled)) - .withContext('Expected every list option to be enabled.') - .toBe(true); - - testComponent.isMultiselectable = true; - fixture.detectChanges(); + // TODO(mmalerba): ensure all keys covered + }); - listboxInstance.writeValue(['arc', 'stasis']); + describe('with roving tabindex', () => { + it('should shift focus on keyboard navigation', async () => { + const {fixture, listbox, listboxEl, optionEls} = await setupComponent(ListboxWithOptions); + listbox.focus(); fixture.detectChanges(); - const selectedValues = listboxInstance.getSelectedValues(); - expect(selectedValues.length).toBe(2); - expect(selectedValues[0]).toBe('arc'); - expect(selectedValues[1]).toBe('stasis'); + expect(document.activeElement).toBe(optionEls[0]); + expect(listboxEl.hasAttribute('aria-activedescendant')).toBeFalse(); - expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse(); - expect(optionElements[1].hasAttribute('aria-selected')).toBeFalse(); + dispatchKeyboardEvent(listboxEl, 'keydown', DOWN_ARROW); + fixture.detectChanges(); - expect(optionInstances[2].selected).toBeTrue(); - expect(optionElements[2].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[3].selected).toBeTrue(); - expect(optionElements[3].getAttribute('aria-selected')).toBe('true'); + expect(document.activeElement).toBe(optionEls[1]); + expect(listboxEl.hasAttribute('aria-activedescendant')).toBeFalse(); }); - it('should be able to disable options from the control', () => { - expect(testComponent.listbox.disabled).toBeFalse(); - expect(optionInstances.every(option => !option.disabled)) - .withContext('Expected every list option to be enabled.') - .toBe(true); - - testComponent.form.disable(); + it('should focus first option on listbox focus', async () => { + const {fixture, listbox, optionEls} = await setupComponent(ListboxWithOptions); + listbox.focus(); fixture.detectChanges(); - expect(testComponent.listbox.disabled).toBeTrue(); - for (const option of optionElements) { - expect(option.getAttribute('aria-disabled')).toBe('true'); - } + expect(document.activeElement).toBe(optionEls[0]); }); - it('should be able to toggle disabled state after form control is disabled', () => { - expect(testComponent.listbox.disabled).toBeFalse(); - expect(optionInstances.every(option => !option.disabled)) - .withContext('Expected every list option to be enabled.') - .toBe(true); - - testComponent.form.disable(); - fixture.detectChanges(); - - expect(testComponent.listbox.disabled).toBeTrue(); - for (const option of optionElements) { - expect(option.getAttribute('aria-disabled')).toBe('true'); - } + it('should focus listbox if no focusable options available', async () => { + const {fixture, listbox, listboxEl} = await setupComponent(ListboxWithNoOptions); - listboxInstance.disabled = false; + listbox.focus(); fixture.detectChanges(); - expect(testComponent.listbox.disabled).toBeFalse(); - expect(optionInstances.every(option => !option.disabled)) - .withContext('Expected every list option to be enabled.') - .toBe(true); + expect(document.activeElement).toBe(listboxEl); }); + }); - it('should be able to select options via setting the value in form control', () => { - expect(optionInstances.every(option => option.selected)).toBeFalse(); - - testComponent.isMultiselectable = true; + describe('with aria-activedescendant', () => { + it('should update active descendant on keyboard navigation', async () => { + const {testComponent, fixture, listbox, listboxEl, optionEls} = await setupComponent( + ListboxWithOptions, + ); + testComponent.isActiveDescendant = true; fixture.detectChanges(); - - testComponent.form.setValue(['purple', 'arc']); + listbox.focus(); + dispatchKeyboardEvent(listboxEl, 'keydown', DOWN_ARROW); fixture.detectChanges(); - expect(optionElements[0].getAttribute('aria-selected')).toBe('true'); - expect(optionElements[2].getAttribute('aria-selected')).toBe('true'); - expect(optionInstances[0].selected).toBeTrue(); - expect(optionInstances[2].selected).toBeTrue(); + expect(listboxEl.getAttribute('aria-activedescendant')).toBe(optionEls[0].id); + expect(document.activeElement).toBe(listboxEl); - testComponent.form.setValue(null); + dispatchKeyboardEvent(listboxEl, 'keydown', DOWN_ARROW); fixture.detectChanges(); - expect(optionInstances.every(option => option.selected)).toBeFalse(); + expect(listboxEl.getAttribute('aria-activedescendant')).toBe(optionEls[1].id); + expect(document.activeElement).toBe(listboxEl); }); - it('should only select the first matching option if multiple is not enabled', () => { - expect(optionInstances.every(option => option.selected)).toBeFalse(); - - testComponent.form.setValue(['solar', 'arc']); + it('should not activate an option on listbox focus', async () => { + const {testComponent, fixture, listbox, options} = await setupComponent(ListboxWithOptions); + testComponent.isActiveDescendant = true; + fixture.detectChanges(); + listbox.focus(); fixture.detectChanges(); - expect(optionElements[1].getAttribute('aria-selected')).toBe('true'); - expect(optionElements[2].hasAttribute('aria-selected')).toBeFalse(); - expect(optionInstances[1].selected).toBeTrue(); - expect(optionInstances[2].selected).toBeFalse(); + for (let option of options) { + expect(option.isActive()).toBeFalse(); + } }); - it('should deselect an option selected via form control once its value changes', () => { - const option = optionInstances[1]; - const element = optionElements[1]; - - testComponent.form.setValue(['solar']); + it('should focus listbox and make option active on option focus', async () => { + const {testComponent, fixture, listboxEl, options, optionEls} = await setupComponent( + ListboxWithOptions, + ); + testComponent.isActiveDescendant = true; fixture.detectChanges(); - - expect(element.getAttribute('aria-selected')).toBe('true'); - expect(option.selected).toBeTrue(); - - option.value = 'new-value'; + optionEls[2].focus(); fixture.detectChanges(); - expect(element.hasAttribute('aria-selected')).toBeFalse(); - expect(option.selected).toBeFalse(); + expect(document.activeElement).toBe(listboxEl); + expect(options[2].isActive()).toBeTrue(); }); + }); - it('should maintain the form control on listbox destruction', function () { - testComponent.form.setValue(['solar']); + describe('with FormControl', () => { + it('should reflect disabled state of the FormControl', async () => { + const {testComponent, fixture, listbox} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); + testComponent.formControl.disable(); fixture.detectChanges(); - expect(testComponent.form.value).toEqual(['solar']); + expect(listbox.disabled).toBeTrue(); + }); - testComponent.showListbox = false; + it('should update when FormControl value changes', async () => { + const {testComponent, fixture, options} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); + testComponent.formControl.setValue(['banana']); fixture.detectChanges(); - expect(testComponent.form.value).toEqual(['solar']); + expect(options[2].isSelected()).toBeTrue(); }); - }); - - describe('inside a combobox', () => { - let fixture: ComponentFixture; - let testComponent: ListboxInsideCombobox; - let listbox: DebugElement; - let listboxInstance: CdkListbox; - let listboxElement: HTMLElement; - - let combobox: DebugElement; - let comboboxInstance: CdkCombobox; - let comboboxElement: HTMLElement; - - let options: DebugElement[]; - let optionInstances: CdkOption[]; - let optionElements: HTMLElement[]; + it('should update FormControl when selection changes', async () => { + const {testComponent, fixture, optionEls} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); + const spy = jasmine.createSpy(); + const subscription = testComponent.formControl.valueChanges.subscribe(spy); + fixture.detectChanges(); - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkListboxModule, CdkComboboxModule], - declarations: [ListboxInsideCombobox], - }).compileComponents(); - })); + expect(spy).not.toHaveBeenCalled(); - beforeEach(() => { - fixture = TestBed.createComponent(ListboxInsideCombobox); + optionEls[1].click(); fixture.detectChanges(); - testComponent = fixture.debugElement.componentInstance; - - combobox = fixture.debugElement.query(By.directive(CdkCombobox)); - comboboxInstance = combobox.injector.get>(CdkCombobox); - comboboxElement = combobox.nativeElement; + expect(spy).toHaveBeenCalledWith(['orange']); + subscription.unsubscribe(); }); - it('should update combobox value on selection of an option', () => { - expect(comboboxInstance.value).toBeUndefined(); - expect(comboboxInstance.isOpen()).toBeFalse(); - - dispatchMouseEvent(comboboxElement, 'click'); + it('should update multi-select listbox when FormControl value changes', async () => { + const {testComponent, fixture, options} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); + testComponent.isMultiselectable = true; fixture.detectChanges(); - - listbox = fixture.debugElement.query(By.directive(CdkListbox)); - listboxInstance = listbox.injector.get>(CdkListbox); - - options = fixture.debugElement.queryAll(By.directive(CdkOption)); - optionInstances = options.map(o => o.injector.get(CdkOption)); - optionElements = options.map(o => o.nativeElement); - - expect(comboboxInstance.isOpen()).toBeTrue(); - - dispatchMouseEvent(optionElements[0], 'click'); + testComponent.formControl.setValue(['orange', 'banana']); fixture.detectChanges(); - expect(comboboxInstance.isOpen()).toBeFalse(); - expect(comboboxInstance.value).toBe('purple'); + expect(options[1].isSelected()).toBeTrue(); + expect(options[2].isSelected()).toBeTrue(); }); - it('should update combobox value on selection via keyboard', () => { - expect(comboboxInstance.value).toBeUndefined(); - expect(comboboxInstance.isOpen()).toBeFalse(); - - dispatchMouseEvent(comboboxElement, 'click'); + it('should update FormControl when multi-selection listbox changes', async () => { + const {testComponent, fixture, optionEls} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); + testComponent.isMultiselectable = true; + fixture.detectChanges(); + const spy = jasmine.createSpy(); + const subscription = testComponent.formControl.valueChanges.subscribe(spy); fixture.detectChanges(); - listbox = fixture.debugElement.query(By.directive(CdkListbox)); - listboxInstance = listbox.injector.get>(CdkListbox); - listboxElement = listbox.nativeElement; - - options = fixture.debugElement.queryAll(By.directive(CdkOption)); - optionInstances = options.map(o => o.injector.get(CdkOption)); - optionElements = options.map(o => o.nativeElement); - - expect(comboboxInstance.isOpen()).toBeTrue(); + expect(spy).not.toHaveBeenCalled(); - listboxInstance.setActiveOption(optionInstances[1]); - dispatchKeyboardEvent(listboxElement, 'keydown', SPACE); + optionEls[1].click(); fixture.detectChanges(); + expect(spy).toHaveBeenCalledWith(['orange']); - expect(comboboxInstance.isOpen()).toBeFalse(); - expect(comboboxInstance.value).toBe('solar'); + optionEls[2].click(); + fixture.detectChanges(); + expect(spy).toHaveBeenCalledWith(['orange', 'banana']); + subscription.unsubscribe(); }); - it('should not close panel if listbox is in multiple mode', () => { - expect(comboboxInstance.value).toBeUndefined(); - expect(comboboxInstance.isOpen()).toBeFalse(); - - dispatchMouseEvent(comboboxElement, 'click'); + it('should have FormControl error multiple values selected in single-select listbox', async () => { + const {testComponent, fixture} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); + testComponent.formControl.setValue(['orange', 'banana']); fixture.detectChanges(); - listbox = fixture.debugElement.query(By.directive(CdkListbox)); - listboxInstance = listbox.injector.get>(CdkListbox); - listboxElement = listbox.nativeElement; + expect(testComponent.formControl.hasError('cdkListboxMultipleValues')).toBeTrue(); + expect(testComponent.formControl.hasError('cdkListboxInvalidValues')).toBeFalse(); + }); + it('should have FormControl error when non-option value selected', async () => { + const {testComponent, fixture} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); testComponent.isMultiselectable = true; + testComponent.formControl.setValue(['orange', 'dragonfruit', 'mango']); fixture.detectChanges(); - options = fixture.debugElement.queryAll(By.directive(CdkOption)); - optionInstances = options.map(o => o.injector.get(CdkOption)); - optionElements = options.map(o => o.nativeElement); - - expect(comboboxInstance.isOpen()).toBeTrue(); + expect(testComponent.formControl.hasError('cdkListboxInvalidValues')).toBeTrue(); + expect(testComponent.formControl.hasError('cdkListboxMultipleValues')).toBeFalse(); + expect(testComponent.formControl.errors?.['cdkListboxInvalidValues']).toEqual({ + 'values': ['dragonfruit', 'mango'], + }); + }); - listboxInstance.setActiveOption(optionInstances[1]); - dispatchKeyboardEvent(listboxElement, 'keydown', SPACE); - testComponent.combobox.updateAndClose(testComponent.listbox.getSelectedValues()); + it('should have multiple FormControl errors when multiple non-option values selected in single-select listbox', async () => { + const {testComponent, fixture} = await setupComponent(ListboxWithFormControl, [ + ReactiveFormsModule, + ]); + testComponent.formControl.setValue(['dragonfruit', 'mango']); fixture.detectChanges(); - expect(comboboxInstance.isOpen()).toBeFalse(); - expect(comboboxInstance.value).toEqual(['solar']); + expect(testComponent.formControl.hasError('cdkListboxInvalidValues')).toBeTrue(); + expect(testComponent.formControl.hasError('cdkListboxMultipleValues')).toBeTrue(); + expect(testComponent.formControl.errors?.['cdkListboxInvalidValues']).toEqual({ + 'values': ['dragonfruit', 'mango'], + }); }); }); }); @@ -883,131 +680,101 @@ describe('CdkOption and CdkListbox', () => { @Component({ template: `
      -
      - Purple + [id]="listboxId" + [tabindex]="listboxTabindex" + [cdkListboxMultiple]="isMultiselectable" + [cdkListboxDisabled]="isListboxDisabled" + [cdkListboxUseActiveDescendant]="isActiveDescendant" + (cdkListboxValueChange)="onSelectionChange($event)"> +
      + Apple
      -
      - Solar -
      -
      Arc
      -
      Stasis
      -
      `, +
      Orange
      +
      Banana
      +
      Peach
      +
      + `, }) class ListboxWithOptions { changedOption: CdkOption; - isListboxDisabled: boolean = false; - isPurpleDisabled: boolean = false; - isSolarDisabled: boolean = false; + isListboxDisabled = false; + isAppleDisabled = false; + isOrangeDisabled = false; + isMultiselectable = false; + isActiveDescendant = false; + listboxId: string; + listboxTabindex: number; + appleId: string; + appleTabindex: number; onSelectionChange(event: ListboxValueChangeEvent) { this.changedOption = event.option; } } +@Component({ + template: `
      `, +}) +class ListboxWithNoOptions {} + @Component({ template: `
      -
      Purple
      -
      Solar
      -
      Arc
      -
      Stasis
      -
      `, + [formControl]="formControl" + [cdkListboxMultiple]="isMultiselectable" + [cdkListboxUseActiveDescendant]="isActiveDescendant"> +
      Apple
      +
      Orange
      +
      Banana
      +
      Peach
      + + `, }) -class ListboxMultiselect { - changedOption: CdkOption; - isMultiselectable: boolean = false; - - onSelectionChange(event: ListboxValueChangeEvent) { - this.changedOption = event.option; - } +class ListboxWithFormControl { + formControl = new FormControl(); + isMultiselectable = false; + isActiveDescendant = false; } @Component({ template: ` -
      -
      Purple
      -
      Solar
      -
      Arc
      -
      Stasis
      -
      `, +
        +
      • 🍎
      • +
      • 🍊
      • +
      • 🍌
      • +
      • 🍑
      • +
      + `, }) -class ListboxActiveDescendant { - changedOption: CdkOption; - isActiveDescendant: boolean = true; - focusedOption: string; - - onSelectionChange(event: ListboxValueChangeEvent) { - this.changedOption = event.option; - } - - onFocus(option: string) { - this.focusedOption = option; - } -} +class ListboxWithCustomTypeahead {} @Component({ template: ` - `, +
      +
      Apple
      +
      Orange
      +
      Banana
      +
      Peach
      +
      + `, }) -class ListboxControlValueAccessor { - form = new FormControl([]); - changedOption: CdkOption; - isDisabled: boolean = false; - isMultiselectable: boolean = false; - showListbox: boolean = true; - @ViewChild(CdkListbox) listbox: CdkListbox; - - onSelectionChange(event: ListboxValueChangeEvent) { - this.changedOption = event.option; - } +class ListboxWithBoundValue { + value = ['banana']; } @Component({ template: ` - - - - - +
      +
      {{fruit.name}}
      +
      `, }) -class ListboxInsideCombobox { - changedOption: CdkOption; - isDisabled: boolean = false; - isMultiselectable: boolean = false; - @ViewChild(CdkListbox) listbox: CdkListbox; - @ViewChild(CdkCombobox) combobox: CdkCombobox; - - onSelectionChange(event: ListboxValueChangeEvent) { - this.changedOption = event.option; - } +class ListboxWithObjectValues { + fruits = [{name: 'Apple'}, {name: 'Orange'}, {name: 'Banana'}, {name: 'Peach'}]; + + fruitCompare = (a: {name: string}, b: {name: string}) => a.name === b.name; } diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index 10397fed1176..964c1f5ae3a1 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -22,12 +22,7 @@ import { } from '@angular/core'; import {ActiveDescendantKeyManager, Highlightable, ListKeyManagerOption} from '@angular/cdk/a11y'; import {DOWN_ARROW, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, UP_ARROW} from '@angular/cdk/keycodes'; -import { - BooleanInput, - coerceArray, - coerceBooleanProperty, - coerceNumberProperty, -} from '@angular/cdk/coercion'; +import {BooleanInput, coerceArray, coerceBooleanProperty} from '@angular/cdk/coercion'; import {SelectionModel} from '@angular/cdk/collections'; import {BehaviorSubject, combineLatest, defer, merge, Observable, Subject} from 'rxjs'; import {filter, mapTo, startWith, switchMap, take, takeUntil} from 'rxjs/operators'; @@ -47,6 +42,10 @@ import {CdkCombobox} from '@angular/cdk-experimental/combobox'; /** The next id to use for creating unique DOM IDs. */ let nextId = 0; +// TODO(mmalerba): +// - should listbox wrap be configurable? +// - should skipping disabled options be configurable? + /** A selectable option in a listbox. */ @Directive({ selector: '[cdkOption]', @@ -67,7 +66,15 @@ let nextId = 0; }) export class CdkOption implements ListKeyManagerOption, Highlightable, OnDestroy { /** The id of the option's host element. */ - @Input() id = `cdk-option-${nextId++}`; + @Input() + get id() { + return this._id || this._generatedId; + } + set id(value) { + this._id = value; + } + private _id: string; + private _generatedId = `cdk-option-${nextId++}`; /** The value of this option. */ @Input('cdkOption') value: T; @@ -177,6 +184,7 @@ export class CdkOption implements ListKeyManagerOption, Highlightab // In this case, we push focus back to the parent listbox to prevent an extra tab stop when // the user performs a shift+tab. if (this.listbox.useActiveDescendant) { + this.listbox._setActiveOption(this); this.listbox.focus(); } } @@ -223,7 +231,15 @@ export class CdkListbox implements AfterContentInit, OnDestroy, ControlValueAccessor, Validator { /** The id of the option's host element. */ - @Input() id = `cdk-listbox-${nextId++}`; + @Input() + get id() { + return this._id || this._generatedId; + } + set id(value) { + this._id = value; + } + private _id: string; + private _generatedId = `cdk-listbox-${nextId++}`; /** The tabindex to use when the listbox is enabled. */ @Input('tabindex') @@ -568,10 +584,18 @@ export class CdkListbox } } + /** + * Sets the given option as active. + * @param option The option to make active + */ + _setActiveOption(option: CdkOption) { + this.listKeyManager.setActiveItem(option); + } + /** Called when the listbox receives focus. */ protected _handleFocus() { if (!this.useActiveDescendant) { - this.listKeyManager.setActiveItem(this.listKeyManager.activeItem ?? this.options.first); + this.listKeyManager.setNextItemActive(); this._focusActiveOption(); } } @@ -682,8 +706,11 @@ export class CdkListbox * @param value The list of new selected values. */ private _setSelection(value: readonly T[]) { + const coercedValue = this._coerceValue(value); this.selectionModel().setSelection( - ...this._getValuesWithValidity(this._coerceValue(value), true), + ...(!this.multiple && coercedValue.length > 1 + ? [] + : this._getValuesWithValidity(coercedValue, true)), ); } From 18125ca7d1c61b8786949c8bb55da936a0f8796e Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 24 May 2022 22:28:00 +0000 Subject: [PATCH 08/10] fix(cdk-experimental/listbox): fix lint issues --- src/cdk-experimental/listbox/listbox.spec.ts | 32 +++++++++---------- .../cdk-listbox-demo.ts | 10 ++---- tools/public_api_guard/cdk/collections.md | 4 ++- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.spec.ts b/src/cdk-experimental/listbox/listbox.spec.ts index a9694f49a4ea..82f8c84e2a2e 100644 --- a/src/cdk-experimental/listbox/listbox.spec.ts +++ b/src/cdk-experimental/listbox/listbox.spec.ts @@ -342,9 +342,7 @@ describe('CdkOption and CdkListbox', () => { }); it('should not change selection on click of a disabled option', async () => { - const {fixture, testComponent, listbox, listboxEl, options} = await setupComponent( - ListboxWithOptions, - ); + const {fixture, testComponent, listbox, listboxEl} = await setupComponent(ListboxWithOptions); listbox.focus(); fixture.detectChanges(); @@ -751,20 +749,20 @@ class ListboxWithFormControl { }) class ListboxWithCustomTypeahead {} -@Component({ - template: ` -
      -
      Apple
      -
      Orange
      -
      Banana
      -
      Peach
      -
      - `, -}) -class ListboxWithBoundValue { - value = ['banana']; -} +// @Component({ +// template: ` +//
      +//
      Apple
      +//
      Orange
      +//
      Banana
      +//
      Peach
      +//
      +// `, +// }) +// class ListboxWithBoundValue { +// value = ['banana']; +// } @Component({ template: ` diff --git a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts index f035d27c4472..8ff808d681f1 100644 --- a/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts +++ b/src/dev-app/cdk-experimental-listbox/cdk-listbox-demo.ts @@ -6,16 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import { - AfterViewInit, - ChangeDetectionStrategy, - Component, - ViewChild, - ViewEncapsulation, -} from '@angular/core'; +import {ChangeDetectionStrategy, Component} from '@angular/core'; import {CdkListboxModule} from '@angular/cdk-experimental/listbox'; import {CommonModule} from '@angular/common'; -import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms'; +import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; import {MatSelectModule} from '@angular/material/select'; function dumbCompare(o1: string, o2: string) { diff --git a/tools/public_api_guard/cdk/collections.md b/tools/public_api_guard/cdk/collections.md index c97b30f47179..6b422b4ced26 100644 --- a/tools/public_api_guard/cdk/collections.md +++ b/tools/public_api_guard/cdk/collections.md @@ -71,7 +71,7 @@ export interface SelectionChange { // @public export class SelectionModel { - constructor(_multiple?: boolean, initiallySelectedValues?: T[], _emitChanges?: boolean); + constructor(_multiple?: boolean, initiallySelectedValues?: T[], _emitChanges?: boolean, _compareWith?: ((o1: T, o2: T) => boolean) | undefined); readonly changed: Subject>; clear(): void; deselect(...values: T[]): void; @@ -81,6 +81,8 @@ export class SelectionModel { isSelected(value: T): boolean; select(...values: T[]): void; get selected(): T[]; + // (undocumented) + setSelection(...values: T[]): void; sort(predicate?: (a: T, b: T) => number): void; toggle(value: T): void; } From 072effa387fc6606d29fdb721c0529e565f598ab Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 24 May 2022 22:52:42 +0000 Subject: [PATCH 09/10] fix(cdk-experimental/listbox): fix tests --- src/cdk-experimental/listbox/listbox.spec.ts | 58 +++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.spec.ts b/src/cdk-experimental/listbox/listbox.spec.ts index 82f8c84e2a2e..3f3221a7d478 100644 --- a/src/cdk-experimental/listbox/listbox.spec.ts +++ b/src/cdk-experimental/listbox/listbox.spec.ts @@ -356,19 +356,21 @@ describe('CdkOption and CdkListbox', () => { expect(fixture.componentInstance.changedOption).toBeUndefined(); }); - it('should not handle type ahead on a disabled listbox', fakeAsync(async () => { + it('should not handle type ahead on a disabled listbox', async (...args: unknown[]) => { const {fixture, testComponent, listboxEl, options} = await setupComponent(ListboxWithOptions); - testComponent.isListboxDisabled = true; - fixture.detectChanges(); + await fakeAsync(() => { + testComponent.isListboxDisabled = true; + fixture.detectChanges(); - dispatchKeyboardEvent(listboxEl, 'keydown', B); - fixture.detectChanges(); - tick(200); + dispatchKeyboardEvent(listboxEl, 'keydown', B); + fixture.detectChanges(); + tick(200); - for (let option of options) { - expect(option.isActive()).toBeFalse(); - } - })); + for (let option of options) { + expect(option.isActive()).toBeFalse(); + } + })(args); + }); it('should skip disabled options when navigating with arrow keys', async () => { const {testComponent, fixture, listbox, listboxEl, options} = await setupComponent( @@ -438,31 +440,35 @@ describe('CdkOption and CdkListbox', () => { expect(optionEls[0].classList).toContain('cdk-option-active'); }); - it('should change active item using type ahead', fakeAsync(async () => { + it('should change active item using type ahead', async (...args: unknown[]) => { const {fixture, listbox, listboxEl, options} = await setupComponent(ListboxWithOptions); - listbox.focus(); - fixture.detectChanges(); + await fakeAsync(() => { + listbox.focus(); + fixture.detectChanges(); - dispatchKeyboardEvent(listboxEl, 'keydown', B); - fixture.detectChanges(); - tick(200); + dispatchKeyboardEvent(listboxEl, 'keydown', B); + fixture.detectChanges(); + tick(200); - expect(options[2].isActive()).toBeTrue(); - })); + expect(options[2].isActive()).toBeTrue(); + })(args); + }); - it('should allow custom type ahead label', fakeAsync(async () => { + it('should allow custom type ahead label', async (...args: unknown[]) => { const {fixture, listbox, listboxEl, options} = await setupComponent( ListboxWithCustomTypeahead, ); - listbox.focus(); - fixture.detectChanges(); + await fakeAsync(() => { + listbox.focus(); + fixture.detectChanges(); - dispatchKeyboardEvent(listboxEl, 'keydown', B); - fixture.detectChanges(); - tick(200); + dispatchKeyboardEvent(listboxEl, 'keydown', B); + fixture.detectChanges(); + tick(200); - expect(options[2].isActive()).toBeTrue(); - })); + expect(options[2].isActive()).toBeTrue(); + })(args); + }); it('should focus and toggle the next item when pressing SHIFT + DOWN_ARROW', async () => { const {fixture, listbox, listboxEl, options} = await setupComponent(ListboxWithOptions); From a51e8ffa965e1319c806e8dc1b1fb8bae12e63d5 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Wed, 25 May 2022 15:56:33 +0000 Subject: [PATCH 10/10] fix(cdk-experimental/listbox): address feedback --- src/cdk-experimental/listbox/listbox.ts | 35 +++++++++++++++---------- src/cdk/collections/selection-model.ts | 7 ++++- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/cdk-experimental/listbox/listbox.ts b/src/cdk-experimental/listbox/listbox.ts index 964c1f5ae3a1..ca6c637a42e2 100644 --- a/src/cdk-experimental/listbox/listbox.ts +++ b/src/cdk-experimental/listbox/listbox.ts @@ -716,7 +716,6 @@ export class CdkListbox /** Update the internal value of the listbox based on the selection model. */ private _updateInternalValue() { - const options = [...this.options]; const indexCache = new Map(); // Check if we need to remove any values due to them becoming invalid // (e.g. if the option was removed from the DOM.) @@ -726,8 +725,8 @@ export class CdkListbox this.selectionModel().setSelection(...validSelected); } this.selectionModel().sort((a: T, b: T) => { - const aIndex = this._getIndexForValue(indexCache, options, a); - const bIndex = this._getIndexForValue(indexCache, options, b); + const aIndex = this._getIndexForValue(indexCache, a); + const bIndex = this._getIndexForValue(indexCache, b); return aIndex - bIndex; }); this.changeDetectorRef.markForCheck(); @@ -736,17 +735,20 @@ export class CdkListbox /** * Gets the index of the given value in the given list of options. * @param cache The cache of indices found so far - * @param options The list of options to search in * @param value The value to find * @return The index of the value in the options list */ - private _getIndexForValue(cache: Map, options: CdkOption[], value: T) { + private _getIndexForValue(cache: Map, value: T) { const isEqual = this.compareWith || Object.is; if (!cache.has(value)) { - cache.set( - value, - options.findIndex(option => isEqual(value, option.value)), - ); + let index = -1; + for (let i = 0; i < this.options.length; i++) { + if (isEqual(value, this.options.get(i)!.value)) { + index = i; + break; + } + } + cache.set(value, index); } return cache.get(value)!; } @@ -768,11 +770,16 @@ export class CdkListbox this.options.changes.pipe(startWith(this.options)), ]).subscribe(() => { const isEqual = this.compareWith ?? Object.is; - const options = [...this.options]; - for (const option of options) { - let duplicate = options.find( - other => option !== other && isEqual(option.value, other.value), - ); + for (let i = 0; i < this.options.length; i++) { + const option = this.options.get(i)!; + let duplicate: CdkOption | null = null; + for (let j = i + 1; j < this.options.length; j++) { + const other = this.options.get(j)!; + if (isEqual(option.value, other.value)) { + duplicate = other; + break; + } + } if (duplicate) { // TODO(mmalerba): Link to docs about this. if (this.compareWith) { diff --git a/src/cdk/collections/selection-model.ts b/src/cdk/collections/selection-model.ts index 93a25650ba1d..0324b90f5ac3 100644 --- a/src/cdk/collections/selection-model.ts +++ b/src/cdk/collections/selection-model.ts @@ -103,7 +103,12 @@ export class SelectionModel { */ isSelected(value: T): boolean { if (this._compareWith) { - return [...this._selection].some(v => this._compareWith!(v, value)); + for (const otherValue of this._selection) { + if (this._compareWith(otherValue, value)) { + return true; + } + } + return false; } return this._selection.has(value); }