Bug 21918 - Clean up pay fines template
Summary: Clean up pay fines template
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2018-11-29 17:34 UTC by Christopher Brannon
Modified: 2019-02-01 15:35 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21918: Update buttons on fines to match newer buttons (10.37 KB, patch)
2018-12-03 19:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21918: Update buttons on fines to match newer buttons (11.38 KB, patch)
2018-12-04 19:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21918: Sorry about that, I think I may have been changing the wrong setting as it is working now. (9.90 KB, patch)
2019-01-17 20:15 UTC, helene
Details | Diff | Splinter Review
Bug 21918: Update buttons on fines to match newer buttons (11.47 KB, patch)
2019-01-18 19:40 UTC, José-Mario Monteiro-Santos
Details | Diff | Splinter Review
Bug 21918: Update buttons on fines to match newer buttons (11.55 KB, patch)
2019-01-20 14:00 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2018-11-29 17:34:07 UTC
Replace buttons with new buttons with font awesome, and change cancel links to buttons.
Comment 1 Owen Leonard 2018-11-29 17:38:40 UTC
(In reply to Christopher Brannon from comment #0)
> change cancel links to buttons.

"Cancel" is a link throughout the staff client. I think the only place where there is inconsistency is some modals.
Comment 2 Christopher Brannon 2018-11-29 18:12:08 UTC
(In reply to Owen Leonard from comment #1)
> (In reply to Christopher Brannon from comment #0)
> > change cancel links to buttons.
> 
> "Cancel" is a link throughout the staff client. I think the only place where
> there is inconsistency is some modals.

Is there a reason we want to keep these links?  If it is okay in the modals (and we can't do anything about the pop-up message buttons), should we change them to buttons for consistency?
Comment 3 Owen Leonard 2018-11-29 18:30:01 UTC
(In reply to Christopher Brannon from comment #2)
> Is there a reason we want to keep these links?

The intention is to make the "cancel" option less visible. The primary action is always going to be "Submit," and having a cancel link makes it very clear which is which.
Comment 4 Christopher Brannon 2018-11-29 19:32:08 UTC
(In reply to Owen Leonard from comment #3)
> (In reply to Christopher Brannon from comment #2)
> > Is there a reason we want to keep these links?
> 
> The intention is to make the "cancel" option less visible. The primary
> action is always going to be "Submit," and having a cancel link makes it
> very clear which is which.

Okay.  Is there a reason we are not doing this on the modals?
Comment 5 Owen Leonard 2018-12-03 18:19:41 UTC
I'd like to expand the scope of this bug to include:

  - Styling the "Pay" and "Write off" buttons as Bootstrap buttons.
  - Adding DataTables sorting, searching, and filtering.
  - Adding a column showing the fine date.
  - Hiding the "Add note" input, add a link to show it.
Comment 6 Owen Leonard 2018-12-03 19:01:22 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2018-12-03 19:52:22 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2018-12-04 19:26:26 UTC
Created attachment 82851 [details] [review]
Bug 21918: Update buttons on fines to match newer buttons

This patch makes a number of changes to the "Pay fines" template, the
goal of which is to make the interface look better in general. Changes
include:

 - Styling the "Pay" and "Write off" buttons as Bootstrap buttons.
 - Adding DataTables sorting, searching, and filtering.
 - Adding a column showing the fine date.
 - Hide the "Add note" input, add a link to show it.
 - Add some missing "preventDefault" methods from events.
 - Add "Writeoff selected" to the buttons which are disabled when there
   is no checkbox selection.
 - The <tfoot> tag is now expected by the HTML validator to be after
   <tbody>, so this section has been moved to suppress validator errors.

To test, apply the patch and locate a patron who has multiple
outstanding fines.

 - Go to Fines -> Pay fines
 - Confirm that the table of fines is shown as a DataTable, with sorting
   on columns which have data.
 - Test columns settings in Administration -> Columns settings.
 - Confirm that fine dates are shown, and that sorting by this column
   works correctly.
 - Click an "Add note" link to show a note input field. Focus should be
   in the newly-displayed input.
   - Confirm that notes are still saved correctly when paying or writing
     off single or multiple fines.
   - Confirm that when the "add note" input is hidden, its contents are
     erased.
 - Uncheck all checkboxes by clicking the "Clear all" link at the top.
   The "Pay selected" and "Write off selected" buttons should become
   disabled.
   - Checking any checkbox should re-enable those buttons.
 - Test that the relocated "Apply" button (for outstanding credits)
   still works as expected.
Comment 9 helene 2019-01-15 22:39:24 UTC
This is really good, but the column settings in administration are not having any effect.
Comment 10 Owen Leonard 2019-01-17 19:09:07 UTC
I can't reproduce that problem in my test system. Could you please try again?
Comment 11 helene 2019-01-17 20:15:39 UTC
Created attachment 84155 [details] [review]
Bug 21918: Sorry about that, I think I may have been changing the wrong setting as it is working now.

Signed-off-by: helene hickey <hickeyhe@wgc.school.nz>
Comment 12 José-Mario Monteiro-Santos 2019-01-18 19:40:28 UTC
Created attachment 84213 [details] [review]
Bug 21918: Update buttons on fines to match newer buttons

This patch makes a number of changes to the "Pay fines" template, the
goal of which is to make the interface look better in general. Changes
include:

 - Styling the "Pay" and "Write off" buttons as Bootstrap buttons.
 - Adding DataTables sorting, searching, and filtering.
 - Adding a column showing the fine date.
 - Hide the "Add note" input, add a link to show it.
 - Add some missing "preventDefault" methods from events.
 - Add "Writeoff selected" to the buttons which are disabled when there
   is no checkbox selection.
 - The <tfoot> tag is now expected by the HTML validator to be after
   <tbody>, so this section has been moved to suppress validator errors.

To test, apply the patch and locate a patron who has multiple
outstanding fines.

 - Go to Fines -> Pay fines
 - Confirm that the table of fines is shown as a DataTable, with sorting
   on columns which have data.
 - Test columns settings in Administration -> Columns settings.
 - Confirm that fine dates are shown, and that sorting by this column
   works correctly.
 - Click an "Add note" link to show a note input field. Focus should be
   in the newly-displayed input.
   - Confirm that notes are still saved correctly when paying or writing
     off single or multiple fines.
   - Confirm that when the "add note" input is hidden, its contents are
     erased.
 - Uncheck all checkboxes by clicking the "Clear all" link at the top.
   The "Pay selected" and "Write off selected" buttons should become
   disabled.
   - Checking any checkbox should re-enable those buttons.
 - Test that the relocated "Apply" button (for outstanding credits)
   still works as expected.

Signed-off-by: Jose-Mario Monteiro-Santos <jose-mario.monteiro-santos@inLibro.com>
Comment 13 Katrin Fischer 2019-01-20 14:00:33 UTC
Created attachment 84248 [details] [review]
Bug 21918: Update buttons on fines to match newer buttons

This patch makes a number of changes to the "Pay fines" template, the
goal of which is to make the interface look better in general. Changes
include:

 - Styling the "Pay" and "Write off" buttons as Bootstrap buttons.
 - Adding DataTables sorting, searching, and filtering.
 - Adding a column showing the fine date.
 - Hide the "Add note" input, add a link to show it.
 - Add some missing "preventDefault" methods from events.
 - Add "Writeoff selected" to the buttons which are disabled when there
   is no checkbox selection.
 - The <tfoot> tag is now expected by the HTML validator to be after
   <tbody>, so this section has been moved to suppress validator errors.

To test, apply the patch and locate a patron who has multiple
outstanding fines.

 - Go to Fines -> Pay fines
 - Confirm that the table of fines is shown as a DataTable, with sorting
   on columns which have data.
 - Test columns settings in Administration -> Columns settings.
 - Confirm that fine dates are shown, and that sorting by this column
   works correctly.
 - Click an "Add note" link to show a note input field. Focus should be
   in the newly-displayed input.
   - Confirm that notes are still saved correctly when paying or writing
     off single or multiple fines.
   - Confirm that when the "add note" input is hidden, its contents are
     erased.
 - Uncheck all checkboxes by clicking the "Clear all" link at the top.
   The "Pay selected" and "Write off selected" buttons should become
   disabled.
   - Checking any checkbox should re-enable those buttons.
 - Test that the relocated "Apply" button (for outstanding credits)
   still works as expected.

Signed-off-by: Jose-Mario Monteiro-Santos <jose-mario.monteiro-santos@inLibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2019-01-20 14:01:52 UTC
Testing this I got confused about the payment note feature. I am wondering, do we need it to be on the payments table? It seems a little confusing as you seem to only be able to save it actually paying a fine. Wouldn't it be enough if the field appeared editable on the actual form? Maybe I am missing something!
Comment 15 Christopher Brannon 2019-01-22 15:33:38 UTC
(In reply to Katrin Fischer from comment #14)
> Testing this I got confused about the payment note feature. I am wondering,
> do we need it to be on the payments table? It seems a little confusing as
> you seem to only be able to save it actually paying a fine. Wouldn't it be
> enough if the field appeared editable on the actual form? Maybe I am missing
> something!

I agree that this is a little wonky.

It would also be helpful if notes could be updated or edited after the fact, but this is a topic for another ticket.
Comment 16 Aleisha Amohia 2019-01-22 20:19:40 UTC
(In reply to Christopher Brannon from comment #15)
> (In reply to Katrin Fischer from comment #14)
> > Testing this I got confused about the payment note feature. I am wondering,
> > do we need it to be on the payments table? It seems a little confusing as
> > you seem to only be able to save it actually paying a fine. Wouldn't it be
> > enough if the field appeared editable on the actual form? Maybe I am missing
> > something!

Made Bug 22184 for this

> 
> I agree that this is a little wonky.
> 
> It would also be helpful if notes could be updated or edited after the fact,
> but this is a topic for another ticket.

Made Bug 22185 for this
Comment 17 Katrin Fischer 2019-01-24 07:25:00 UTC
Thx, Aleihsa! Also thx Christopher for weighing in! (not just me :)
Comment 18 Nick Clemens 2019-01-31 13:38:06 UTC
Awesome work all!

Pushed to master for 19.05
Comment 19 Martin Renvoize 2019-02-01 15:35:24 UTC
Enhancement, will not be backported to 18.11.x series.