Bug 29552

Summary: flatpickr quick shortcuts should be 'Disabled' for invalid dates
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: TemplatesAssignee: Jonathan Druart <jonathan.druart+koha>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrewfh, david, jonathan.druart+koha, kyle, martin.renvoize, nugged, testopia
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29735
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.02
Bug Depends on: 29478    
Bug Blocks:    
Attachments: Bug 29552: Flatpickr - Disable shortcut buttons if inactive
Bug 29552: Flatpickr - Disable shortcut buttons if inactive
Bug 29552: (follow-up) Use CSS to set disabled class
Bug 29552: Flatpickr - Disable shortcut buttons if inactive
Bug 29552: (follow-up) Use CSS to set disabled class
Bug 29552: (follow-up) Reverse SCSS declaration order
Bug 29552: Disable today for dates in the future
Bug 29552: Flatpickr - Disable shortcut buttons if inactive
Bug 29552: (follow-up) Use CSS to set disabled class
Bug 29552: Disable today for dates in the future
Bug 29552: Flatpickr - Disable shortcut buttons if inactive
Bug 29552: (follow-up) Use CSS to set disabled class
Bug 29552: Disable today for dates in the future

Description Martin Renvoize 2021-11-22 10:40:07 UTC
From bug 29478 comment 5:
 
> I thought of an issue when testing my follow-up: Should the shortcut controls reflect the date limitations of the flatpickr instance? For instance, if selection is limited to a date before today, should the "today" and "tomorrow" controls be disabled? Currently clicking a shortcut for a date which is disallowed will empty the date field.

I decided we should go with what we have for this coming release, but we should really implement the above asap after release.
Comment 1 Jonathan Druart 2021-11-23 13:18:38 UTC
Created attachment 127954 [details] [review]
Bug 29552: Flatpickr - Disable shortcut buttons if inactive

If one of the buttons is not relevant we should disable it and mark is
as such on the interface.
Comment 2 Jonathan Druart 2021-11-23 13:20:09 UTC
I was curious to know how easy/hard it would be to implement so I gave it a try. It seems quite easy actually, here is my version.
Comment 3 Owen Leonard 2021-12-03 14:41:47 UTC
Created attachment 128221 [details] [review]
Bug 29552: Flatpickr - Disable shortcut buttons if inactive

If one of the buttons is not relevant we should disable it and mark is
as such on the interface.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Owen Leonard 2021-12-03 14:41:50 UTC
Created attachment 128222 [details] [review]
Bug 29552: (follow-up) Use CSS to set disabled class
Comment 5 David Nind 2021-12-03 23:04:59 UTC
I had a go at testing but came across some issues (or I don't understand how to test this...).

Testing notes using koha-testing-docker:

1. Update the CSS (yarn install, yarn build, yarn build --view opac, flush_memcached, restart_all)

2. Examples of where yesterday, today and tomorrow should not be active:
   . Place a hold for: for 'Hold expires on date:' yesterday and today should not be active [see potential issue [1]]
   . Circulation > Overdue report: in the filter make the from date in the future, Yesterday, Today and Tomorrow should all be not active, but are active even though they don't work
   . Struggled to find examples of when Tomorrow should not be selectable

Potential issues noted - mainly with Yesterday, Today and Tomorrow not showing as inactive when they should be:
1. When placing a hold you can't enter today's date but 'Today' is still active
2. Same for Circulation > Renew
3. Administration > Patron categories > New category - Yesterday and Today are still active, even though you can't enter a date
4. Reports > Patrons: the "Date of birth" fields are linked so that to cannot be before from dates, if you add a date in the from taht is a few days in the future tehy should all be inactive, but aren't
Comment 6 Martin Renvoize 2021-12-14 14:53:08 UTC
Created attachment 128510 [details] [review]
Bug 29552: Flatpickr - Disable shortcut buttons if inactive

If one of the buttons is not relevant we should disable it and mark is
as such on the interface.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2021-12-14 14:53:12 UTC
Created attachment 128511 [details] [review]
Bug 29552: (follow-up) Use CSS to set disabled class

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2021-12-14 14:53:15 UTC
Created attachment 128512 [details] [review]
Bug 29552: (follow-up) Reverse SCSS declaration order

I'm not sure why, but the order matters in this SCSS declaration. If the
&:hover is placed below the &:disabled, as it was, then it appears to
negate the effect of the &:disabled rule entirely.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2021-12-14 14:55:33 UTC
This works well once I applied my follow-up.

Test plan I followed:
1. Update CSS (This is a staff client patch.. so yarn build [no --view=opac])
2. Follow Davids test plan, but for staff client pages only.
Comment 10 Katrin Fischer 2021-12-19 11:24:47 UTC
I still see some glitches here:

1) Place a hold in staff and set the expiration date
- Yesterday is inactive - OK
- Today doesn't jump to today when clicked, but is still active (not OK)

2) Set a date of birth for a patron
- You can set a date of birth into the future. All options are active an and work. (possibly not related to this patch?)

3) Administration > Edit or add a patron category > Set the "Until date"
- Dates in the past are greyed out, Yesterday and Today are active, but won't select a date (not OK)
Comment 11 Jonathan Druart 2021-12-20 10:45:17 UTC
Created attachment 128729 [details] [review]
Bug 29552: Disable today for dates in the future
Comment 12 Jonathan Druart 2021-12-20 10:46:50 UTC
(In reply to Katrin Fischer from comment #10)
> I still see some glitches here:
> 
> 1) Place a hold in staff and set the expiration date
> - Yesterday is inactive - OK
> - Today doesn't jump to today when clicked, but is still active (not OK)

Done

> 2) Set a date of birth for a patron
> - You can set a date of birth into the future. All options are active an and
> work. (possibly not related to this patch?)

That does not seem directly related.

> 3) Administration > Edit or add a patron category > Set the "Until date"
> - Dates in the past are greyed out, Yesterday and Today are active, but
> won't select a date (not OK)

Opened bug 29735.
Comment 13 Jonathan Druart 2021-12-20 10:53:57 UTC
(In reply to Martin Renvoize from comment #9)
> This works well once I applied my follow-up.
> 
> Test plan I followed:
> 1. Update CSS (This is a staff client patch.. so yarn build [no --view=opac])
> 2. Follow Davids test plan, but for staff client pages only.

It's working without your patch but the cache is hard to clear (?)
I had to open a private window to make it work (so with Owen's patch only, not yours).
Comment 14 Katrin Fischer 2021-12-20 23:30:13 UTC
Created attachment 128806 [details] [review]
Bug 29552: Flatpickr - Disable shortcut buttons if inactive

If one of the buttons is not relevant we should disable it and mark is
as such on the interface.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Katrin Fischer 2021-12-20 23:30:17 UTC
Created attachment 128807 [details] [review]
Bug 29552: (follow-up) Use CSS to set disabled class

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Katrin Fischer 2021-12-20 23:30:21 UTC
Created attachment 128808 [details] [review]
Bug 29552: Disable today for dates in the future
Comment 17 Katrin Fischer 2021-12-20 23:31:01 UTC
Tested on top of bug 29735.
Comment 18 Katrin Fischer 2021-12-20 23:31:27 UTC
Created attachment 128809 [details] [review]
Bug 29552: Flatpickr - Disable shortcut buttons if inactive

If one of the buttons is not relevant we should disable it and mark is
as such on the interface.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Katrin Fischer 2021-12-20 23:31:31 UTC
Created attachment 128810 [details] [review]
Bug 29552: (follow-up) Use CSS to set disabled class

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2021-12-20 23:31:36 UTC
Created attachment 128811 [details] [review]
Bug 29552: Disable today for dates in the future

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Fridolin Somers 2022-01-14 02:38:40 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 22 Kyle M Hall 2022-01-14 14:31:48 UTC
Pushed to 21.11.x for 21.11.02
Comment 23 Andrew Fuerste-Henry 2022-01-25 17:44:19 UTC
Missing dependency, not backported to 21.05