Skip to content

Commit

Permalink
Combobox listbox refactor (#20339)
Browse files Browse the repository at this point in the history 
* build: Added required files to listbox directory.

* build: added listbox option directive and renamed listbox directive files.

* build: Added required files to listbox directory.

* build: added listbox option directive and renamed listbox directive files.

* build: Added required files to listbox directory.

* build: added listbox option directive and renamed listbox directive files.

* build: Added required files to listbox directory.

* build: added listbox option directive and renamed listbox directive files.

* feat(dev-app/listbox): added cdk listbox example to the dev-app.

* fix(listbox): removed duplicate dep in dev-app build file.

* fix(listbox): deleted unused files.

* refactor(combobox): changed names and made coerceOpenActionProperty simpler for this PR.

* fix(combobox): fixed trailing whitespace.

* refactor(listbox): added default classes to listbox and option, and made aria-active descendant not the default.

* refactor(listbox): added focus management for focusing listbox.

* feat(listbox): added support for horizontal listbox.

* refactor(combobox): added support for array of values which enables multiselect listbox to be compatible.

* feat(combobox): let the user determine what element is focused first in the popup.

* feat(combobox): pressing down when focused on text trigger will move focus to popup.

* refactor(listbox): updated listbox key management to handle horizontal orientation.

* refactor(listbox): prevented listbox from auto closing when in multiple mode in a combobox.

* fix(combobox): fixed lint errors.

* refactor(combobox): made updates to focus management and keyboard handling.

* fix(combobox): fixed keyboard logic bug that only handled down arrow.

* fix(combobox): close popup on Tab.

* refactor(listbox): added comments and todos throughout.

* fix(listbox): fixed listbox id to be cdk-listbox instead of cdk-option.

* fix(listbox): prevent SPACE from scrolling scren.
  • Loading branch information
@nielsr98
nielsr98 committed Aug 18, 2020
1 parent 23d3c21 commit d7754e7
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 28 deletions.
9 changes: 7 additions & 2 deletions src/cdk-experimental/combobox/combobox-panel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {Subject} from 'rxjs';
})
export class CdkComboboxPanel<T = unknown> {

valueUpdated: Subject<T> = new Subject<T>();
valueUpdated: Subject<T | T[]> = new Subject<T | T[]>();
contentIdUpdated: Subject<string> = new Subject<string>();
contentTypeUpdated: Subject<AriaHasPopupValue> = new Subject<AriaHasPopupValue>();

Expand All @@ -32,7 +32,7 @@ export class CdkComboboxPanel<T = unknown> {
) {}

/** Tells the parent combobox to close the panel and sends back the content value. */
closePanel(data?: T) {
closePanel(data?: T | T[]) {
this.valueUpdated.next(data);
}

Expand All @@ -44,6 +44,11 @@ export class CdkComboboxPanel<T = unknown> {

/** Registers the content's id and the content type with the panel. */
_registerContent(contentId: string, contentType: AriaHasPopupValue) {
// If content has already been registered, no further contentIds are registered.
if (this.contentType && this.contentType !== contentType) {
return;
}

this.contentId = contentId;
if (contentType !== 'listbox' && contentType !== 'dialog') {
throw Error('CdkComboboxPanel currently only supports listbox or dialog content.');
Expand Down
30 changes: 28 additions & 2 deletions src/cdk-experimental/combobox/combobox-popup.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, Inject, InjectionToken, Input, OnInit, Optional} from '@angular/core';
import {
Directive,
ElementRef,
Inject,
InjectionToken,
Input,
OnInit,
Optional} from '@angular/core';
import {AriaHasPopupValue, CdkComboboxPanel} from './combobox-panel';

export const PANEL = new InjectionToken<CdkComboboxPanel>('CdkComboboxPanel');
Expand All @@ -20,7 +27,8 @@ let nextId = 0;
'class': 'cdk-combobox-popup',
'[attr.role]': 'role',
'[id]': 'id',
'tabindex': '-1'
'tabindex': '-1',
'(focus)': 'focusFirstElement()'
}
})
export class CdkComboboxPopup<T = unknown> implements OnInit {
Expand All @@ -33,11 +41,21 @@ export class CdkComboboxPopup<T = unknown> implements OnInit {
}
private _role: AriaHasPopupValue = 'dialog';

@Input()
get firstFocus(): HTMLElement {
return this._firstFocusElement;
}
set firstFocus(id: HTMLElement) {
this._firstFocusElement = id;
}
private _firstFocusElement: HTMLElement;

@Input() id = `cdk-combobox-popup-${nextId++}`;

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

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

Expand All @@ -52,4 +70,12 @@ export class CdkComboboxPopup<T = unknown> implements OnInit {
this._parentPanel._registerContent(this.id, this._role);
}
}

focusFirstElement() {
if (this._firstFocusElement) {
this._firstFocusElement.focus();
} else {
this._elementRef.nativeElement.focus();
}
}
}
54 changes: 44 additions & 10 deletions src/cdk-experimental/combobox/combobox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* 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 All @@ -30,7 +31,7 @@ import {
} from '@angular/cdk/overlay';
import {Directionality} from '@angular/cdk/bidi';
import {BooleanInput, coerceBooleanProperty, coerceArray} from '@angular/cdk/coercion';
import {DOWN_ARROW, ESCAPE} from '@angular/cdk/keycodes';
import {DOWN_ARROW, ENTER, ESCAPE, TAB} from '@angular/cdk/keycodes';

const allowedOpenActions = ['focus', 'click', 'downKey', 'toggle'];

Expand Down Expand Up @@ -58,7 +59,7 @@ export class CdkCombobox<T = unknown> implements OnDestroy, AfterContentInit {
private _panel: CdkComboboxPanel<T> | undefined;

@Input()
value: T;
value: T | T[];

@Input()
get disabled(): boolean { return this._disabled; }
Expand All @@ -74,9 +75,16 @@ export class CdkCombobox<T = unknown> implements OnDestroy, AfterContentInit {
}
private _openActions: OpenAction[] = ['click'];

/** Whether the textContent is automatically updated upon change of the combobox value. */
@Input()
get autoSetText(): boolean { return this._autoSetText; }
set autoSetText(value: boolean) { this._autoSetText = coerceBooleanProperty(value); }
private _autoSetText: boolean = true;

@Output('comboboxPanelOpened') readonly opened: EventEmitter<void> = new EventEmitter<void>();
@Output('comboboxPanelClosed') readonly closed: EventEmitter<void> = new EventEmitter<void>();
@Output('panelValueChanged') readonly panelValueChanged: EventEmitter<T> = new EventEmitter<T>();
@Output('panelValueChanged') readonly panelValueChanged: EventEmitter<T[]>
= new EventEmitter<T[]>();

private _overlayRef: OverlayRef;
private _panelContent: TemplatePortal;
Expand Down Expand Up @@ -114,14 +122,28 @@ export class CdkCombobox<T = unknown> implements OnDestroy, AfterContentInit {
_keydown(event: KeyboardEvent) {
const {keyCode} = event;

if (keyCode === DOWN_ARROW && this._openActions.indexOf('downKey') !== -1) {
this.open();
if (keyCode === DOWN_ARROW) {
if (this.isOpen()) {
this._panel?.focusContent();
} else if (this._openActions.indexOf('downKey') !== -1) {
this.open();
}
} else if (keyCode === ENTER) {
if (this._openActions.indexOf('toggle') !== -1) {
this.toggle();
} else if (this._openActions.indexOf('click') !== -1) {
this.open();
}

} else if (keyCode === ESCAPE) {
event.preventDefault();
this.close();
} else if (keyCode === TAB) {
this.close();
}
}

/** Handles click or focus interactions. */
_handleInteractions(interaction: OpenAction) {
if (interaction === 'click') {
if (this._openActions.indexOf('toggle') !== -1) {
Expand All @@ -136,6 +158,7 @@ export class CdkCombobox<T = unknown> implements OnDestroy, AfterContentInit {
}
}

/** Given a click in the document, determines if the click was inside a combobox. */
_attemptClose(event: MouseEvent) {
if (this.isOpen()) {
let target = event.composedPath ? event.composedPath()[0] : event.target;
Expand Down Expand Up @@ -163,7 +186,9 @@ export class CdkCombobox<T = unknown> implements OnDestroy, AfterContentInit {
this.opened.next();
this._overlayRef = this._overlayRef || this._overlay.create(this._getOverlayConfig());
this._overlayRef.attach(this._getPanelContent());
this._panel?.focusContent();
if (!this._isTextTrigger()) {
this._panel?.focusContent();
}
}
}

Expand All @@ -189,22 +214,30 @@ export class CdkCombobox<T = unknown> implements OnDestroy, AfterContentInit {
return this.disabled ? null : '0';
}

private _setComboboxValue(value: T) {
private _setComboboxValue(value: T | T[]) {

const valueChanged = (this.value !== value);
this.value = value;

if (valueChanged) {
this.panelValueChanged.emit(value);
this._setTextContent(value);
this.panelValueChanged.emit(coerceArray(value));
if (this._autoSetText) {
this._setTextContent(value);
}
}
}

private _setTextContent(content: T) {
private _setTextContent(content: T | T[]) {
const contentArray = coerceArray(content);
this._elementRef.nativeElement.textContent = contentArray.join(' ');
}

private _isTextTrigger() {
// TODO: Should check if the trigger is contenteditable.
const tagName = this._elementRef.nativeElement.tagName.toLowerCase();
return tagName === 'input' || tagName === 'textarea' ? true : false;
}

private _getOverlayConfig() {
return new OverlayConfig({
positionStrategy: this._getOverlayPositionStrategy(),
Expand Down Expand Up @@ -247,5 +280,6 @@ export class CdkCombobox<T = unknown> implements OnDestroy, AfterContentInit {
}

static ngAcceptInputType_openActions: OpenActionInput;
static ngAcceptInputType_autoSetText: OpenActionInput;
static ngAcceptInputType_disabled: BooleanInput;
}
10 changes: 6 additions & 4 deletions src/cdk-experimental/listbox/listbox.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ 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';
import {CdkCombobox, CdkComboboxModule, CdkComboboxPanel} from '@angular/cdk-experimental/combobox';


describe('CdkOption and CdkListbox', () => {
Expand Down Expand Up @@ -853,7 +853,7 @@ describe('CdkOption and CdkListbox', () => {
expect(comboboxInstance.value).toBe('solar');
});

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

Expand All @@ -875,10 +875,11 @@ describe('CdkOption and CdkListbox', () => {

listboxInstance.setActiveOption(optionInstances[1]);
dispatchKeyboardEvent(listboxElement, 'keydown', SPACE);
testComponent.panel.closePanel(testComponent.listbox.getSelectedValues());
fixture.detectChanges();

expect(comboboxInstance.isOpen()).toBeTrue();
expect(comboboxInstance.value).toBeUndefined();
expect(comboboxInstance.isOpen()).toBeFalse();
expect(comboboxInstance.value).toEqual(['solar']);
});
});
});
Expand Down Expand Up @@ -1009,6 +1010,7 @@ class ListboxInsideCombobox {
isDisabled: boolean = false;
isMultiselectable: boolean = false;
@ViewChild(CdkListbox) listbox: CdkListbox<string>;
@ViewChild('panel') panel: CdkComboboxPanel<string>;

onSelectionChange(event: ListboxSelectionChangeEvent<string>) {
this.changedOption = event.option;
Expand Down

0 comments on commit d7754e7

Please sign in to comment.