Bug 28376

I really like it.

The code is really simple, and handling dateformat seems sleek as well.

The only thing I could mention is the modal feels too big.

Yes, go for it!

Can we have a working version of the patch?

I would like, as a first step, a POC witch takes care of two occurrences, a simple one and a complex one.

We need to think about the edge cases:
* time format (now it's 12h when the syspref is 24h)
* manual editing
* format validation
* range validation (from/to)
* "from today only" (no date from the paste)

(In reply to Jonathan Druart from comment #3)
> * time format (now it's 12h when the syspref is 24h)

Well, maybe not, but it's confusing!
https://snipboard.io/IzOBMF.jpg

There is also bootstrap-datepicker - https://github.com/uxsolutions/bootstrap-datepicker

(In reply to Jonathan Druart from comment #5)
> There is also bootstrap-datepicker -
> https://github.com/uxsolutions/bootstrap-datepicker

I like the bootstrap integration. FTR: I chose Flatpickr because it looked like a more active project (maybe I'm biased and it is just that is buggier :-P) but also because it had time handling.

The only thing I've found so far that Flatpickr lacks compared to jQueryUI is an option to show a "Today" button in the calendar widget. There is an addon which can provide this (https://github.com/jcsmorais/shortcut-buttons-flatpickr) but I'm not sure it's worth adding another dependency.

FWIW you can type the word "today" in a Flatpickr date field and it will correctly interpret it.

Created attachment 123352 [details] [review]
Bug 28376: Replace jQueryUI date/timepicker with Flatpickr

This patch is a proof of concept demonstrating how jQueryUI date & time
pickers could be replaced using the Flatpickr library
(https://flatpickr.js.org/).

NEW: I've modified the default configuration of Flatpickr instances so
that a "Clear date" link is automatically appended. This eliminates the
need to add a button to the markup and event handling for each case.

NEW: Date/time formatting should be corrected in this revised patch.

The patch modifies three pages as test cases:
 - Circulation -> Renew (with SpecifyDueDates enabled), to demonstrate
   date and time selection.
   - NEW: You can also test the datepicker shown when you renew an
     on-hold item. This demonstrates a configuration which requires that
     the selection be after today.
 - Administration -> Patron categories -> New category, to demonstrate a
   calendar-only date picker enforcing a date after today.
 - NEW: Reports -> Patrons. The "Date of birth" fields are linked so
   that the second cannot be before the first.

I've made some customizations to the default Flatpickr library's CSS and
incorporated it into staff-global.scss, so you must rebuild the staff
client SCSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

Signed-off-by: David Nind <david@davidnind.com>

Everything seemed to work okay for me. Also like that the calendar icon is now back inside the input box.

Created attachment 123997 [details] [review]
Bug 28376: Replace jQueryUI date/timepicker with Flatpickr

This patch is a proof of concept demonstrating how jQueryUI date & time
pickers could be replaced using the Flatpickr library
(https://flatpickr.js.org/).

NEW: I've modified the default configuration of Flatpickr instances so
that a "Clear date" link is automatically appended. This eliminates the
need to add a button to the markup and event handling for each case.

NEW: Date/time formatting should be corrected in this revised patch.

The patch modifies three pages as test cases:
 - Circulation -> Renew (with SpecifyDueDates enabled), to demonstrate
   date and time selection.
   - NEW: You can also test the datepicker shown when you renew an
     on-hold item. This demonstrates a configuration which requires that
     the selection be after today.
 - Administration -> Patron categories -> New category, to demonstrate a
   calendar-only date picker enforcing a date after today.
 - NEW: Reports -> Patrons. The "Date of birth" fields are linked so
   that the second cannot be before the first.

I've made some customizations to the default Flatpickr library's CSS and
incorporated it into staff-global.scss, so you must rebuild the staff
client SCSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

I like the aesthetics of this library and it works like a charm. It is an easy replacement to the current library.

The use cases work: limiting date ranges, time pick, etc.

The only thing I would change is the font size, as it feels a bit bigger than the rest of the Koha fonts. We might even revisit the font itself.

What's the next step? This should not be part of 21.11 alone, we must replace all other occurrences.

We should be able to set a date without using the widget (ie. keep date validation and remove readonly on the input).

(In reply to Jonathan Druart from comment #15)
> We should be able to set a date without using the widget (ie. keep date
> validation and remove readonly on the input).

From IRC
 oleonard> Joubu: flatpickr's "allowInput" option is true by default.

Tried again and it's working as expected now (??)

Pushed to master for 21.11, thanks to everybody involved!

Created attachment 124368 [details] [review]
Bug 28928: Minor follow-ups to Bug 28376 - Flatpickr introduction

This patch corrects three errors in the original Flatpickr introduction
patch:

- Missing document.ready() in borrowers_stats.tt.
- Redundant calendarFirstDayOfWeek setting in caregories.js
- Missing preventDefault() in calendar.inc

The first two issues don't appear to cause any malfunction but are best
practices. The third issue can cause the page to scroll unexpectedly.

To reproduce this bug, go to (for instance) Administration -> Patron
categories -> New category.

 - If necessary, narrow the height of your browser window so that there
   is a vertical scrollbar.
 - Scroll down the page so that the "Until date" field is at the top.
 - Click the "X" next to the field.
 - The page will scroll to the top.

Apply the patch and test again. The page jump should not occur.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Created attachment 127627 [details] [review]
Bug 28376: (follow-up) Replace jQueryUI date/timepicker with Flatpickr

Remove uneeded include of flatpickr.min.js (we load it from
calendar.inc)

(In reply to Jonathan Druart from comment #19)
> Created attachment 127627 [details] [review] [review]
> Bug 28376: (follow-up) Replace jQueryUI date/timepicker with Flatpickr
> 
> Remove uneeded include of flatpickr.min.js (we load it from
> calendar.inc)

Pushed to master.