Code in prog/js/datatables.js and columns_settings.inc have both the (almost) same code for buttons. Can we avoid that?
I just bumped into this myself recently and it drove me a little crazy...
Rescoping this bug. I will make kohaTable works in the different cases, so we will have only one way to instanciate DataTable.
*** Bug 38114 has been marked as a duplicate of this bug. ***
Created attachment 175186 [details] [review] Bug 26553: Replace KohaTable with kohaTable
Created attachment 175187 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - Staff
Created attachment 175188 [details] [review] Bug 26553: Remove columns_settings.inc
Created attachment 175189 [details] [review] Bug 26553: Replace KohaTable with kohaTable - OPAC
Created attachment 175190 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - OPAC
Created attachment 175191 [details] [review] Bug 26553: Remove columns_settings.inc - OPAC
Same as bug 38255, we remove KohaTable (columns_settings.inc) in favor of kohaTable. At this point we have kohaTable available for both interfaces, for the different use cases (svc, REST API, no server-side processing). We remove columns_settings.inc. Test plan: Test some table modified by this patchset.
QA: `git grep KohaTable ':(exclude)*.vue' ':(exclude)misc/release_notes/' | wc -l` must return 0
I noticed two small problems when testing, both JS errors: acqui-home.tt: ReferenceError: oTable is not defined itemsearch.tt: TypeError: order_by is undefined
Created attachment 175874 [details] [review] Bug 26553: Replace KohaTable with kohaTable
Created attachment 175875 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - Staff
Created attachment 175876 [details] [review] Bug 26553: Remove columns_settings.inc
Created attachment 175877 [details] [review] Bug 26553: Replace KohaTable with kohaTable - OPAC
Created attachment 175878 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - OPAC
Created attachment 175879 [details] [review] Bug 26553: Remove columns_settings.inc - OPAC
(In reply to Lucas Gass (lukeg) from comment #12) Thanks a lot! > I noticed two small problems when testing, both JS errors: > > acqui-home.tt: ReferenceError: oTable is not defined Fixed inline in the first patch. - $("#accounts").kohaTable({ + var oTable $("#accounts").kohaTable({ > itemsearch.tt: TypeError: order_by is undefined Fixed on "Bug 38255: Exception - Item search"
Created attachment 176359 [details] [review] Bug 26553: Replace KohaTable with kohaTable Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176360 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - Staff Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176361 [details] [review] Bug 26553: Remove columns_settings.inc Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176362 [details] [review] Bug 26553: Replace KohaTable with kohaTable - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176363 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176364 [details] [review] Bug 26553: Remove columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Awesome, thanks Lucas!
*** Bug 28058 has been marked as a duplicate of this bug. ***
Created attachment 176588 [details] [review] Bug 26553: Remove columns_settings.inc Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176589 [details] [review] Bug 26553: Replace KohaTable with kohaTable - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176590 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176591 [details] [review] Bug 26553: Remove columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This doesn't apply for me (on top of Bug 38255): Applying: Bug 26553: Replace KohaTable with kohaTable error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt). error: could not build fake ancestor Patch failed at 0001 Bug 26553: Replace KohaTable with kohaTable
Created attachment 177087 [details] [review] Bug 26553: Replace KohaTable with kohaTable Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177088 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - Staff Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177089 [details] [review] Bug 26553: Remove columns_settings.inc Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177090 [details] [review] Bug 26553: Replace KohaTable with kohaTable - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177091 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177092 [details] [review] Bug 26553: Remove columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(In reply to Owen Leonard from comment #32) > This doesn't apply for me (on top of Bug 38255): > > Applying: Bug 26553: Replace KohaTable with kohaTable > error: sha1 information is lacking or useless > (koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt). > error: could not build fake ancestor > Patch failed at 0001 Bug 26553: Replace KohaTable with kohaTable Everything good for me here but I had a tiny conflict on bug 38255. I pushed a branch to my gitlab repo in case it does not apply again, see bug_26553.
Created attachment 177152 [details] [review] Bug 26553: (follow-up) Replace KohaTable with kohaTable - OPAC
Created attachment 177153 [details] [review] Bug 26553: Replace KohaTable with kohaTable Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> JD Amended patch: replace wrong occurrences of "kohatable" with "kohaTable"
Created attachment 177154 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - Staff Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177155 [details] [review] Bug 26553: Remove columns_settings.inc Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177156 [details] [review] Bug 26553: Replace KohaTable with kohaTable - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> JD Amended patch: replace wrong occurrences of "kohatable" with "kohaTable"
Created attachment 177157 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177158 [details] [review] Bug 26553: Remove columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Looks like something got lost in the transition... There should be some defaults being set, e.g. var default_column_defs = [ { "aTargets": ["string-sort"], "sType": "string" }, { "aTargets": ["anti-the"], "sType": "anti-the" }, { "aTargets": ["NoSort"], "bSortable": false, "bSearchable": false } ]; I don't think this is working in the OPAC or staff interface. I should be able to remove the table options in a template (e.g. opac-user.tt) and still have the options work. I don't find this to be the case.
It also looks like some defaults may be different? On opac-user.pl I notice that the holdings and holds tables don't show all their contents. It needs to be updated to add "paging: false" to show all results.
Created attachment 177234 [details] [review] Bug 26553: Replace KohaTable with kohaTable Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> JD Amended patch: replace wrong occurrences of "kohatable" with "kohaTable"
Created attachment 177235 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - Staff Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177236 [details] [review] Bug 26553: Remove columns_settings.inc Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177237 [details] [review] Bug 26553: Replace KohaTable with kohaTable - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> JD Amended patch: replace wrong occurrences of "kohatable" with "kohaTable"
Created attachment 177238 [details] [review] Bug 26553: Remove useless INCLUDE of columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177239 [details] [review] Bug 26553: Remove columns_settings.inc - OPAC Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 177240 [details] [review] Bug 26553: Replace KohaTable with kohaTable - curbside pickups Adjust this new occurrence
Created attachment 177241 [details] [review] Bug 26553: Bring default columnDefs back
Created attachment 177242 [details] [review] Bug 26553: Fix incorrect occurrences of 'sortable' => 'orderable'
Created attachment 177243 [details] [review] Bug 26553: Restore default 'paging: false' - OPAC
(In reply to Owen Leonard from comment #47) > Looks like something got lost in the transition... There should be some > defaults being set, e.g. > > var default_column_defs = [ > { "aTargets": ["string-sort"], "sType": "string" }, > { "aTargets": ["anti-the"], "sType": "anti-the" }, > { "aTargets": ["NoSort"], "bSortable": false, "bSearchable": > false } > ]; > > I don't think this is working in the OPAC or staff interface. I should be > able to remove the table options in a template (e.g. opac-user.tt) and still > have the options work. I don't find this to be the case. Good catch, should be fixed now (see "Bring default columnDefs back") (In reply to Owen Leonard from comment #48) > It also looks like some defaults may be different? On opac-user.pl I notice > that the holdings and holds tables don't show all their contents. It needs > to be updated to add "paging: false" to show all results. See "Restore default 'paging: false' - OPAC". Did you notice another default that is wrong?
Created attachment 177246 [details] [review] Bug 26553: Fix incorrect occurrences of 'sortable' => 'orderable'
Created attachment 177247 [details] [review] Bug 26553: Restore default 'paging: false' - OPAC
Created attachment 177264 [details] [review] Bug 26553: (follow-up) Fix patron attribute types page
Created attachment 177265 [details] [review] Bug 26553: (follow-up) Fix pending patron discharges
> See "Restore default 'paging: false' - OPAC". Did you notice another default > that is wrong? I notice that now the "Export" button is always shown, whereas previously it would only be shown if there were also column configuration buttons. I guess this makes sense because exporting is relevant whether or not the table can be configured. Looking at the other buttons in that set, I wonder if the "Copy shareable link" should also be globally enabled?
Created attachment 177271 [details] [review] Bug 26553: Split Export and restore "clear filters" - OPAC "Clear filters" is not in main but should be (I don't really understand why it is not displayed). The Export options were not grouped within an "Export" button in main, this patch restores that.
(In reply to Owen Leonard from comment #64) > > See "Restore default 'paging: false' - OPAC". Did you notice another default > > that is wrong? > > I notice that now the "Export" button is always shown, whereas previously it > would only be shown if there were also column configuration buttons. I guess > this makes sense because exporting is relevant whether or not the table can > be configured. Looking at the other buttons in that set, I wonder if the > "Copy shareable link" should also be globally enabled? The last patch ungroup the export options and restore the "clear filters" button. However I don't understand the "configuration" part, this is only for the staff interface. Also the "copy shareable link" is not available for at the OPAC.