[Query Builder] Refactor, bug fixing and test coverage improved

CHANGELOG.md

@@ -58,6 +58,13 @@ All notable changes for each version of this project will be documented in this
     - Setting `expandChildren` now also applies to child grids that are not in the DOM at the time, for example scrolled out of view, when they are rendered again. Previously they kept their previous state.
     - A row island added conditionally inside another row island, for example with `@if`, as well as a nested `igc-row-island` in Ignite UI for Web Components, is now registered under its parent row island. Previously it was registered as a top-level row island, so its child grids were missing from the parent row island's child grids and were not cleaned up when the row island was removed.
     - `resourceStrings` now returns the resource strings of the root grid when none are set on the row island, as documented, instead of the default ones.
+- `IgxQueryBuilderComponent`
+    - When reordering conditions with the keyboard, `Space` now drops the dragged condition at the chosen location, like `Enter`. Previously it did nothing.
+    - Pressing `Enter` or `Space` during a keyboard drag before a drop location is chosen no longer deletes the dragged condition.
+    - During a keyboard drag, the arrow keys and the `Enter`/`Space` keys that drop the condition no longer scroll the page or the expressions container.
+    - During a keyboard drag, `Tab` and `Shift+Tab` are ignored while the drop ghost is shown, so focus stays on it until the condition is dropped (`Enter`/`Space`) or the drag is cancelled (`Escape`). Previously they cancelled the drag and focus was lost.
+    - While a condition is dragged with the mouse, the arrow keys, `Enter` and `Space` no longer move or drop it, and focusing another condition's drag indicator no longer starts a keyboard drag. Previously, after moving focus into the tree, the keyboard could move the drop ghost or drop the condition while the mouse button was still held.
+    - Selecting a `time` field that has no `editorOptions` no longer throws an error.
 
 ## 22.2.0
 

projects/igniteui-angular/query-builder/src/query-builder/query-builder-drag.service.ts

@@ -1,4 +1,4 @@
-import { filter, fromEvent, sampleTime, Subscription } from 'rxjs';
+import { filter, fromEvent, sampleTime, Subscription, tap } from 'rxjs';
 import { IgxQueryBuilderTreeComponent } from './query-builder-tree.component';
 import { ElementRef, Injectable } from '@angular/core';
 import { ExpressionGroupItem, ExpressionItem, ExpressionOperandItem, QueryBuilderSelectors } from './query-builder.common';
@@ -44,6 +44,16 @@ export class IgxQueryBuilderDragService {
         return this._queryBuilderTreeComponentElRef.nativeElement.querySelector(`.${QueryBuilderSelectors.FILTER_TREE}`);
     }
 
+    /** Whether a chip is currently being dragged with the mouse */
+    private get isMouseDragInProgress(): boolean {
+        return !!this._sourceElement && !this.isKeyboardDrag;
+    }
+
+    /** Whether a chip is currently being dragged with the keyboard */
+    private get isKeyboardDragInProgress(): boolean {
+        return !!this._sourceElement && this.isKeyboardDrag;
+    }
+
 
     public register(tree: IgxQueryBuilderTreeComponent, el: ElementRef) {
         this._queryBuilderTreeComponent = tree;
@@ -239,8 +249,8 @@ export class IgxQueryBuilderDragService {
      *
     */
     public onChipDragIndicatorFocus(sourceDragElement: HTMLElement, sourceExpressionItem: ExpressionItem) {
-        //if drag is not underway, already
-        if (!this.getDropGhostElement) {
+        //if drag is not underway, already (a mouse drag may not have rendered a drop ghost yet)
+        if (!this.getDropGhostElement && !this.isMouseDragInProgress) {
             this.onMoveStart(sourceDragElement, sourceExpressionItem, true);
         }
     }
@@ -378,13 +388,16 @@ export class IgxQueryBuilderDragService {
     private listenToKeyboard() {
         this._keyboardSubscription$?.unsubscribe();
         this._keyboardSubscription$ = fromEvent<KeyboardEvent>(this.getMainExpressionTree, 'keydown')
-            .pipe(filter(e => ['ArrowUp', 'ArrowDown', 'Enter', 'Space', 'Escape', 'Tab'].includes(e.key)))
-            // .pipe(tap(e => {
-            //     //Inhibit Tabs if keyboard drag is underway (don't allow to loose focus of the drop ghost's drag indicator)
-            //     if (e.key === 'Tab' && this.getDropGhostElement) {
-            //         e.preventDefault();
-            //     }
-            // }))
+            .pipe(filter(e => ['ArrowUp', 'ArrowDown', 'Enter', ' ', 'Space', 'Escape', 'Tab'].includes(e.key)))
+            //Keep the browser from scrolling on the keys that move or drop a keyboard-dragged chip (held keys included)
+            //and inhibit Tabs once there's a drop ghost (don't allow to lose focus of the drop ghost's drag indicator)
+            .pipe(tap(e => {
+                if (this.isKeyboardDragInProgress &&
+                    (e.key === 'ArrowUp' || e.key === 'ArrowDown' ||
+                        (this.dropGhostExpression && (e.key === 'Enter' || e.key === ' ' || e.key === 'Space' || e.key === 'Tab')))) {
+                    e.preventDefault();
+                }
+            }))
             .pipe(filter(event => !event.repeat))
             .subscribe(e => {
                 if (e.key === 'Escape') {
@@ -394,18 +407,24 @@ export class IgxQueryBuilderDragService {
                     if (this.isKeyboardDrag) {
                         (this._sourceElement.firstElementChild!.firstElementChild!.firstElementChild!.firstElementChild as HTMLElement).focus();
                     }
+                } else if (this.isMouseDragInProgress) {
+                    //A mouse-dragged chip follows the pointer only and is dropped on release
+                    return;
                 } else if (e.key === 'ArrowUp' || e.key === 'ArrowDown') {
                     this.arrowDrag(e.key);
-                } else if (e.key === 'Enter' || e.key === 'Space') {
+                } else if (e.key === 'Enter' || e.key === ' ' || e.key === 'Space') {
                     //this.platform.isActivationKey(eventArgs) Maybe use this rather that Enter/Space?
-                    this.onChipDropped();
-                    this._keyboardSubscription$.unsubscribe();
+                    //Without a drop ghost there is no drop target yet; committing would only delete the source expression.
+                    if (this.dropGhostExpression) {
+                        this.onChipDropped();
+                        this._keyboardSubscription$.unsubscribe();
+                    }
                 }
             });
     }
 
     /** Perform up/down movement of drop ghost along the expression tree*/
-    private arrowDrag(key: string) {
+    private arrowDrag(key: 'ArrowUp' | 'ArrowDown') {
         if (!this._sourceElement || !this._sourceExpressionItem) {
             return;
         }
@@ -426,12 +445,9 @@ export class IgxQueryBuilderDragService {
         if (key === 'ArrowUp') {
             //decrease index capped at top of tree
             newKeyIndexOffset && newKeyIndexOffset--;
-        } else if (key === 'ArrowDown') {
+        } else {
             //increase index capped at bottom of tree
             newKeyIndexOffset < this._possibleDropLocations.length - 1 && newKeyIndexOffset++;
-        } else {
-            console.error('wrong key');
-            return;
         }
 
         //if drop location has no change

projects/igniteui-angular/query-builder/src/query-builder/query-builder-tree.component.html

@@ -507,7 +507,7 @@
                 [locale]="this.locale"
                 [formatter]="selectedField.formatter!"
                 [displayFormat]="selectedField.pipeArgs!.format!"
-                [inputFormat]="selectedField.editorOptions!.dateTimeFormat!"
+                [inputFormat]="selectedField.editorOptions?.dateTimeFormat!"
                 [placeholder]="
                   this.resourceStrings.igx_query_builder_time_placeholder
                 !"

projects/igniteui-angular/query-builder/src/query-builder/query-builder-tree.component.ts

@@ -300,9 +300,6 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
     @ViewChild('picker')
     private picker!: IgxDatePickerComponent | IgxTimePickerComponent;
 
-    @ViewChild('addRootAndGroupButton', { read: ElementRef })
-    private addRootAndGroupButton!: ElementRef;
-
     @ViewChild('addConditionButton', { read: ElementRef })
     private addConditionButton!: ElementRef;
 
@@ -333,11 +330,6 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
         this._editingInputsContainer = value;
     }
 
-    /** @hidden */
-    protected get editingInputsContainer(): ElementRef {
-        return this._editingInputsContainer;
-    }
-
     @ViewChild('currentGroupButtonsContainer', { read: ElementRef })
     protected set currentGroupButtonsContainer(value: ElementRef) {
         if ((value && !this._currentGroupButtonsContainer) ||
@@ -350,11 +342,6 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
         this._currentGroupButtonsContainer = value;
     }
 
-    /** @hidden */
-    protected get currentGroupButtonsContainer(): ElementRef {
-        return this._currentGroupButtonsContainer;
-    }
-
     @ViewChild('expressionsContainer')
     private expressionsContainer!: ElementRef;
 
@@ -471,15 +458,14 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
     private _focusDelay = DEFAULT_CHIP_FOCUS_DELAY;
     private _parentExpression!: ExpressionOperandItem;
     private _selectedEntity!: EntityType;
-    private _selectedReturnFields!: string | string[];
+    private _selectedReturnFields!: string[];
     private _selectedField: FieldType | null = null;
     private _editingInputsContainer!: ElementRef;
     private _currentGroupButtonsContainer!: ElementRef;
     private _addModeExpression!: ExpressionOperandItem;
     private _editedExpression!: ExpressionOperandItem;
     private _preventInit = false;
     private _prevFocusedContainer: HTMLElement | null = null;
-    private _expandedExpressions: IFilteringExpression[] = [];
     private _fields!: FieldType[];
     private _locale?: string;
     private _defaultLocale!: string;
@@ -634,8 +620,7 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
         if (this._expressionTree) {
             this._expressionTree.entity = this._entityNewValue.name;
 
-            const returnFields = Array.isArray(this._selectedReturnFields) ? this._selectedReturnFields : [this._selectedReturnFields];
-            this._expressionTree.returnFields = this.fields.length === returnFields.length ? ['*'] : returnFields;
+            this._expressionTree.returnFields = this.fields.length === this._selectedReturnFields.length ? ['*'] : this._selectedReturnFields;
 
             this._expressionTree.filteringOperands = [];
 
@@ -679,9 +664,6 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
      * @hidden @internal
      */
     public get selectedReturnFields(): string[] {
-        if (typeof this._selectedReturnFields == 'string') {
-            return [this._selectedReturnFields];
-        }
         return this._selectedReturnFields;
     }
 
@@ -786,13 +768,6 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
         }
     }
 
-    /**
-     * @hidden @internal
-     */
-    public endGroup(groupItem: ExpressionGroupItem) {
-        this.currentGroup = groupItem.parent;
-    }
-
     /**
      * @hidden @internal
      */
@@ -1212,13 +1187,6 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
         this.cdr.detectChanges();
     }
 
-    /**
-     * @hidden @internal
-     */
-    public onKeyDown(eventArgs: KeyboardEvent) {
-        eventArgs.stopPropagation();
-    }
-
     /**
      * @hidden @internal
      */
@@ -1393,11 +1361,7 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
      * used by the grid
      */
     public setAddButtonFocus() {
-        if (this.addRootAndGroupButton) {
-            this.addRootAndGroupButton.nativeElement.focus();
-        } else if (this.addConditionButton) {
-            this.addConditionButton.nativeElement.focus();
-        }
+        this.addConditionButton?.nativeElement.focus();
     }
 
     /**
@@ -1586,9 +1550,6 @@ export class IgxQueryBuilderTreeComponent implements AfterViewInit, OnDestroy {
                     const operandItem = new ExpressionOperandItem(exprCopy, groupItem);
                     const field = this.fields?.find(el => el.field === filteringExpr.fieldName);
                     operandItem.fieldLabel = field?.label || field?.header || field?.field!;
-                    if (this._expandedExpressions.filter(e => e.searchTree == operandItem.expression.searchTree).length > 0) {
-                        operandItem.expanded = true;
-                    }
                     groupItem.children.push(operandItem);
                 }
             }