Bug 28937

Summary: Use Flatpickr on circulation and patron pages
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bolshawh, jonathan.druart, kyle, lucas, martin.renvoize, nugged, wainuiwitikapark
Version: MainKeywords: Manual
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on: 28938    
Bug Blocks: 28377, 28942, 28945, 28949, 28958, 28961, 28963, 28982, 28983, 29239, 29278    
Attachments: Bug 28937: Use Flatpickr on circulation and patron pages
Bug 28937: Use Flatpickr on circulation and patron pages
Bug 28937: Use Flatpickr on circulation and patron pages
Bug 28937: (QA follow-up) Accessability improvement
Bug 28937: (QA follow-up) Accessability improvement

Description Owen Leonard 2021-09-01 14:05:07 UTC
Bug 28376 added Flatpickr for date and time-picker widgets, with the goal of replacing all usage of jQueryUI's datepicker widget.

This bug will cover Flatpickr additions to these pages/files:

 includes/borrower_debarments.inc
 includes/calendar.inc
 includes/holds_table.inc
 circ/circulation.tt
 circ/overdue.tt
 circ/pendingreserves.tt
 circ/reserveratios.tt
 members/memberentrygen.tt
 members/moremember.tt
 reserve/request.tt
 prog/js/checkouts.js
 prog/js/holds.js
Comment 1 Owen Leonard 2021-09-02 12:47:52 UTC
Created attachment 124428 [details] [review]
Bug 28937: Use Flatpickr on circulation and patron pages

This patch replaces the use of jQueryUI's datepicker on circulation and
patron-related pages.

The patch modifies Flatpickr's default configuration (in calendar.inc)
so that it has the following features:

 - A Flatpickr input with a "futuredate" class will require that the
   selected date be after today.
 - The Flatpickr input field will be wrapped in a container to
   facilitate better CSS styling.
 - Generic handling of paired date fields is enabled using
   ".flatpickrfrom" and ".flatpickrto" field classes. This mimics the
   same feature we have for jQueryUI datepickers using ".datepickerfrom"
   and ".datepickerto".

This patch also removes an unused function which was repeated in three
templates: validate1.

To test, apply the patch and rebuild the staff interface CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation and check out to a patron.
  - Open the Restrictions tab and click "Add manual restriction."
    - In the "Expiration" field, test that the Flatpickr widget works
      correctly and limits to dates after today.
  - Enable the SpecifyDueDate preference if necessary.
    - Test the behavior of the SpecifyDueDate controls: Setting a date,
      clearing a date, session persistence.
- Enable the SuspendHoldsIntranet system preference if necessary.
  - Check out to a patron with existing holds.
    - Open the Holds tab and click the "Suspend" button for one of the
      holds.
      - In the modal window which appears, check that the Flatpickr
        widget works correctly and limits to dates after today.
      - At the bottom of the table of holds, test that the "Suspend all
        holds" Flatpickr works correctly and limits to dates after
        today.
        - Perform this same test from the patron details page.
- Enable the  BatchCheckouts system preference if necessary.
  - Open a patron record and click "Batch check out" in the left-hand
    sidebar menu.
    - Test that the "Hard due date" Flatpickr works correctly as a date
      and time picker.
- Go to Circulation -> Overdues.
  - Test that the date due filters in the sidebar work correctly and
    are linked, e.g. the "to" field cannot be before the "from" field.
     - Perform the same test here: Circulation -> Holds to pull; and
       here: Circulation -> Hold ratios.
- Enable the  HouseboundModule system preference if necessary.
  - Check out to or view details of a patron.
    - Click "Housebound" in the sidebar menu.
      - Save delivery day and frequency settings for that patron.
      - Click "Add a new delivery."
      - Test that the "Date" Flatpickr widget works correctly.
- Go to Patrons -> A patron record -> Edit.
  - Test that Flatpickr widgets work on the following fields:
    - Date of birth
    - Registration date & Expiration date (linked).
    - Patron restrictions -> Add manual restriction -> Expiration.
- View a bibliographic record and start the process of placing a hold.
  - After selecting a patron, test the "Hold starts on" and "Hold
    expires on" date fields. The fields should be linked and each
    should limit to future dates.
  - Confirm that the dates are saved correctly when you submit the
    hold.
- Locate a bibliographic record with multiple holds and view the holds.
  - In the table of holds, test each date field: Date, expiration, and
    suspend-until.
- Test that Flatpickr's static "formatDate" method is working
  correctly:
  - Locate a bibliographic record's item so that there is text in both
    the "Public note" and "Non-public note" field.
  - Check that item out to a patron.
  - After the page reloads the public and non-public notes should be
    shown under the checkout title highlighted in red.
- Check for references to a "validate1" function. There should be none.
Comment 2 Lucas Gass 2021-09-07 18:12:16 UTC
Created attachment 124630 [details] [review]
Bug 28937: Use Flatpickr on circulation and patron pages

This patch replaces the use of jQueryUI's datepicker on circulation and
patron-related pages.

The patch modifies Flatpickr's default configuration (in calendar.inc)
so that it has the following features:

 - A Flatpickr input with a "futuredate" class will require that the
   selected date be after today.
 - The Flatpickr input field will be wrapped in a container to
   facilitate better CSS styling.
 - Generic handling of paired date fields is enabled using
   ".flatpickrfrom" and ".flatpickrto" field classes. This mimics the
   same feature we have for jQueryUI datepickers using ".datepickerfrom"
   and ".datepickerto".

This patch also removes an unused function which was repeated in three
templates: validate1.

To test, apply the patch and rebuild the staff interface CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation and check out to a patron.
  - Open the Restrictions tab and click "Add manual restriction."
    - In the "Expiration" field, test that the Flatpickr widget works
      correctly and limits to dates after today.
  - Enable the SpecifyDueDate preference if necessary.
    - Test the behavior of the SpecifyDueDate controls: Setting a date,
      clearing a date, session persistence.
- Enable the SuspendHoldsIntranet system preference if necessary.
  - Check out to a patron with existing holds.
    - Open the Holds tab and click the "Suspend" button for one of the
      holds.
      - In the modal window which appears, check that the Flatpickr
        widget works correctly and limits to dates after today.
      - At the bottom of the table of holds, test that the "Suspend all
        holds" Flatpickr works correctly and limits to dates after
        today.
        - Perform this same test from the patron details page.
- Enable the  BatchCheckouts system preference if necessary.
  - Open a patron record and click "Batch check out" in the left-hand
    sidebar menu.
    - Test that the "Hard due date" Flatpickr works correctly as a date
      and time picker.
- Go to Circulation -> Overdues.
  - Test that the date due filters in the sidebar work correctly and
    are linked, e.g. the "to" field cannot be before the "from" field.
     - Perform the same test here: Circulation -> Holds to pull; and
       here: Circulation -> Hold ratios.
- Enable the  HouseboundModule system preference if necessary.
  - Check out to or view details of a patron.
    - Click "Housebound" in the sidebar menu.
      - Save delivery day and frequency settings for that patron.
      - Click "Add a new delivery."
      - Test that the "Date" Flatpickr widget works correctly.
- Go to Patrons -> A patron record -> Edit.
  - Test that Flatpickr widgets work on the following fields:
    - Date of birth
    - Registration date & Expiration date (linked).
    - Patron restrictions -> Add manual restriction -> Expiration.
- View a bibliographic record and start the process of placing a hold.
  - After selecting a patron, test the "Hold starts on" and "Hold
    expires on" date fields. The fields should be linked and each
    should limit to future dates.
  - Confirm that the dates are saved correctly when you submit the
    hold.
- Locate a bibliographic record with multiple holds and view the holds.
  - In the table of holds, test each date field: Date, expiration, and
    suspend-until.
- Test that Flatpickr's static "formatDate" method is working
  correctly:
  - Locate a bibliographic record's item so that there is text in both
    the "Public note" and "Non-public note" field.
  - Check that item out to a patron.
  - After the page reloads the public and non-public notes should be
    shown under the checkout title highlighted in red.
- Check for references to a "validate1" function. There should be none.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 3 Lucas Gass 2021-09-07 18:13:56 UTC
This seems to be working nicely, I tested on all the pages with each of the different 'dateformat' options. 

Love the look of Flatpickr!
Comment 4 Martin Renvoize 2021-09-16 12:43:46 UTC
Love this work.. but..

I'm afraid it fell at the first hurdle for me; accessibility is affected.

I tested the check-in screen; Upon navigating to the due date input the widget pops up as expected, however the focus remains in the input box and the widget cannot be navigated or submitted via keyboard.
Comment 5 Katrin Fischer 2021-09-20 21:21:40 UTC
(In reply to Martin Renvoize from comment #4)
> Love this work.. but..
> 
> I'm afraid it fell at the first hurdle for me; accessibility is affected.
> 
> I tested the check-in screen; Upon navigating to the due date input the
> widget pops up as expected, however the focus remains in the input box and
> the widget cannot be navigated or submitted via keyboard.

But would that not be the intended behavior if we want to allow for direct input of the date in the input field for accessibility?
Comment 6 Katrin Fischer 2021-09-20 21:25:09 UTC
(In reply to Katrin Fischer from comment #5)
> (In reply to Martin Renvoize from comment #4)
> > Love this work.. but..
> > 
> > I'm afraid it fell at the first hurdle for me; accessibility is affected.
> > 
> > I tested the check-in screen; Upon navigating to the due date input the
> > widget pops up as expected, however the focus remains in the input box and
> > the widget cannot be navigated or submitted via keyboard.
> 
> But would that not be the intended behavior if we want to allow for direct
> input of the date in the input field for accessibility?

Also it seems that the behaviour is the same with our current widget:

* new patron
* tab to date of birth input field
* enter date with keyboard - ok
* try to use widget with keyboard keys/arrows - doesn't work

To me that makes sense. People preferring the mouse, will use the mouse to handle the widget, but people preferring the keyboard, will probably want to type the date directly.
Comment 7 Martin Renvoize 2021-09-21 10:48:09 UTC
(In reply to Katrin Fischer from comment #6)
> > But would that not be the intended behavior if we want to allow for direct
> > input of the date in the input field for accessibility?

For full accessibility, you should support both options ;)

> 
> Also it seems that the behaviour is the same with our current widget:
> 
> * new patron
> * tab to date of birth input field
> * enter date with keyboard - ok
> * try to use widget with keyboard keys/arrows - doesn't work

I worked on this recently for an accessibility audit of the OPAC ;).

Try the arrow keys whilst holding ctrl or shift ;). Holding ctrl sets the arrow keys to navigate through days, holding shift allows them to navigate month to month, year to year.  (This also works with the screen reader, reading out the date after a small pause on your selected date.

I think I even submitted a documentation patch for this.. it was a requirement to ensure it worked this way to pass the accessibility audit undertaken by UK Gov.
Comment 8 Jonathan Druart 2021-09-22 08:15:48 UTC
Does it mean we should drop Flatpickr and revert what has been pushed already?

It seems that their accessibility issues don't get much attention: https://github.com/flatpickr/flatpickr/milestone/5

See also https://github.com/flatpickr/flatpickr/issues/1329

Workaround suggested at https://github.com/flatpickr/flatpickr/issues/1329#issuecomment-380805839

(In reply to Martin Renvoize from comment #7)
> (In reply to Katrin Fischer from comment #6)
> > > But would that not be the intended behavior if we want to allow for direct
> > > input of the date in the input field for accessibility?
> 
> For full accessibility, you should support both options ;)
> 
> > 
> > Also it seems that the behaviour is the same with our current widget:
> > 
> > * new patron
> > * tab to date of birth input field
> > * enter date with keyboard - ok
> > * try to use widget with keyboard keys/arrows - doesn't work
> 
> I worked on this recently for an accessibility audit of the OPAC ;).
> 
> Try the arrow keys whilst holding ctrl or shift ;). Holding ctrl sets the
> arrow keys to navigate through days, holding shift allows them to navigate
> month to month, year to year.  (This also works with the screen reader,
> reading out the date after a small pause on your selected date.
> 
> I think I even submitted a documentation patch for this.. it was a
> requirement to ensure it worked this way to pass the accessibility audit
> undertaken by UK Gov.

I do confirm that ctrl allows to navigate through days, however shift does nothing for me. Alt does but conflicts with browser's shortcuts (tab switch).
Comment 9 Martin Renvoize 2021-09-27 11:51:44 UTC
Created attachment 125355 [details] [review]
Bug 28937: Use Flatpickr on circulation and patron pages

This patch replaces the use of jQueryUI's datepicker on circulation and
patron-related pages.

The patch modifies Flatpickr's default configuration (in calendar.inc)
so that it has the following features:

 - A Flatpickr input with a "futuredate" class will require that the
   selected date be after today.
 - The Flatpickr input field will be wrapped in a container to
   facilitate better CSS styling.
 - Generic handling of paired date fields is enabled using
   ".flatpickrfrom" and ".flatpickrto" field classes. This mimics the
   same feature we have for jQueryUI datepickers using ".datepickerfrom"
   and ".datepickerto".

This patch also removes an unused function which was repeated in three
templates: validate1.

To test, apply the patch and rebuild the staff interface CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation and check out to a patron.
  - Open the Restrictions tab and click "Add manual restriction."
    - In the "Expiration" field, test that the Flatpickr widget works
      correctly and limits to dates after today.
  - Enable the SpecifyDueDate preference if necessary.
    - Test the behavior of the SpecifyDueDate controls: Setting a date,
      clearing a date, session persistence.
- Enable the SuspendHoldsIntranet system preference if necessary.
  - Check out to a patron with existing holds.
    - Open the Holds tab and click the "Suspend" button for one of the
      holds.
      - In the modal window which appears, check that the Flatpickr
        widget works correctly and limits to dates after today.
      - At the bottom of the table of holds, test that the "Suspend all
        holds" Flatpickr works correctly and limits to dates after
        today.
        - Perform this same test from the patron details page.
- Enable the  BatchCheckouts system preference if necessary.
  - Open a patron record and click "Batch check out" in the left-hand
    sidebar menu.
    - Test that the "Hard due date" Flatpickr works correctly as a date
      and time picker.
- Go to Circulation -> Overdues.
  - Test that the date due filters in the sidebar work correctly and
    are linked, e.g. the "to" field cannot be before the "from" field.
     - Perform the same test here: Circulation -> Holds to pull; and
       here: Circulation -> Hold ratios.
- Enable the  HouseboundModule system preference if necessary.
  - Check out to or view details of a patron.
    - Click "Housebound" in the sidebar menu.
      - Save delivery day and frequency settings for that patron.
      - Click "Add a new delivery."
      - Test that the "Date" Flatpickr widget works correctly.
- Go to Patrons -> A patron record -> Edit.
  - Test that Flatpickr widgets work on the following fields:
    - Date of birth
    - Registration date & Expiration date (linked).
    - Patron restrictions -> Add manual restriction -> Expiration.
- View a bibliographic record and start the process of placing a hold.
  - After selecting a patron, test the "Hold starts on" and "Hold
    expires on" date fields. The fields should be linked and each
    should limit to future dates.
  - Confirm that the dates are saved correctly when you submit the
    hold.
- Locate a bibliographic record with multiple holds and view the holds.
  - In the table of holds, test each date field: Date, expiration, and
    suspend-until.
- Test that Flatpickr's static "formatDate" method is working
  correctly:
  - Locate a bibliographic record's item so that there is text in both
    the "Public note" and "Non-public note" field.
  - Check that item out to a patron.
  - After the page reloads the public and non-public notes should be
    shown under the checkout title highlighted in red.
- Check for references to a "validate1" function. There should be none.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2021-09-27 11:51:48 UTC
Created attachment 125356 [details] [review]
Bug 28937: (QA follow-up) Accessability improvement

This patch adds a keydown event listener onto the flatpickr instance and
looks for the down arrow key. When such an event is detected we take
that to mean the user wants to keyboard navigate the calendar widget and
so disable the buggy allowInput option allowing keyboard navigation to
work as expected.
Comment 11 Martin Renvoize 2021-09-27 11:57:15 UTC
OK, I think my follow-up should resolve the accessibility concerns I had.. Please test it.

With that in mind, I'm happy to PQA on the original patch, but would really like someone to SO my follow-up to check for anything I've missed.

In short, I add a listener onto the flatpickr instance at it's creation time and watch for the 'Down arrow' keydown even.. upon detection of that event we temporarily disable the buggy allowInput configuration option of the flatpickr instant and restore the keyboard navigation of the picker widget in doing so.  The only caveat I can see with this is that there's not a clear keyboard navigable escape back to the input to get manual input back once you've enabled down arrowed into the widget... hitting escape does close the widget, but it drops you back at totally different focus location for some reason.
Comment 12 Martin Renvoize 2021-09-27 12:00:38 UTC
We should document this feature too..

"Down arrow" when in a flatpickr input field will focus the flatpickr widget for keyboard navigation. Arrow keys can then be used to navigate days, holding ctrl whilst using the arrow keys switched navigation to month and year fields. Hitting return will submit the selected value, or if this is a datetime picker it will submit the date part and move the focus to the timepicker for the next selection step.
Comment 13 Owen Leonard 2021-09-27 14:27:23 UTC
Created attachment 125363 [details] [review]
Bug 28937: (QA follow-up) Accessability improvement

This patch adds a keydown event listener onto the flatpickr instance and
looks for the down arrow key. When such an event is detected we take
that to mean the user wants to keyboard navigate the calendar widget and
so disable the buggy allowInput option allowing keyboard navigation to
work as expected.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 14 Jonathan Druart 2021-09-29 10:44:32 UTC
Pushed to master for 21.11, thanks to everybody involved!