Table: Fix Advance filtering table #15811
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
There was a problem hiding this comment.
I believe trying to fix this problem by adding to the changes made in PR #15594 is a mistake. The PR #15594 doesn't take into account that there is more then one way to save/apply a filter. Developers can also programmatically "apply and change" filters. This was never taken into account by PR #15594.
I think the proper approach is to create a method that wraps "this.dt_filter()" call. This is where i belielve that most of the logic should be added.
There was a problem hiding this comment.
I believe trying to fix this problem by adding to the changes made in PR #15594 is a mistake. The PR #15594 doesn't take into account that there is more then one way to save/apply a filter. Developers can also programmatically "apply and change" filters. This was never taken into account by PR #15594.
I think the proper approach is to create a method that wraps "this.dt_filter()" call. This is where i believe that most of the logic should be added.
I personally tried to solve this problem and was unsuccessful since it was so complicated. I believe this problem would have been fixed years ago, but it is really very complicated based on the current design.
| @@ -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])); | |||
There was a problem hiding this comment.
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"
| // 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(); |
There was a problem hiding this comment.
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
|
Over the weekend I have been thinking about the how to fix this problem (issue #15811, issue #15778, issue #15557). I believe that the best/proper solution is when the filter overlay/dialog is hidden (via the hide method or any other means) the overlay/dialog is completely destroyed. When the the filter/dialog is displayed, it is always created and filled with the current filter setting. This is obviously not a trivial solution or this bug would have fixed (or initially implemented) years ago with this approach. |
|
@rosenthalj |
#15804
NB: Waiting for Issue author's response