Bug 10995 - Accessibility: Allow manual entry of renewal date and specific due date in circ
Summary: Accessibility: Allow manual entry of renewal date and specific due date in circ
Status: RESOLVED DUPLICATE of bug 14060
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-04 09:02 UTC by Katrin Fischer
Modified: 2015-04-24 14:05 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10995 - Accessibility: Allow manual entry of renewal date and specific due date in circ (2.10 KB, patch)
2013-12-19 17:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10995 - Accessibility: Allow manual entry of renewal date and specific due date in circ (2.16 KB, patch)
2013-12-30 23:12 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2013-10-04 09:02:52 UTC
In 3.12 it's no longer possible to enter dates manually as specific due date and renewal date in the patron account in circulation. This is a big accessibility problem as not everyone can use the datepickers for filling those fields. We have a library where this is a blocker.
Comment 1 Mathieu Saby 2013-10-06 21:37:45 UTC
+1

M. Saby
Comment 2 Kyle M Hall 2013-12-19 17:48:57 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2013-12-22 21:13:23 UTC
(In reply to Kyle M Hall from comment #2)
> Created attachment 23699 [details] [review] [review]
> Bug 10995 - Accessibility: Allow manual entry of renewal date and specific
> due date in circ
> 
> From Katrin Fischer:
> In 3.12 it's no longer possible to enter dates manually as specific due
> date and renewal date in the patron account in circulation. This is a
> big accessibility problem as not everyone can use the datepickers for
> filling those fields. We have a library where this is a blocker.
> 
> Test Plan:
> 1) Apply this patch
> 2) Start checking out a patron
> 3) Note when you click the due date field, the calendar pops up,
>    but you can now edit the field by hand as well

Did you try the calendar popup in a screen reader? Does it work? If not, we kinda haven't solved the accessibility issue, I can't easily test this, can someone?
Comment 4 Chris Cormack 2013-12-22 21:14:45 UTC
(In reply to Chris Cormack from comment #3)
> (In reply to Kyle M Hall from comment #2)
> > Created attachment 23699 [details] [review] [review] [review]
> > Bug 10995 - Accessibility: Allow manual entry of renewal date and specific
> > due date in circ
> > 
> > From Katrin Fischer:
> > In 3.12 it's no longer possible to enter dates manually as specific due
> > date and renewal date in the patron account in circulation. This is a
> > big accessibility problem as not everyone can use the datepickers for
> > filling those fields. We have a library where this is a blocker.
> > 
> > Test Plan:
> > 1) Apply this patch
> > 2) Start checking out a patron
> > 3) Note when you click the due date field, the calendar pops up,
> >    but you can now edit the field by hand as well
> 
> Did you try the calendar popup in a screen reader? Does it work? If not, we
> kinda haven't solved the accessibility issue, I can't easily test this, can
> someone?

Ahh I see whats happening, you can ignore the pop up, hmm so that works, the question is if people will know that, maybe we could add a tooltip for the screen reader to read?
Comment 5 Katrin Fischer 2013-12-23 12:52:06 UTC
I can only imagine that getting the calendar widget read out to you is quite painful and takes too long in circ to be any fun. The library is currently using 3.6 where you can enter the date directly. I think if we could get that back, it would probably work best for them.

What about making the calendar only pop up when you click on the calendar icon? I think that is how it works in some other places.
Comment 6 Christopher Brannon 2013-12-27 16:30:50 UTC
Forgive me, I am not sure what the protocol is here.  Not sure if I should change the status to discussion.

I've tested the patch, and it does what is described, however, this condition does exist in other places throughout Koha.

request.pl
opac-reserve.pl
opac-memberentry.pl

And I am sure there are other places I didn't think of.  It seems to me we should comb through Koha and address all the date fields, and not just one.  Am I incorrect on this matter?

Christopher
Comment 7 Katrin Fischer 2013-12-30 08:31:40 UTC
Hi Christopher,

yes, I think we should fix all of those to be more accessible, especially in the OPAC. I have started pointing out the problem when testing new patches and I filed some bugs (you shoudl be able to find most of them with 'accessibility'). Until the library we support pointed it out, I was not aware of the problem, but now that we know it is a problem, it would be nice to find a more accessible best practice for the date fields.

But I think it shouldn't stop this patch from going in, because if it works well it will already be a small improvement and help a library.
Comment 8 Christopher Brannon 2013-12-30 15:53:21 UTC
(In reply to Katrin Fischer from comment #7)
> Hi Christopher,
> 
> yes, I think we should fix all of those to be more accessible, especially in
> the OPAC. I have started pointing out the problem when testing new patches
> and I filed some bugs (you shoudl be able to find most of them with
> 'accessibility'). Until the library we support pointed it out, I was not
> aware of the problem, but now that we know it is a problem, it would be nice
> to find a more accessible best practice for the date fields.
> 
> But I think it shouldn't stop this patch from going in, because if it works
> well it will already be a small improvement and help a library.

If that is the case, I will go ahead and sign off on this.  As I stated earlier, this fixes the issue as described.

Christopher
Comment 9 Chris Cormack 2013-12-30 23:04:51 UTC
> 
> If that is the case, I will go ahead and sign off on this.  As I stated
> earlier, this fixes the issue as described.
> 
> Christopher

Hi Christopher (from another Christopher)

Do you want to do the sign off on the patch? Do you know how to do that, would you like me to add your sign off for you? Or if you plan to test more (please say yes) 
This is probably the easiest way
http://wiki.koha-community.org/wiki/Git_bz_configuration

Alternatively, you can just
git commit --amend -s

To add your signoff
git format-patch origin/master
To create the signed off patch, and then attach it in bugzilla, obsoleting the old patch.
Comment 10 Christopher Brannon 2013-12-30 23:09:55 UTC
Chris,
No worries.  I've got it.  I didn't realize there was more to signing off than changing the status.  I'll just do it from the sandbox.  Thanks!

Christopher
Comment 11 Biblibre Sandboxes 2013-12-30 23:12:17 UTC
Patch tested with a sandbox, by Christopher Brannon <cbrannon@cdalibrary.org>
Comment 12 Biblibre Sandboxes 2013-12-30 23:12:47 UTC
Created attachment 23875 [details] [review]
Bug 10995 - Accessibility: Allow manual entry of renewal date and specific due date in circ

From Katrin Fischer:
In 3.12 it's no longer possible to enter dates manually as specific due
date and renewal date in the patron account in circulation. This is a
big accessibility problem as not everyone can use the datepickers for
filling those fields. We have a library where this is a blocker.

Test Plan:
1) Apply this patch
2) Start checking out a patron
3) Note when you click the due date field, the calendar pops up,
   but you can now edit the field by hand as well

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 13 Christopher Brannon 2013-12-30 23:15:40 UTC
(In reply to Chris Cormack from comment #9)
> Hi Christopher (from another Christopher)
> 
> Do you want to do the sign off on the patch? Do you know how to do that,
> would you like me to add your sign off for you? Or if you plan to test more
> (please say yes) 

Chris,
I just realized you said "please say yes" to testing more.  Did I miss something?  Does something else need tested?  I did find a bunch of other pages with the same issue, but for the sake of this bug, the patch works as expected.  Should I have done more?

Christopher
Comment 14 Chris Cormack 2013-12-30 23:18:17 UTC
(In reply to Christopher Brannon from comment #13)
> (In reply to Chris Cormack from comment #9)
> > Hi Christopher (from another Christopher)
> > 
> > Do you want to do the sign off on the patch? Do you know how to do that,
> > would you like me to add your sign off for you? Or if you plan to test more
> > (please say yes) 
> 
> Chris,
> I just realized you said "please say yes" to testing more.  Did I miss
> something?  Does something else need tested?  I did find a bunch of other
> pages with the same issue, but for the sake of this bug, the patch works as
> expected.  Should I have done more?
> 
> Christopher

Not in relation to this bug, but we always need more people signing off/testing. It's even more important than writing patches. Without sign offs, patches can go no where.

If you ever have free time, check out http://dashboard.koha-community.org you should be able to find a ton of patches that need testing :)
Comment 15 Christopher Brannon 2013-12-30 23:21:35 UTC
(In reply to Chris Cormack from comment #14)

> Not in relation to this bug, but we always need more people signing
> off/testing. It's even more important than writing patches. Without sign
> offs, patches can go no where.
> 
> If you ever have free time, check out http://dashboard.koha-community.org
> you should be able to find a ton of patches that need testing :)

Actually, I have been checking this as I can and chipping away at the items I can test in the sandbox.  Always glad to do what I can.

Christopher
Comment 16 Katrin Fischer 2014-01-05 22:35:14 UTC
I'd like to ask the library who reported this problem to take a look as I think that would be the best way to find out if the proposed solution works.

I noticed while testing that an invalid date will somehow automatically be 'corrected'. I am not sure if this will cause problems.
Comment 17 Jonathan Druart 2014-01-10 12:59:57 UTC
QA comment:

There are some concerns here, I think:
There is another readonly field in the circulation.tt file. Trying to test it, I entered an invalid date (14/14/2014), and I got a crashing error (The 'month' parameter ("14") to DateTime::new did not pass the 'an integer between 1 and 12' callback). In fact the INVALID_DATE error code is set only if the date does not match the syspref format.
If we allow the user to enter what he wants, we have to catch correctly any error.

Marked as Failed QA.
Comment 18 Katrin Fischer 2014-01-10 13:04:21 UTC
We have asked the library who reported this to take a look, but catching errors is indeed the biggest problem I see. I think it's not so bad that this only fixes the due date, as we are still trying to figure out the best way of doing it. 

Javascript alerts seemed to work ok for this library in the past, so I think we might be able to do that, although server side would probably be the most accessible.
Comment 19 Katrin Fischer 2014-01-16 15:00:06 UTC
The library we asked to test reported that it's ok now and they can enter dates. I think we still need some error handling, but in general this seems to work.
Comment 20 Jonathan Druart 2015-04-24 14:05:25 UTC
See bug 14060 for a global solution of this problem.

*** This bug has been marked as a duplicate of bug 14060 ***