Bug 38391

Summary: DT's add filters called too many times
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Staff interfaceAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: major    
Priority: P5 - low CC: gmcharlt, lucas, martin.renvoize, paul.derscheid
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38130
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38433
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:
Bug Depends on:    
Bug Blocks: 38436    
Attachments: Bug 38391: Redraw the filters row on table redraw
Bug 38391: Redraw the filters row on table redraw
Bug 38391: Redraw the filters row on table redraw
Bug 38391: Redraw the filters row on table redraw
Bug 38391: Do not redraw on adjusting col vis in drawcallback
Bug 38391: Do not redraw on adjusting col vis in drawcallback
Bug 38391: Do not redraw on adjusting col vis in drawcallback
Bug 38391: Do not redraw on adjusting col vis in drawcallback

Description Jonathan Druart 2024-11-07 13:02:33 UTC

    
Comment 1 Jonathan Druart 2024-11-07 13:20:08 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2024-11-07 13:56:38 UTC Comment hidden (obsolete)
Comment 3 Lucas Gass (lukeg) 2024-11-07 15:43:29 UTC Comment hidden (obsolete)
Comment 4 Martin Renvoize (ashimema) 2024-11-08 10:30:35 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2024-11-08 15:10:09 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 6 Jonathan Druart 2024-11-13 15:10:26 UTC
This is not working correctly. The filters header is not adjusted correctly.
Comment 7 Jonathan Druart 2024-11-13 15:22:39 UTC
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.
Comment 8 Jonathan Druart 2024-11-13 15:26:23 UTC
Moved to bug 38433 (specific to the items table).
Comment 9 Katrin Fischer 2024-11-13 15:36:27 UTC
Reverted from main by Joubu's request.
Comment 10 Jonathan Druart 2024-11-13 15:59:12 UTC
Created attachment 174476 [details] [review]
Bug 38391: Do not redraw on adjusting col vis in drawcallback
Comment 11 Jonathan Druart 2024-11-13 15:59:45 UTC
(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.
Comment 12 Katrin Fischer 2024-11-14 15:11:39 UTC
Removing keyword as this is currently not pushed.
Comment 13 Jonathan Druart 2024-11-15 15:20:18 UTC
Created attachment 174596 [details] [review]
Bug 38391: Do not redraw on adjusting col vis in drawcallback
Comment 14 Jonathan Druart 2024-11-15 15:21:36 UTC
"""
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()
Comment 15 Owen Leonard 2024-11-18 12:50:42 UTC
Created attachment 174683 [details] [review]
Bug 38391: Do not redraw on adjusting col vis in drawcallback

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 16 Paul Derscheid 2024-11-18 15:08:55 UTC
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>
Comment 17 Paul Derscheid 2024-11-18 15:09:42 UTC
This seems to work as intended.
Comment 18 Katrin Fischer 2024-11-18 16:08:44 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 19 Lucas Gass (lukeg) 2024-12-05 22:43:17 UTC
Missing 24.05.x deps, no backport