From f599832c18a8c9f75cc7c0faf032002ca544fa67 Mon Sep 17 00:00:00 2001 From: Antoni Dalmases Date: Mon, 28 Jun 2021 13:45:31 +0200 Subject: [PATCH] fix(filter-field): Multiselect keyboard interaction not working correctly --- .../filter-field/filter-field.e2e.ts | 4 ++-- .../filter-field-multi-select-trigger.ts | 4 ++-- .../filter-field-multi-select.ts | 16 ++++++++++----- .../filter-field/src/filter-field.ts | 20 ++++++++++++------- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/apps/components-e2e/src/components/filter-field/filter-field.e2e.ts b/apps/components-e2e/src/components/filter-field/filter-field.e2e.ts index 84331cab44..a3e9c2d1d3 100644 --- a/apps/components-e2e/src/components/filter-field/filter-field.e2e.ts +++ b/apps/components-e2e/src/components/filter-field/filter-field.e2e.ts @@ -340,11 +340,11 @@ test('should choose a multiselect node with the keyboard and submit the correct await testController // Select the multiselect node .pressKey('down down down enter') - // Wait for a certain amout fo time to let the filterfield refresh + // Wait for a certain amount of time to let the filterfield refresh .wait(250) // Select the desired option .pressKey('down space enter') - // Wait for a certain amout fo time to let the filterfield refresh + // Wait for a certain amount of time to let the filterfield refresh .wait(250); const tags = await getFilterfieldTags(); diff --git a/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select-trigger.ts b/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select-trigger.ts index 4bc47aa12c..cfa373b010 100644 --- a/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select-trigger.ts +++ b/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select-trigger.ts @@ -55,7 +55,8 @@ import { DtFilterFieldMultiSelect } from './filter-field-multi-select'; }) export class DtFilterFieldMultiSelectTrigger extends DtFilterFieldElementTrigger> - implements OnDestroy { + implements OnDestroy +{ /** The filter-field multiSelect panel to be attached to this trigger. */ @Input('dtFilterFieldMultiSelect') get element(): DtFilterFieldMultiSelect { @@ -106,7 +107,6 @@ export class DtFilterFieldMultiSelectTrigger openPanel(): void { if (!this.element._isOpen) { super.openPanel(); - this._element.focus(); } } diff --git a/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select.ts b/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select.ts index 0d2d4ba546..49f9b2c493 100644 --- a/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select.ts +++ b/libs/barista-components/filter-field/src/filter-field-multi-select/filter-field-multi-select.ts @@ -63,7 +63,8 @@ export class DtFilterFieldMultiSelectSubmittedEvent { changeDetection: ChangeDetectionStrategy.OnPush, }) export class DtFilterFieldMultiSelect - implements DtFilterFieldElement, AfterViewInit { + implements DtFilterFieldElement, AfterViewInit +{ /** * Whether the first option should be highlighted when the multi-select panel is opened. * Can be configured globally through the `DT_MULTI_SELECT_DEFAULT_OPTIONS` token. @@ -190,9 +191,11 @@ export class DtFilterFieldMultiSelect takeUntil(this._destroy$), ) .subscribe((option) => { - this._ngZone?.run(() => { - this._keyManager.setActiveItem(option); - }); + if (!option.disabled) { + this._ngZone?.run(() => { + this._keyManager.setActiveItem(option); + }); + } }); } @@ -330,6 +333,9 @@ export class DtFilterFieldMultiSelect } focus(): void { - this._keyManager.setActiveItem(this._options.first); + const firstOption = this._options.find((option) => !option.disabled); + if (firstOption) { + this._keyManager.setActiveItem(firstOption); + } } } diff --git a/libs/barista-components/filter-field/src/filter-field.ts b/libs/barista-components/filter-field/src/filter-field.ts index 0c43128049..7ab0f6db79 100644 --- a/libs/barista-components/filter-field/src/filter-field.ts +++ b/libs/barista-components/filter-field/src/filter-field.ts @@ -22,13 +22,14 @@ import { useAnimation, } from '@angular/animations'; import { FocusMonitor } from '@angular/cdk/a11y'; -import { coerceBooleanProperty, BooleanInput } from '@angular/cdk/coercion'; +import { BooleanInput, coerceBooleanProperty } from '@angular/cdk/coercion'; import { BACKSPACE, DELETE, ENTER, ESCAPE, LEFT_ARROW, + SPACE, UP_ARROW, } from '@angular/cdk/keycodes'; import { DOCUMENT } from '@angular/common'; @@ -59,12 +60,12 @@ import { DtAutocompleteTrigger, } from '@dynatrace/barista-components/autocomplete'; import { + _readKeyCode, CanDisable, DT_ERROR_ENTER_ANIMATION, DT_ERROR_ENTER_DELAYED_ANIMATION, ErrorStateMatcher, isDefined, - _readKeyCode, } from '@dynatrace/barista-components/core'; import { fromEvent, @@ -129,6 +130,7 @@ import { } from './filter-field-util'; import { DtFilterFieldControl } from './filter-field-validation'; import { + _getSourcesOfDtFilterValues, DefaultSearchOption, DtAutocompleteValue, DtFilterFieldTagData, @@ -151,7 +153,6 @@ import { isDtRangeDef, isDtRangeValue, isPartialDtOptionDef, - _getSourcesOfDtFilterValues, } from './types'; // tslint:disable:no-any @@ -207,7 +208,8 @@ let currentlyOpenFilterField: DtFilterField | null = null; ], }) export class DtFilterField - implements CanDisable, OnInit, AfterViewInit, OnDestroy, OnChanges { + implements CanDisable, OnInit, AfterViewInit, OnDestroy, OnChanges +{ /** Label for the filter field (e.g. "Filter by"). Will be placed next to the filter icon. */ @Input() label = ''; @@ -610,6 +612,7 @@ export class DtFilterField return; } else if (isDtMultiSelectDef(this._currentDef)) { this._multiSelectTrigger.openPanel(); + this._multiSelectTrigger.element.focus(); } // It is necessary to restore the focus back to the input field // so the user can directly continue creating more filter nodes. @@ -826,6 +829,8 @@ export class DtFilterField } } else if (keyCode === ESCAPE || (keyCode === UP_ARROW && event.altKey)) { this._closeFilterPanels(); + } else if (isDtMultiSelectDef(this._currentDef) && keyCode === SPACE) { + event.preventDefault(); } else { if (this._inputFieldKeyboardLocked) { return; @@ -1107,7 +1112,8 @@ export class DtFilterField const recentRangeValue = removed[0]; if (isDtRangeValue(recentRangeValue) && this._currentDef.range) { // Needed to set this in typescript, because template binding of input would be evaluated to late. - this._filterfieldRange.enabledOperators = this._currentDef.range.operatorFlags; + this._filterfieldRange.enabledOperators = + this._currentDef.range.operatorFlags; this._filterfieldRange._setValues(recentRangeValue.range); this._filterfieldRange._setOperator( recentRangeValue.operator as DtFilterFieldRangeOperator, @@ -1545,7 +1551,7 @@ export class DtFilterField return observableOf(); } - return (merge( + return merge( fromEvent(this._document, 'click'), fromEvent(this._document, 'touchend'), ).pipe( @@ -1558,7 +1564,7 @@ export class DtFilterField (!filterField || !filterField.contains(clickTarget)) ); }), - ) as any) as Observable; + ) as any as Observable; } /**