Staff client patron interface/Display overdue items in red

Bug #1184009 reported by Steven Chan on 2013-05-24
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Dan Wells

Bug Description

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.

Steven Chan (schan2) wrote :
Ben Shum (bshum) on 2013-05-24
Changed in evergreen:
importance: Undecided → Wishlist
status: New → Triaged
milestone: none → 2.5.0-m1
Pasi Kallinen (paxed) wrote :

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) on 2013-06-13
Changed in evergreen:
milestone: 2.5.0-m1 → 2.5.0-m2
Dan Wells (dbw2) on 2013-07-12
Changed in evergreen:
assignee: nobody → Dan Wells (dbw2)
Dan Wells (dbw2) wrote :

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

Changed in evergreen:
status: Triaged → Incomplete
assignee: Dan Wells (dbw2) → nobody
Dan Wells (dbw2) on 2013-07-15
Changed in evergreen:
milestone: 2.5.0-m2 → 2.5.0-alpha1
Remington Steed (rjs7) on 2013-08-12
Changed in evergreen:
milestone: 2.5.0-alpha1 → 2.5.0-alpha2
Dan Wells (dbw2) on 2013-08-26
Changed in evergreen:
milestone: 2.5.0-alpha2 → 2.5.0-beta1
Dan Wells (dbw2) on 2013-09-29
Changed in evergreen:
milestone: 2.5.0-beta1 → 2.5.0-rc
Dan Wells (dbw2) on 2013-10-07
Changed in evergreen:
milestone: 2.5.0-rc → 2.next
Dan Wells (dbw2) on 2014-02-05
Changed in evergreen:
milestone: 2.6.0-alpha1 → 2.6.0-beta1
assignee: nobody → Dan Wells (dbw2)
Dan Wells (dbw2) on 2014-02-27
Changed in evergreen:
milestone: 2.6.0-beta1 → 2.6.0-rc1
Changed in evergreen:
milestone: 2.6.0-rc1 → 2.next
Bill Erickson (berick) wrote :

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".

tags: removed: pullrequest
Andrea Neiman (aneiman) wrote :

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.

Changed in evergreen:
status: Incomplete → Won't Fix
milestone: 3.next → none
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers