The table row showing the label "Today's checkouts" shows the id of "group-id-issues-table_-strong-today-s-checkouts-strong-" The id is inaccurate and is taking on values from a child cell.
Created attachment 60997 [details] [review] Bug 18211: Corrected intended id's in jquery. To test: 1) Checkout items - if you can, have items checked out from previous day too, but not necessary. 2) Review source code and see that the ids on the <tr> and <td> for cells containing "Today's checkouts" and "Previous checkouts" labels have some crazy ids with partial html tags (strong) and other unnecessary elements. 3) Apply patch. 4) Examine source code again and see a better and smaller id.
Hi Christopher I am still seeing partial html tags in the ids for todays checkout and previous checkouts with your patch applied, for example: var RENEWALS_REMAINING = _("%s of %s renewals remaining"); var HOLD_IS_SUSPENDED = _("Hold is <strong>suspended</strong>"); var UNTIL = _("until %s"); var NEXT_AVAILABLE_ITYPE = _("Next available %s item"); var ITEM_IS_WAITING = _("Item is <strong>waiting</strong>"); var ITEM_IS_WAITING_HERE = _("Item is <strong>waiting here</strong>"); var AT = _("at %s"); var ITEM_IS_IN_TRANSIT = _("Item is <strong>in transit</strong> from %s since %s"); Can you please clarify your test plan further to clarify if this is intended or not
Created attachment 78989 [details] [review] Bug 18211: Corrected intended id's in jquery. To test: 1) Checkout items - if you can, have items checked out from previous day too, but not necessary. 2) Review source code and see that the ids on the <tr> and <td> for cells containing "Today's checkouts" and "Previous checkouts" labels have some crazy ids with partial html tags (strong) and other unnecessary elements. 3) Apply patch. 4) Examine source code again and see a better and smaller id. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 79001 [details] [review] Bug 18211: Corrected intended id's in jquery. To test: 1) Checkout items - if you can, have items checked out from previous day too, but not necessary. 2) Review source code and see that the ids on the <tr> and <td> for cells containing "Today's checkouts" and "Previous checkouts" labels have some crazy ids with partial html tags (strong) and other unnecessary elements. 3) Apply patch. 4) Examine source code again and see a better and smaller id. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Changes made to jquery.dataTables.rowGrouping.js will be overwritten when we will upgrade to a newer version, we should not modify these jQuery plugins. What is your need exactly?
The need is to correctly apply an id to these rows. They are malformed.
This is not clear to me, switching back to Signed off to get other opinions.
Adding Owen for an additional opinion on how to fix this.
(In reply to Jonathan Druart from comment #5) > ... we should not modify these jQuery plugins. I agree. If this is a dependency, then we shouldn't be tweaking it.
(In reply to M. Tompsett from comment #9) > (In reply to Jonathan Druart from comment #5) > > ... we should not modify these jQuery plugins. > > I agree. If this is a dependency, then we shouldn't be tweaking it. True. If we pass the wrong values, correct them at the source.
(In reply to Jonathan Druart from comment #5) > Changes made to jquery.dataTables.rowGrouping.js will be overwritten when we > will upgrade to a newer version, we should not modify these jQuery plugins. Agreed. While I was focusing on the effect of the patch I wasn't looking at how the changes were made -- my bad.
I'll have to take a look at what I did and see if I can do it the correct way. :/
I think I am going to scrap this bug altogether. Id's seem to be created by jquery.dataTables.rowGrouping.js, which is an outside library that was written in 2013. I am not seeing a newer version of it. There are other scripts written for row grouping. In particular, one that appears to be active is: https://cdn.datatables.net/v/dt/dt-1.10.18/rg-1.0.3/datatables.min.js This is a bit beyond me in tackling a project like this. If someone else wants to see if this particular script would work and improve on the id's, that would be great. For now, I am unable to do so.
Just so we are clear on the issue, it looks like jquery.dataTables.rowGrouping.js is adding an unneeded prefix "group-id-" to id's, and including html tags in the sGroup. My solution was to remove the prefix and point the sGroup to the text only, and add another regex to remove some other random characters. Unless we want to adopt this script into koha's libraries and maintain it ourselves, we should probably consider updating to a newer script at some point, like the one I suggested. I haven't tested it, so I don't know if it is any better or worse that what is currently being used.