Staff client patron interface/Display overdue items in red

[Steven Chan]

For the staff client, Patrons screen, Items Out screen, the idea is to highlight 'overdue items' in the same way that 'items s
till checked out' are highlighted in the Holds screen.

[Pasi Kallinen (paxed)]

I haven't tested the patch yet, but it would be better if you didn't modify the value of overdue_hint label with javascript, and instead used something like this directly:

<label id="overdue_hint" hidden="true" style="background: red; color: white" value="&staff.patron.items.overdue_hint;"/>

[Dan Wells (dbw2)]

I've reviewed this code, and overall, I think it is great. I would like to offer some feedback first, though, before pushing this to master.

1) I completely agree with what Pasi said, we shouldn't be using Javascript to translate the overdue hint, unless there is a reason I am not noticing. Let us know if you need help with the entity-style translation path.

[The following points are basically nits, but here they are anyway. Feel free to pass on them if you disagree.]

2) There is a check_past() function in util.date which is similar in spirit to your isPast() one. I think I like yours better, since it works beyond just the day level (hourly checkouts don't get much love in EG). Ideally we could unify these, but it isn't a dealbreaker, and maybe should simply be a later enhancement.

3) I'm not sold on the indexOf() function you created. First, it makes me nervous to use a function name which matches a built-in name. Maybe it was intentional, but I find it potentially confusing. Second, I am not sure we need the function at all. Might it be better to calculate these indexes once, when the list is created, rather than separately for each row? Not a big deal, but I don't think it would create any code readability issues, so I'll take an easy win.

4) Very minor, but I don't think we need to add the 'overdue' property to the treerow itself.

5) I would prefer the hint be styled more subtly, in a way to match similar 'notes' in the interface. In this case, I would add parens and make it red text on default background, something like (but of course using an entity as noted above):

<label id="overdue_hint" hidden="true" style="color: red" value="(Red items are overdue)"/>

I also dropped the plural from 'overdues'. This point is all my preference, but try it and see what you think. The idea is simply to make the data more noticeable than the interface.

Thanks,
Dan

[Bill Erickson (berick)]

Removing pullrequest since ticket is incomplete. As discussed in IRC, unless someone pops up soon to champion this one, it will be marked "won't fix".

[Andrea Neiman (aneiman)]

Marking won't fix, per Bill's comment #4; and also because it's XUL. Potentially worth a webby-based revisit at some point in the future.