Description
Kelly McElligott
2020-04-13 16:42:54 UTC
12 hour format is easier for our staff to read. We would appreciate if this bug could be fixed to restore the functionality we had prior to the 19.05.08 update. Confirmed on master Created attachment 103137 [details] [review] Bug 25133: Fix time part of due date for 12hr Restoring the previous behaviour would be a regression in my opinion. I think this patch is the correct way to fix it (ie. always display 12hr format) Nick, can you give it a try before I request a signoff please? (In reply to Jonathan Druart from comment #4) > Restoring the previous behaviour would be a regression in my opinion. > I think this patch is the correct way to fix it (ie. always display 12hr > format) > > Nick, can you give it a try before I request a signoff please? In testing, all of my times are now AM (In reply to Nick Clemens from comment #5) > (In reply to Jonathan Druart from comment #4) > > Restoring the previous behaviour would be a regression in my opinion. > > I think this patch is the correct way to fix it (ie. always display 12hr > > format) > > > > Nick, can you give it a try before I request a signoff please? > > In testing, all of my times are now AM Hum, looks like it's a bug in master actually. (In reply to Jonathan Druart from comment #6) > (In reply to Nick Clemens from comment #5) > > (In reply to Jonathan Druart from comment #4) > > > Restoring the previous behaviour would be a regression in my opinion. > > > I think this patch is the correct way to fix it (ie. always display 12hr > > > format) > > > > > > Nick, can you give it a try before I request a signoff please? > > > > In testing, all of my times are now AM > > Hum, looks like it's a bug in master actually. Forget that, wrong git branch! Created attachment 103236 [details] [review] Bug 25133: Handle 12hr format for dt_from_string Created attachment 103366 [details] [review] Bug 25133: Fix time part of due date for 12hr Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 103367 [details] [review] Bug 25133: Handle 12hr format for dt_from_string Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 103368 [details] [review] Bug 25133: (follow-up) Remove Names as they do no affect display In testing I couldn't make any changes to the names have an effect and everythign worked as expected without these lines Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 103369 [details] [review] Bug 25133: Fix time part of due date for 12hr Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Created attachment 103370 [details] [review] Bug 25133: Handle 12hr format for dt_from_string Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Created attachment 103371 [details] [review] Bug 25133: (follow-up) Remove Names as they do no affect display In testing I couldn't make any changes to the names have an effect and everythign worked as expected without these lines Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Nick, Kelly, could you detail what you have tried? This patch will affect all date inputs so we need quite intensive testing. This works but wondering - is it expected that we only allow to add tags with emojis from the detail page, but not from the result list? (In reply to Katrin Fischer from comment #16) > This works but wondering - is it expected that we only allow to add tags > with emojis from the detail page, but not from the result list? Oviously wrong bug. I know this is a bad bug... but why is there no test plan in the commits and no proper descriptions? :( Created attachment 104228 [details] [review] Bug 25133: Fix time part of due date for 12hr With Timeformat system preference set to 12 hours format: Prior to 19.05.08, when due date was specified at checkout, the time in the box would show in 24hr format (e.g. 15:59) while the due date in the patron's checkouts would show in 12hr format (e.g. 3:59 PM). After 19.05.08, the format in the specify a due date box shows in 24hr format and the due date in 12hr format (in patron checkouts) as before. But when checking out multiple items using the specify due date option for the session, the time changes from PM to AM. To test: - Check out an item with specified due date 15:50 PM - Check out a second item in same transaction - Verify date changed to AM - Apply patch - Repeat - Verify this time the date is kept as set Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 104229 [details] [review] Bug 25133: Handle 12hr format for dt_from_string Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 104230 [details] [review] Bug 25133: (follow-up) Remove Names as they do no affect display In testing I couldn't make any changes to the names have an effect and everythign worked as expected without these lines Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Nice work everyone! Pushed to master for 20.05 I personally did not test those patches enough to confirm it could be backported without risk. Just a quick note here, as I can't test further right now, but this causes a JavaScript error when adding an order line (and possible on other pages): TypeError: $.timepicker is undefined Bad bug, fix needed urgently! (In reply to Katrin Fischer from comment #24) > Just a quick note here, as I can't test further right now, but this causes a > JavaScript error when adding an order line (and possible on other pages): > > TypeError: $.timepicker is undefined > > Bad bug, fix needed urgently! Looking now, should the changes to calendar.inc should have gone into timepicker.inc instead? What about including timepicker.inc from calendar.inc? (In reply to Jonathan Druart from comment #26) > What about including timepicker.inc from calendar.inc? That's silly :) Yes Nick, it should! We actually just need 'timeFormat' to be defined in timepicker.inc Created attachment 104260 [details] [review] Bug 25133: (follow-up) Move changes to timepicker.inc Witht the changes in calendar.inc there can be a js error where the timepicker is not included. Moving the changes to timepicker.inc should resolve this Note: in previous patches the amnames/pmnames were removed. It is possible they had no effect because they were declared in two files, so I have left them in timepicker.inc Created attachment 104269 [details] [review] Bug 25133: (follow-up) Move changes to timepicker.inc Witht the changes in calendar.inc there can be a js error where the timepicker is not included. Moving the changes to timepicker.inc should resolve this Note: in previous patches the amnames/pmnames were removed. It is possible they had no effect because they were declared in two files, so I have left them in timepicker.inc Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Nice work everyone! Pushed to master for 20.05 *** Bug 25365 has been marked as a duplicate of this bug. *** Backported to 19.11.x for 19.11.06 backported to 19.05.x for 19.05.11 |