Description
Jonathan Druart
2024-11-07 13:02:33 UTC
Created attachment 174193 [details] [review] Bug 38391: Redraw the filters row on table redraw We used to adjust the filters row when the column visibility changed. It was working, but at the initialization of the table this event if fired once per columns. On the items table it means 36x. When a filtering/sorting, the draw event is triggered once, but the column-visibility.dt is called 36x Try with and without this patch and compare the delay between the query is received and the page is redrawn. Created attachment 174235 [details] [review] Bug 38391: Redraw the filters row on table redraw We used to adjust the filters row when the column visibility changed. It was working, but at the initialization of the table this event if fired once per columns. On the items table it means 36x. When a filtering/sorting, the draw event is triggered once, but the column-visibility.dt is called 36x Test plan: Try with and without this patch and compare the delay between the query is received and the page is redrawn. You can test with 1000 libraries on the items table. "Show filters" then filters using the global search. On Firefox you will notice that the response is received but the table not redrawn directly. With this patch it should be redrawn pretty fase. Another test would be to add the following 2 lines: + let i = 0; table_dt.on("draw", function(){ + console.log("redraw filters %s".format(i)); if ( add_filters ) { _dt_add_filters(this, table_dt, filters_options); } Open the console and notice that "redraw filters" is logged only once. Created attachment 174253 [details] [review] Bug 38391: Redraw the filters row on table redraw We used to adjust the filters row when the column visibility changed. It was working, but at the initialization of the table this event if fired once per columns. On the items table it means 36x. When a filtering/sorting, the draw event is triggered once, but the column-visibility.dt is called 36x Test plan: Try with and without this patch and compare the delay between the query is received and the page is redrawn. You can test with 1000 libraries on the items table. "Show filters" then filters using the global search. On Firefox you will notice that the response is received but the table not redrawn directly. With this patch it should be redrawn pretty fase. Another test would be to add the following 2 lines: + let i = 0; table_dt.on("draw", function(){ + console.log("redraw filters %s".format(i)); if ( add_filters ) { _dt_add_filters(this, table_dt, filters_options); } Open the console and notice that "redraw filters" is logged only once. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 174268 [details] [review] Bug 38391: Redraw the filters row on table redraw We used to adjust the filters row when the column visibility changed. It was working, but at the initialization of the table this event if fired once per columns. On the items table it means 36x. When a filtering/sorting, the draw event is triggered once, but the column-visibility.dt is called 36x Test plan: Try with and without this patch and compare the delay between the query is received and the page is redrawn. You can test with 1000 libraries on the items table. "Show filters" then filters using the global search. On Firefox you will notice that the response is received but the table not redrawn directly. With this patch it should be redrawn pretty fase. Another test would be to add the following 2 lines: + let i = 0; table_dt.on("draw", function(){ + console.log("redraw filters %s".format(i)); if ( add_filters ) { _dt_add_filters(this, table_dt, filters_options); } Open the console and notice that "redraw filters" is logged only once. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Pushed for 24.11! Well done everyone, thank you! This is not working correctly. The filters header is not adjusted correctly. This patch is wrong, and the assumption in the commit message as well. """ When a filtering/sorting, the draw event is triggered once, but the column-visibility.dt is called 36x """ This is not correct, it's not called at all. It's called several times on the items table (and only on this one!) because of the specific behaviour we have on this table: hide the columns without data. I am requiring a revert from this commit, and I will try to provide something better for the items table. Reverted from main by Joubu's request. Created attachment 174476 [details] [review] Bug 38391: Do not redraw on adjusting col vis in drawcallback (In reply to Jonathan Druart from comment #10) > Created attachment 174476 [details] [review] [review] > Bug 38391: Do not redraw on adjusting col vis in drawcallback Dropping this patch here, but ignore it for now. This might be the correct fix. Removing keyword as this is currently not pushed. Created attachment 174596 [details] [review] Bug 38391: Do not redraw on adjusting col vis in drawcallback """ It was called several times on the items table (and only on this one!) because of the specific behaviour we have on this table: hide the columns without data. """ I managed to fix it correctly I think, we simply needed to pass false to visible so that there is no redraw (redrawCalculations) https://datatables.net/reference/api/columns().visible() Created attachment 174683 [details] [review] Bug 38391: Do not redraw on adjusting col vis in drawcallback Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 174718 [details] [review] Bug 38391: Do not redraw on adjusting col vis in drawcallback Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> This seems to work as intended. Pushed for 24.11! Well done everyone, thank you! Missing 24.05.x deps, no backport |