Skip to content
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.

Sign up for GitHub

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

Table: Fix Advance filtering table #15811

Closed
wants to merge 1 commit into from
Closed

Table: Fix Advance filtering table #15811

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 52 additions & 3 deletions src/app/components/table/table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5397,6 +5397,8 @@ export class ColumnFilter implements AfterContentInit {

applyHasBeenClicked: boolean = false;

private appliedFilter: any;

get fieldConstraints(): FilterMetadata[] | undefined | null {
return this.dt.filters ? <FilterMetadata[]>this.dt.filters[<string>this.field] : null;
}
Expand Down Expand Up @@ -5653,6 +5655,9 @@ export class ColumnFilter implements AfterContentInit {
onEscape() {
if (this.hasFilterNotBeenApplied()) {
this.clearFilter();
if (this.appliedFilter !== null) {
this.dt.filters[<string>this.field] = this.appliedFilter;
}
}
this.applyHasBeenClicked = false;
this.overlayVisible = false;
Expand Down Expand Up @@ -5748,15 +5753,52 @@ export class ColumnFilter implements AfterContentInit {
hasFilter(): boolean {
let fieldFilter = this.dt.filters[<string>this.field];
if (fieldFilter) {
if (Array.isArray(fieldFilter)) return !this.dt.isFilterBlank((<FilterMetadata[]>fieldFilter)[0].value);
else return !this.dt.isFilterBlank(fieldFilter.value);
if (Array.isArray(fieldFilter)) {
if (this.dt.isFilterBlank((<FilterMetadata[]>fieldFilter)[0].value)) {
this.appliedFilter = null;
}
return !this.dt.isFilterBlank((<FilterMetadata[]>fieldFilter)[0].value);
} else {
if (this.dt.isFilterBlank(fieldFilter.value)) {
this.appliedFilter = null;
}
return !this.dt.isFilterBlank(fieldFilter.value);
}
}

return false;
}

hasFilterNotBeenApplied(): boolean {
return this.hasFilter() && !this.applyHasBeenClicked;
let currentFilters = this.dt.filters[<string>this.field];
let lastAppliedFilters = this.appliedFilter;

if (this.applyHasBeenClicked) {
return false;
}
return this.compareFilters(currentFilters, lastAppliedFilters);
}

compareFilters(currentFilters: any, appliedFilters: any): boolean {
if (appliedFilters === null) {
return true;
}
if (Array.isArray(currentFilters)) {
if (currentFilters.length !== appliedFilters.length) {
return true;
}
for (let i = 0; i < currentFilters.length; i++) {
if (currentFilters[i].value !== appliedFilters[i].value) {
return true;
}
}
} else {
if (currentFilters.value !== appliedFilters.value) {
return true;
}
}

return false;
}

isOutsideClicked(event: any): boolean {
Expand Down Expand Up @@ -5835,6 +5877,11 @@ export class ColumnFilter implements AfterContentInit {
hide() {
if (this.hasFilterNotBeenApplied()) {
this.clearFilter();
// apply the previous filter if the user has not clicked the apply button
if (this.appliedFilter !== null) {
this.dt.filters[<string>this.field] = this.appliedFilter;
this.dt._filter();
Copy link
Contributor

@rosenthalj rosenthalj Jun 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In hide, which can be called programmatically and possibly in other places where I have not "tried" to identify, you don't want to clear all filters which updates the table and then apply the old filter to restore the table, if the filters never changed.

I personally work with tables that have over 5000 rows that are not lazy loaded. I actually added a "close" button to the body of the advance filters and this button calls the hide() method

}
}
this.applyHasBeenClicked = false;
this.overlayVisible = false;
Expand All @@ -5856,6 +5903,8 @@ export class ColumnFilter implements AfterContentInit {
}

applyFilter() {
// deep copy of current applied filter
this.appliedFilter = JSON.parse(JSON.stringify(this.dt.filters[<string>this.field]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes to the applyFilter method will not fix the case where the filter is saved not using the "apply" filter button.

I believe this change (note: I have not verified this using your branch) will not fix Issue #15778. Please see Issue #15778. I added a video that demonstrates the issue. In Issue #15778 it describes the steps to reproduce the bug in the genral bug description.

If the filter is a multi-select filter, the apply button will not be called and these changes will have no effect. At the very end of the video that I include for issue #15804, the advance filters are broken using a multi-select filter and the apply button is not pressed.

I have not verified the issue with all advance filters in the demo and "beyond"

this.applyHasBeenClicked = true;
this.dt._filter();
this.hide();
Expand Down
Loading