Bug 13315 - Add feedback for last item checked out to circulation.pl
Summary: Add feedback for last item checked out to circulation.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 14465
  Show dependency treegraph
 
Reported: 2014-11-20 18:38 UTC by Kyle M Hall
Modified: 2016-06-21 21:38 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13315 - Add feedback for last item checked out to circulation.pl (9.83 KB, patch)
2014-11-20 18:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13315 - Add feedback for last item checked out to circulation.pl (9.83 KB, patch)
2014-11-20 18:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13315 - Add feedback for last item checked out to circulation.pl (10.09 KB, patch)
2014-12-09 18:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 13315 [Follow-up] Add feedback for last item checked out to circulation.pl (3.62 KB, patch)
2014-12-09 18:34 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13315 - Add feedback for last item checked out to circulation.pl (10.10 KB, patch)
2014-12-09 19:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13315 [Follow-up] Add feedback for last item checked out to circulation.pl (3.70 KB, patch)
2014-12-09 20:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 13315 - Add feedback for last item checked out to circulation.pl (10.15 KB, patch)
2014-12-14 21:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 13315 [Follow-up] Add feedback for last item checked out to circulation.pl (3.75 KB, patch)
2014-12-14 21:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13315 [Follow-up] Add feedback for last item checked out to circulation.pl (3.75 KB, patch)
2015-03-02 13:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13315 - Add feedback for last item checked out to circulation.pl (10.15 KB, patch)
2015-03-02 13:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13315 [Follow-up] Add feedback for last item checked out to circulation.pl (3.75 KB, patch)
2015-03-02 13:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13315: (QA followup) fix t/db_dependent/Circulation_issue.t (3.16 KB, patch)
2015-04-13 18:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2014-11-20 18:38:38 UTC
It seems that many librarians find it disconcerting to have no feedback with the new checkouts table. It seems that many of them wait for it to fully load, check to verify the item was checked out, and only then check out the next item.

To help alleviate this issue, we can have the checkouts page give feedback about the item that was just checked out.
Comment 1 Kyle M Hall 2014-11-20 18:40:00 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2014-11-20 18:43:40 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2014-11-20 19:35:39 UTC
I think this is an important step towards making the AJAX circ feature more usable, but I think it would be good to get some opinions from librarians about this implementation.
Comment 4 Owen Leonard 2014-12-09 15:18:21 UTC
Here's the way it works with Kyle's patch:

http://www.screencast.com/t/qNw2U3hwz

I'm inclined to do something less obtrusive:

http://www.screencast.com/t/1gsa2prF

...on the assumption that the visual aspect of checking out was working fine before AJAX circ, and fixes to AJAX circ should look to returning things to a proven-functional state.

However, always assuming that it was better before would prevent us from making improvements, so I want hear opinions.
Comment 5 Christopher Brannon 2014-12-09 15:44:54 UTC
(In reply to Owen Leonard from comment #4)
> Here's the way it works with Kyle's patch:
> 
> http://www.screencast.com/t/qNw2U3hwz
> 
> I'm inclined to do something less obtrusive:
> 
> http://www.screencast.com/t/1gsa2prF
> 
> ...on the assumption that the visual aspect of checking out was working fine
> before AJAX circ, and fixes to AJAX circ should look to returning things to
> a proven-functional state.
> 
> However, always assuming that it was better before would prevent us from
> making improvements, so I want hear opinions.

Owen,
I would be more inclined to see the feedback in the same area that I'm scanning in (in the same box).  Maybe at the bottom?  It is less scanning around the screen.  You are already looking there when scanning, why not have it in the same area?

The concept is similar to switching between the keyboard and mouse.  If I am given the option, I will choose a technique that requires the least amount of reorientation.

Just my two cents.

Christopher
Comment 6 Owen Leonard 2014-12-09 17:01:03 UTC
(In reply to Christopher Brannon from comment #5)
> I would be more inclined to see the feedback in the same area that I'm
> scanning in (in the same box).  Maybe at the bottom?

I like this idea too! Here's my take:

http://www.screencast.com/t/ZcsPfJabP

The advantage to this method is that the feedback is visible even for patrons with lost of information in the messages column:

http://www.screencast.com/t/3uEHAaWnUyc
Comment 7 Jason Burds 2014-12-09 17:17:35 UTC
If I have a vote it's for this one.

http://www.screencast.com/t/ZcsPfJabP
Comment 8 Christopher Brannon 2014-12-09 17:25:58 UTC
(In reply to Owen Leonard from comment #6)
> (In reply to Christopher Brannon from comment #5)
> > I would be more inclined to see the feedback in the same area that I'm
> > scanning in (in the same box).  Maybe at the bottom?
> 
> I like this idea too! Here's my take:
> 
> http://www.screencast.com/t/ZcsPfJabP
> 
> The advantage to this method is that the feedback is visible even for
> patrons with lost of information in the messages column:
> 
> http://www.screencast.com/t/3uEHAaWnUyc

Excellent.  My OCD side of me says indent everything past "Checked out:" so it is more visually attractive.  Something like this:

http://screencast.com/t/EC4qcKNixO

But this is just a simulated example.  Don't know how the formatting would actually look or work.

I'd also suggest a contrasting background color to help bring attention to it.  Unlike some of the warning messages that are yellow, I'd maybe use a faint green background?

Here's another example, with the header above and slightly larger, with a colored background:

http://screencast.com/t/hDNBknJ8e

Anyway, I think placement is excellent.

Christopher
Comment 9 Owen Leonard 2014-12-09 18:34:26 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2014-12-09 18:34:55 UTC Comment hidden (obsolete)
Comment 11 Jason Burds 2014-12-09 19:31:19 UTC
Looks good.
Comment 12 Kyle M Hall 2014-12-09 19:57:09 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2014-12-09 20:02:14 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2014-12-09 20:10:24 UTC
I just wanted to add that Owen's followup looks fantastic! Great work!
Comment 15 Katrin Fischer 2014-12-14 21:35:41 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2014-12-14 21:35:58 UTC Comment hidden (obsolete)
Comment 17 Tomás Cohen Arazi 2015-02-12 18:37:17 UTC
Everything looks good, but it doesn't apply anymore, please rebase!
Comment 18 Kyle M Hall 2015-03-02 13:26:45 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2015-03-02 13:27:34 UTC
Created attachment 36291 [details] [review]
Bug 13315 - Add feedback for last item checked out to circulation.pl

It seems that many librarians find it disconcerting to have no feedback
with the new checkouts table. It seems that many of them wait for it to
fully load, check to verify the item was checked out, and only then
check out the next item.

To help alleviate this issue, we can have the checkouts page give
feedback about the item that was just checked out.

Test Plan:
1) Apply this patch
2) Check an item out
3) Note the message "$title ($barcode) due on $date_due"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

This works well and fixes a very problematic issue with the new AJAX
circ. I will be submitting a follow-up which I think is an improvement
to the display.

Signed-off-by: Jason Burds  <jburds@dubuque.lib.ia.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Kyle M Hall 2015-03-02 13:27:55 UTC
Created attachment 36292 [details] [review]
Bug 13315 [Follow-up] Add feedback for last item checked out to circulation.pl

This patch changes the position and styling of the information about the
last checkout so that it is grouped with the checkout form itself,
placing the message in a more visible position, especially for patrons
with a lot of information in the right-hand messages column which might
push the checkouts table down offscreen.

To test, apply the patch and clear your browser cache in order to
refresh the main CSS file.

Check out an item and confirm that a message is prominently displayed.

Signed-off-by: Jason Burds  <jburds@dubuque.lib.ia.us>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Tomás Cohen Arazi 2015-04-13 18:18:08 UTC
Created attachment 37776 [details] [review]
Bug 13315: (QA followup) fix t/db_dependent/Circulation_issue.t

C4::Circulation::AddIssue now returns a Koha::Schema::Result::Issue object
so tests need to be fixed.

The old behaviour was to always return the due date. It now returns
undef if no issue performed. So one of the tests become irrelevant.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 22 Tomás Cohen Arazi 2015-04-13 18:18:49 UTC
Patches pushed to master.

Thanks Kyle and Owen, nice job!
Comment 23 Chris Cormack 2015-04-21 23:03:58 UTC
String changes held for 3.18.7
Comment 24 Chris Cormack 2015-04-29 21:43:37 UTC
Applied on 3.18.x will be in 3.18.7
Comment 25 Colin Campbell 2015-09-03 15:26:37 UTC
Change in return causes sipserver instances to crash on renewals : see bug  14673 for description and workaround
Comment 26 Viktor Sarge 2015-09-15 12:47:07 UTC
I love this patch a lot, but am I the only one who seems to get nothing in the  [% issue.item.biblio.title %]? I only get the barcode and due date (both production server and my dev-install). Since I seem to remember this one working fine I'd just like to hear from others before I do any really extensive testing on my setup.
Comment 27 Katrin Fischer 2015-11-22 21:51:21 UTC
(In reply to Viktor Sarge from comment #26)
> I love this patch a lot, but am I the only one who seems to get nothing in
> the  [% issue.item.biblio.title %]? I only get the barcode and due date
> (both production server and my dev-install). Since I seem to remember this
> one working fine I'd just like to hear from others before I do any really
> extensive testing on my setup.

I Viktor - I just checked, the title, barcode and due date display nicely in my tests.
Comment 28 Viktor Sarge 2015-11-23 12:30:41 UTC
(In reply to Katrin Fischer from comment #27)
> (In reply to Viktor Sarge from comment #26)
> > I love this patch a lot, but am I the only one who seems to get nothing in
> > the  [% issue.item.biblio.title %]? I only get the barcode and due date
> > (both production server and my dev-install). Since I seem to remember this
> > one working fine I'd just like to hear from others before I do any really
> > extensive testing on my setup.
> 
> I Viktor - I just checked, the title, barcode and due date display nicely in
> my tests.

Thanks for the feedback Katrin! It was gone for a while, but it was patched with some later release and I had already forgotten about commenting on this thread by then. Sorry for the extra work!