Skip to content
This repository was archived by the owner on Mar 6, 2024. It is now read-only.

Datagrid filter setFocus #542

Open
irahov wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Datagrid filter setFocus #542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d12f5e9
Refactor DatagridFilter.filterContainer to be protected.
Nov 27, 2023
bd2502b
Allow DatagridFilter to set focus on its elements
Nov 27, 2023
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
25 changes: 22 additions & 3 deletions projects/components/src/datagrid/filters/datagrid-filter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*!
* Copyright 2019 VMware, Inc.
* Copyright 2019-2023 VMware, Inc.
* SPDX-License-Identifier: BSD-2-Clause
*/

import { Directive, Input, OnInit } from '@angular/core';
import { FormGroup } from '@angular/forms';
import { ClrDatagridFilter, ClrDatagridFilterInterface } from '@clr/angular';
import { Subject } from 'rxjs';
import { Subject, timer } from 'rxjs';
import { debounceTime } from 'rxjs/operators';
import { SubscriptionTracker } from '../../common/subscription/subscription-tracker';
import {
Expand Down Expand Up @@ -64,7 +64,10 @@ export abstract class DatagridFilter<V, C extends FilterConfig<V>, F extends For
{
abstract formGroup: F;

protected constructor(filterContainer: ClrDatagridFilter, private subscriptionTracker: SubscriptionTracker) {
protected constructor(
protected filterContainer: ClrDatagridFilter,
private subscriptionTracker: SubscriptionTracker
Copy link
Contributor

@juanmendes juanmendes Dec 14, 2023

Choose a reason for hiding this comment

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

Since we're touching this file, we may as well remove the providers from here since it doesn't have any effect, the concrete classes need to add the providers.

Copy link
Contributor

@juanmendes juanmendes Dec 14, 2023

Choose a reason for hiding this comment

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

@kepelrs discovered this while analyzing the SubscriptionTracker

Copy link

@kepelrs kepelrs Dec 14, 2023

Choose a reason for hiding this comment

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

IIRC it was @irahov who discovered it first :)

) {
filterContainer.setFilter(this);
}

Expand Down Expand Up @@ -96,6 +99,17 @@ export abstract class DatagridFilter<V, C extends FilterConfig<V>, F extends For
? this.formGroup.valueChanges.pipe(debounceTime(this.getDebounceTimeMs()))
: this.formGroup.valueChanges;
this.subscriptionTracker.subscribe(obs, () => this.changes.next(null));
this.subscribeToSetFocusWhenOpened();
}

private subscribeToSetFocusWhenOpened(): void {
this.subscriptionTracker.subscribe(this.filterContainer.openChange, (open: boolean) => {
if (open) {
this.subscriptionTracker.subscribe(timer(50), () => {
this.setFocus();
});
}
});
}

/**
Expand Down Expand Up @@ -128,6 +142,11 @@ export abstract class DatagridFilter<V, C extends FilterConfig<V>, F extends For
*/
abstract isActive(): boolean;

/**
* Called when the filter is opened to allow setting the focus on the desired element
*/
protected abstract setFocus(): void;

/**
* Required by Clarity but ignored since we don't support client side filtering
*/
Expand Down
10 changes: 7 additions & 3 deletions projects/components/src/datagrid/filters/datagrid-multiselect-filter.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/*!
* Copyright 2019 VMware, Inc.
* Copyright 2019-2023 VMware, Inc.
* SPDX-License-Identifier: BSD-2-Clause
*/

import { Component, Host, OnDestroy } from '@angular/core';
import { Component, ElementRef } from '@angular/core';
import { AbstractControl, FormControl, FormGroup } from '@angular/forms';
import { ClrDatagridFilter } from '@clr/angular';
import { SelectOption } from '../../common/interfaces/select-option';
Expand Down Expand Up @@ -71,7 +71,7 @@ type BooleanFormGroup = FormGroup<{ [name: string]: AbstractControl<boolean, boo
providers: [SubscriptionTracker],
})
export class DatagridMultiSelectFilterComponent extends DatagridFilter<string[], DatagridMultiSelectFilterConfig> {
constructor(private filterContainer: ClrDatagridFilter, subTracker: SubscriptionTracker) {
constructor(filterContainer: ClrDatagridFilter, subTracker: SubscriptionTracker, private elemRef: ElementRef) {
super(filterContainer, subTracker);
}

Expand Down Expand Up @@ -127,6 +127,10 @@ export class DatagridMultiSelectFilterComponent extends DatagridFilter<string[],
!!Object.keys(this.formGroup.getRawValue()).filter((frmCtrl) => this.formGroup.get(frmCtrl).value).length
);
}

protected setFocus() {
Copy link
Contributor

@juanmendes juanmendes Dec 14, 2023

Choose a reason for hiding this comment

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

I think a better approach is to have a method called getFocusableElement(), then the callers just have to return the element instead of every implementation calling focus().

Copy link
Contributor Author

@irahov irahov Dec 14, 2023

Choose a reason for hiding this comment

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

I like the focus() since it seems a more complete approach to me.
If I come accross to getFocusableElement() function - it is not that clear how the returned element will be used. Yes, the name suggests to focus, but yet - not that clean. In addition, I'm not sure if there is a case when something else may be needed when focusing the element. On the other hand focus() is more concise and clear.

I agree that the very .focus() call can be reused in the base class but I don't see it has the advantages as the other approach.

this.elemRef?.nativeElement?.querySelector('input')?.focus();
}
}

/**
Expand Down
12 changes: 8 additions & 4 deletions projects/components/src/datagrid/filters/datagrid-numeric-filter.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/*!
* Copyright 2019 VMware, Inc.
* Copyright 2019-2023 VMware, Inc.
* SPDX-License-Identifier: BSD-2-Clause
*/

import { Component, Host, Input, OnDestroy, OnInit, ViewChild } from '@angular/core';
import { FormBuilder, FormControl, FormGroup } from '@angular/forms';
import { Component, ElementRef, Input, OnInit, ViewChild } from '@angular/core';
import { FormControl, FormGroup } from '@angular/forms';
import { ClrDatagridFilter } from '@clr/angular';
import { SubscriptionTracker } from '../../common/subscription/subscription-tracker';
import { NumberWithUnitFormInputComponent } from '../../form';
Expand Down Expand Up @@ -79,7 +79,7 @@ export class DatagridNumericFilterComponent
to: new FormControl(null as number),
});

constructor(private filterContainer: ClrDatagridFilter, subTracker: SubscriptionTracker) {
constructor(filterContainer: ClrDatagridFilter, subTracker: SubscriptionTracker, private elemRef: ElementRef) {
super(filterContainer, subTracker);
}

Expand Down Expand Up @@ -130,6 +130,10 @@ export class DatagridNumericFilterComponent
close(): void {
this.filterContainer.open = false;
}

protected setFocus() {
this.elemRef?.nativeElement?.querySelector('input')?.focus();
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion projects/components/src/datagrid/filters/datagrid-select-filter.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div class="clr-form-control">
<div class="clr-control-container">
<div class="clr-select-wrapper">
<select [formControl]="formGroup.controls.filterSelect" class="clr-select">
<select #selectElement [formControl]="formGroup.controls.filterSelect" class="clr-select">
<option [ngValue]="anyChoice.value">{{ anyChoice.display }}</option>
<option *ngFor="let option of config.options" [ngValue]="option.value">
{{ option.isTranslatable ? (option.display | translate) : option.display }}
Expand Down
12 changes: 9 additions & 3 deletions projects/components/src/datagrid/filters/datagrid-select-filter.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/*!
* Copyright 2019 VMware, Inc.
* Copyright 2019-2023 VMware, Inc.
* SPDX-License-Identifier: BSD-2-Clause
*/
import { Component, OnInit } from '@angular/core';
import { Component, ElementRef, OnInit, ViewChild } from '@angular/core';
import { FormBuilder, FormControl, FormGroup } from '@angular/forms';
import { ClrDatagridFilter } from '@clr/angular';
import { SelectOption } from '../../common/interfaces/select-option';
Expand Down Expand Up @@ -56,6 +56,8 @@ export class DatagridSelectFilterComponent
extends DatagridFilter<string | number, DatagridSelectFilterConfig>
implements OnInit
{
@ViewChild('selectElement') selectElement: ElementRef;

/**
* Displayed as the first option with a falsy value. Selecting this option would deactivate the filter
*/
Expand All @@ -68,7 +70,7 @@ export class DatagridSelectFilterComponent
filterSelect: new FormControl('' as string | number),
});

constructor(private filterContainer: ClrDatagridFilter, private fb: FormBuilder, subTracker: SubscriptionTracker) {
constructor(filterContainer: ClrDatagridFilter, private fb: FormBuilder, subTracker: SubscriptionTracker) {
super(filterContainer, subTracker);
}

Expand Down Expand Up @@ -97,6 +99,10 @@ export class DatagridSelectFilterComponent
isActive(): boolean {
return !!(this.formGroup && this.formGroup.controls.filterSelect.value);
}

protected setFocus() {
this.selectElement?.nativeElement.focus();
}
}

/**
Expand Down
1 change: 1 addition & 0 deletions projects/components/src/datagrid/filters/datagrid-string-filter.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<form>
<input
#inputElement
type="text"
name="search"
class="clr-input"
Expand Down
12 changes: 9 additions & 3 deletions projects/components/src/datagrid/filters/datagrid-string-filter.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/*!
* Copyright 2019 VMware, Inc.
* Copyright 2019-2023 VMware, Inc.
* SPDX-License-Identifier: BSD-2-Clause
*/

import { Component } from '@angular/core';
import { Component, ElementRef, ViewChild } from '@angular/core';
import { FormControl, FormGroup } from '@angular/forms';
import { ClrDatagridFilter } from '@clr/angular';
import { SubscriptionTracker } from '../../common/subscription/subscription-tracker';
Expand Down Expand Up @@ -37,10 +37,12 @@ export class DatagridStringFilterComponent extends DatagridFilter<string, Datagr
filterText: new FormControl(''),
});

@ViewChild('inputElement') inputElement: ElementRef;

protected placeholder: LazyString;

constructor(
private filterContainer: ClrDatagridFilter,
filterContainer: ClrDatagridFilter,
private translationService: TranslationService,
subTracker: SubscriptionTracker
) {
Expand Down Expand Up @@ -71,6 +73,10 @@ export class DatagridStringFilterComponent extends DatagridFilter<string, Datagr
return !!(this.formGroup && this.formGroup.controls.filterText.value);
}

protected setFocus() {
this.inputElement?.nativeElement.focus();
}

/**
* Wraps a string with a `wrapCharacter` in given position;
*/
Expand Down