Skip to content

Combobox listbox compatible #20291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Aug 14, 2020
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9d96266
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
381a91d
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
4a2fc63
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
b3f282b
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
a5b7004
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
0ae5554
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
e13d328
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
c5abfd5
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
5d0868c
feat(dev-app/listbox): added cdk listbox example to the dev-app.
nielsr98 Jul 15, 2020
0610f75
fix(listbox): removed duplicate dep in dev-app build file.
nielsr98 Jul 22, 2020
6e8893e
fix(listbox): deleted unused files.
nielsr98 Aug 4, 2020
5541e6e
refactor(combobox): changed names and made coerceOpenActionProperty s…
nielsr98 Aug 10, 2020
1fe8976
fix(combobox): updated syntax for casting.
nielsr98 Aug 10, 2020
ec79fe4
fix(combobox): fixed trailing whitespace.
nielsr98 Aug 10, 2020
ba873a0
feat(listbox): added functions and panel injection to make listbox au…
nielsr98 Aug 12, 2020
0d63586
feat(listbox): added tests for listbox inside combobox.
nielsr98 Aug 12, 2020
cb3b93c
test(listbox): added more tests for putting a listbox inside a combobox.
nielsr98 Aug 12, 2020
e3a461d
refactor(listbox): used lightweight injection token for CdkComboboxPa…
nielsr98 Aug 13, 2020
a2dd79a
fix(combobox): reformatted build file.
nielsr98 Aug 13, 2020
8221c12
fix(listbox): fixed an issue where the order of emitting change event…
nielsr98 Aug 13, 2020
5009b9f
feat(listbox): added additional focus management logic and a getSelec…
nielsr98 Aug 14, 2020
4c15d53
lint(combobox): removed trailing whitespace.
nielsr98 Aug 14, 2020
48d05c3
refactor(listbox): realized disabled listbox still closes combobox po…
nielsr98 Aug 14, 2020
744d23a
fix(listbox): fixed lint errors.
nielsr98 Aug 14, 2020
e3dd27a
refactor(listbox): simplified register and update panel functions.
nielsr98 Aug 14, 2020
22495bc
refactor(listbox): changed updatePanel to updatePanelForSelection.
nielsr98 Aug 14, 2020
24522be
fix(listbox): removed unused variable.
nielsr98 Aug 14, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/cdk-experimental/combobox/combobox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ describe('Combobox', () => {
No Value
</button>
<div id="other-content"></div>

<ng-template cdkComboboxPanel #panel="cdkComboboxPanel">
<div dialogContent #dialog="dialogContent" [parentPanel]="panel">
<input #input>
Expand Down
1 change: 0 additions & 1 deletion src/cdk-experimental/combobox/combobox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/


export type OpenAction = 'focus' | 'click' | 'downKey' | 'toggle';
export type OpenActionInput = OpenAction | OpenAction[] | string | null | undefined;

Expand Down
2 changes: 2 additions & 0 deletions src/cdk-experimental/listbox/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ ng_module(
),
module_name = "@angular/cdk-experimental/listbox",
deps = [
"//src/cdk-experimental/combobox",
"//src/cdk/a11y",
"//src/cdk/collections",
"//src/cdk/keycodes",
Expand All @@ -25,6 +26,7 @@ ng_test_library(
),
deps = [
":listbox",
"//src/cdk-experimental/combobox",
"//src/cdk/keycodes",
"//src/cdk/testing/private",
"@npm//@angular/forms",
Expand Down
152 changes: 152 additions & 0 deletions src/cdk-experimental/listbox/listbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
} from '@angular/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';


describe('CdkOption and CdkListbox', () => {

Expand Down Expand Up @@ -657,6 +659,11 @@ describe('CdkOption and CdkListbox', () => {
listboxInstance.writeValue(['arc', 'stasis']);
fixture.detectChanges();

const selectedValues = listboxInstance.getSelectedValues();
expect(selectedValues.length).toBe(2);
expect(selectedValues[0]).toBe('arc');
expect(selectedValues[1]).toBe('stasis');

expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse();
expect(optionElements[1].hasAttribute('aria-selected')).toBeFalse();

Expand Down Expand Up @@ -762,6 +769,118 @@ describe('CdkOption and CdkListbox', () => {
expect(testComponent.form.value).toEqual(['solar']);
});
});

describe('inside a combobox', () => {
let fixture: ComponentFixture<ListboxInsideCombobox>;
let testComponent: ListboxInsideCombobox;

let listbox: DebugElement;
let listboxInstance: CdkListbox<unknown>;
let listboxElement: HTMLElement;

let combobox: DebugElement;
let comboboxInstance: CdkCombobox<string>;
let comboboxElement: HTMLElement;

let options: DebugElement[];
let optionInstances: CdkOption[];
let optionElements: HTMLElement[];

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [CdkListboxModule, CdkComboboxModule],
declarations: [ListboxInsideCombobox],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(ListboxInsideCombobox);
fixture.detectChanges();

testComponent = fixture.debugElement.componentInstance;

combobox = fixture.debugElement.query(By.directive(CdkCombobox));
comboboxInstance = combobox.injector.get<CdkCombobox<string>>(CdkCombobox);
comboboxElement = combobox.nativeElement;

});

it('should update combobox value on selection of an option', () => {
expect(comboboxInstance.value).toBeUndefined();
expect(comboboxInstance.isOpen()).toBeFalse();

dispatchMouseEvent(comboboxElement, 'click');
fixture.detectChanges();

listbox = fixture.debugElement.query(By.directive(CdkListbox));
listboxInstance = listbox.injector.get<CdkListbox<unknown>>(CdkListbox);

options = fixture.debugElement.queryAll(By.directive(CdkOption));
optionInstances = options.map(o => o.injector.get<CdkOption>(CdkOption));
optionElements = options.map(o => o.nativeElement);

expect(comboboxInstance.isOpen()).toBeTrue();

dispatchMouseEvent(optionElements[0], 'click');
fixture.detectChanges();

expect(comboboxInstance.isOpen()).toBeFalse();
expect(comboboxInstance.value).toBe('purple');
});

it('should update combobox value on selection via keyboard', () => {
expect(comboboxInstance.value).toBeUndefined();
expect(comboboxInstance.isOpen()).toBeFalse();

dispatchMouseEvent(comboboxElement, 'click');
fixture.detectChanges();

listbox = fixture.debugElement.query(By.directive(CdkListbox));
listboxInstance = listbox.injector.get<CdkListbox<unknown>>(CdkListbox);
listboxElement = listbox.nativeElement;

options = fixture.debugElement.queryAll(By.directive(CdkOption));
optionInstances = options.map(o => o.injector.get<CdkOption>(CdkOption));
optionElements = options.map(o => o.nativeElement);

expect(comboboxInstance.isOpen()).toBeTrue();

listboxInstance.setActiveOption(optionInstances[1]);
dispatchKeyboardEvent(listboxElement, 'keydown', SPACE);
fixture.detectChanges();

expect(comboboxInstance.isOpen()).toBeFalse();
expect(comboboxInstance.value).toBe('solar');
});

it('should not update combobox if listbox is in multiple mode', () => {
expect(comboboxInstance.value).toBeUndefined();
expect(comboboxInstance.isOpen()).toBeFalse();

dispatchMouseEvent(comboboxElement, 'click');
fixture.detectChanges();

listbox = fixture.debugElement.query(By.directive(CdkListbox));
listboxInstance = listbox.injector.get<CdkListbox<unknown>>(CdkListbox);
listboxElement = listbox.nativeElement;

testComponent.isMultiselectable = true;
fixture.detectChanges();

options = fixture.debugElement.queryAll(By.directive(CdkOption));
optionInstances = options.map(o => o.injector.get<CdkOption>(CdkOption));
optionElements = options.map(o => o.nativeElement);

expect(comboboxInstance.isOpen()).toBeTrue();

listboxInstance.setActiveOption(optionInstances[1]);
dispatchKeyboardEvent(listboxElement, 'keydown', SPACE);
fixture.detectChanges();

expect(comboboxInstance.isOpen()).toBeTrue();
expect(comboboxInstance.value).toBeUndefined();
});
});
});

@Component({
Expand Down Expand Up @@ -862,3 +981,36 @@ class ListboxControlValueAccessor {
this.changedOption = event.option;
}
}

@Component({
template: `
<button cdkCombobox #toggleCombobox class="example-combobox"
[cdkComboboxTriggerFor]="panel"
[openActions]="'click'">
No Value
</button>

<ng-template cdkComboboxPanel #panel="cdkComboboxPanel">
<select cdkListbox
[parentPanel]="panel"
[disabled]="isDisabled"
[multiple]="isMultiselectable"
(selectionChange)="onSelectionChange($event)">
<option cdkOption [value]="'purple'">Purple</option>
<option cdkOption [value]="'solar'">Solar</option>
<option cdkOption [value]="'arc'">Arc</option>
<option cdkOption [value]="'stasis'">Stasis</option>
</select>
</ng-template>
`
})
class ListboxInsideCombobox {
changedOption: CdkOption<string>;
isDisabled: boolean = false;
isMultiselectable: boolean = false;
@ViewChild(CdkListbox) listbox: CdkListbox<string>;

onSelectionChange(event: ListboxSelectionChangeEvent<string>) {
this.changedOption = event.option;
}
}
58 changes: 56 additions & 2 deletions src/cdk-experimental/listbox/listbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
ContentChildren,
Directive,
ElementRef, EventEmitter, forwardRef,
Inject,
Input, OnDestroy, OnInit, Output,
Inject, InjectionToken,
Input, OnDestroy, OnInit, Optional, Output,
QueryList
} from '@angular/core';
import {ActiveDescendantKeyManager, Highlightable, ListKeyManagerOption} from '@angular/cdk/a11y';
Expand All @@ -22,15 +22,19 @@ import {SelectionChange, SelectionModel} from '@angular/cdk/collections';
import {defer, merge, Observable, Subject} from 'rxjs';
import {startWith, switchMap, takeUntil} from 'rxjs/operators';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
import {CdkComboboxPanel} from '@angular/cdk-experimental/combobox';

let nextId = 0;
let listboxId = 0;

export const CDK_LISTBOX_VALUE_ACCESSOR: any = {
provide: NG_VALUE_ACCESSOR,
useExisting: forwardRef(() => CdkListbox),
multi: true
};

export const PANEL = new InjectionToken<CdkComboboxPanel>('CdkComboboxPanel');

@Directive({
selector: '[cdkOption]',
exportAs: 'cdkOption',
Expand Down Expand Up @@ -172,6 +176,10 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab
}
}

getElementRef() {
return this._elementRef;
}

/** Sets the active property to true to enable the active css class. */
setActiveStyles() {
this._active = true;
Expand All @@ -191,6 +199,7 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab
exportAs: 'cdkListbox',
host: {
'role': 'listbox',
'[id]': 'id',
'(keydown)': '_keydown($event)',
'[attr.tabindex]': '_tabIndex',
'[attr.aria-disabled]': 'disabled',
Expand Down Expand Up @@ -231,6 +240,8 @@ export class CdkListbox<T> implements AfterContentInit, OnDestroy, OnInit, Contr
@Output() readonly selectionChange: EventEmitter<ListboxSelectionChangeEvent<T>> =
new EventEmitter<ListboxSelectionChangeEvent<T>>();

@Input() id = `cdk-option-${listboxId++}`;

/**
* Whether the listbox allows multiple options to be selected.
* If `multiple` switches from `true` to `false`, all options are deselected.
Expand Down Expand Up @@ -263,18 +274,27 @@ export class CdkListbox<T> implements AfterContentInit, OnDestroy, OnInit, Contr

@Input() compareWith: (o1: T, o2: T) => boolean = (a1, a2) => a1 === a2;

@Input('parentPanel') private readonly _explicitPanel: CdkComboboxPanel;

constructor(
private readonly _elementRef: ElementRef,
@Optional() @Inject(PANEL) readonly _parentPanel?: CdkComboboxPanel<T>,
) { }

ngOnInit() {
this._selectionModel = new SelectionModel<CdkOption<T>>(this.multiple);
}

ngAfterContentInit() {
this._initKeyManager();
this._initSelectionModel();
this._registerWithPanel();

this.optionSelectionChanges.subscribe(event => {
this._emitChangeEvent(event.source);
this._updateSelectionModel(event.source);
this.setActiveOption(event.source);
this._updatePanel(event.source);
});
}

Expand All @@ -284,6 +304,16 @@ export class CdkListbox<T> implements AfterContentInit, OnDestroy, OnInit, Contr
this._destroyed.complete();
}

private _registerWithPanel(): void {
if (this._parentPanel === null || this._parentPanel === undefined) {
if (this._explicitPanel !== null && this._explicitPanel !== undefined) {
this._explicitPanel._registerContent(this.id, 'listbox');
}
} else {
this._parentPanel._registerContent(this.id, 'listbox');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (this._parentPanel === null || this._parentPanel === undefined) {
if (this._explicitPanel !== null && this._explicitPanel !== undefined) {
this._explicitPanel._registerContent(this.id, 'listbox');
}
} else {
this._parentPanel._registerContent(this.id, 'listbox');
}
const panel = this._parentPanel || this._explicitPanel;
panel?._registerContent(this.id, 'listbox');

Similar for _updatePanel below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay that's simpler.

}

private _initKeyManager() {
this._listKeyManager = new ActiveDescendantKeyManager(this._options)
.withWrap()
Expand Down Expand Up @@ -358,6 +388,20 @@ export class CdkListbox<T> implements AfterContentInit, OnDestroy, OnInit, Contr
this._selectionModel.deselect(option);
}

_updatePanel(option: CdkOption<T>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_updatePanel(option: CdkOption<T>) {
_updatePanelForSelection(option: CdkOption<T>) {

const data = option.selected ? option.value : null;
if (!this.multiple) {
if (this._parentPanel === null || this._parentPanel === undefined) {
if (this._explicitPanel !== null && this._explicitPanel !== undefined) {
this._explicitPanel.closePanel(data);
}
} else {
option.selected ?
this._parentPanel.closePanel(option.value) : this._parentPanel.closePanel();
}
}
}

/** Toggles the selected state of the active option if not disabled. */
private _toggleActiveOption() {
const activeOption = this._listKeyManager.activeItem;
Expand All @@ -383,6 +427,10 @@ export class CdkListbox<T> implements AfterContentInit, OnDestroy, OnInit, Contr

if (!this.useActiveDescendant) {
this._activeOption.focus();
} else {
if (document.activeElement === this._activeOption.getElementRef().nativeElement) {
this._elementRef.nativeElement.focus();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite follow this- why would the focus move back to the listbox?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was something I added last night that might not make sense. But basically if the listbox is using aria-active descendant it was my understanding that we wouldn't want to focus the options on click. The way I had it implemented the options wouldn't focus when reached via keyboard but on click they would. This moved the focus back to the listbox.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit of a strange way to do it but that's what I could figure out last night. If there's a better way to do it then I can remove this for now and just note it as a current bug to fix this weekend.

}
}

Expand Down Expand Up @@ -420,6 +468,7 @@ export class CdkListbox<T> implements AfterContentInit, OnDestroy, OnInit, Contr
/** Updates the key manager's active item to the given option. */
setActiveOption(option: CdkOption<T>) {
this._listKeyManager.updateActiveItem(option);
this._updateActiveOption();
}

/**
Expand Down Expand Up @@ -450,6 +499,11 @@ export class CdkListbox<T> implements AfterContentInit, OnDestroy, OnInit, Contr
this.disabled = isDisabled;
}

/** Returns the values of the currently selected options. */
getSelectedValues(): T[] {
return this._options.filter(option => option.selected).map(option => option.value);
}

/** Selects an option that has the corresponding given value. */
private _setSelectionByValue(values: T | T[]) {
for (const option of this._options.toArray()) {
Expand Down