Bug 28346

Summary: Action buttons should have a class per action type
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Fines and feesAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, kyle, lucas, sally.healey, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on:    
Bug Blocks: 13985    
Attachments: Bug 28346: Add classes to account action buttons
Bug 28346: Add classes to account action buttons
Bug 28346: (QA follow-up) Correct silly mistake with cancel-action
Bug 28346: (QA follow-up) Correct silly mistake with details-action
Bug 28346: Add classes to account action buttons
Bug 28346: (QA follow-up) Correct silly mistake with cancel-action
Bug 28346: (QA follow-up) Correct silly mistake with details-action
Bug 28346: Add classes to account action buttons
Bug 28346: (QA follow-up) Add `-action` for consistency
Bug 28346: Add classes to account action buttons
Bug 28346: (QA follow-up) Add `-action` for consistency

Description Martin Renvoize 2021-05-13 10:49:17 UTC
The action buttons are hard to select via jQuery to allow custom changes via UserCSS/UserJS.. we should add classes for each type of action so they can be custom styled.
Comment 1 Martin Renvoize 2021-05-13 10:55:27 UTC
Created attachment 120917 [details] [review]
Bug 28346: Add classes to account action buttons

This patch adds identifiable classes to each action button that may be
displayed next to an accountline on the borrower account page.
Comment 2 Sally 2021-05-18 10:50:36 UTC
Hi Martin,

Just one button missing a unique class, which is the cancel charge button:

button type="submit" class="btn btn-default btn-xs"
Comment 3 Martin Renvoize 2021-05-18 13:00:24 UTC
Created attachment 121108 [details] [review]
Bug 28346: Add classes to account action buttons

This patch adds identifiable classes to each action button that may be
displayed next to an accountline on the borrower account page.
Comment 4 Martin Renvoize 2021-05-18 13:00:27 UTC
Created attachment 121109 [details] [review]
Bug 28346: (QA follow-up) Correct silly mistake with cancel-action

This patch moves the cancel-action class introduced in the previous
commit from the font-icon to the button where it should be.
Comment 5 Martin Renvoize 2021-05-18 13:00:57 UTC
Good catch, seems I attached the class to the wrong element.. Doh!
Comment 6 Lucas Gass 2021-05-18 13:43:51 UTC
What about the 'Details' button that goes to 'accountline-details.pl?accountlines_id=X'. That one should have a class as well right?
Comment 7 Martin Renvoize 2021-05-18 14:41:18 UTC
Did I miss a case... 'details-action' looks like that one to me.
Comment 8 Lucas Gass 2021-05-18 15:11:10 UTC
(In reply to Martin Renvoize from comment #7)
> Did I miss a case... 'details-action' looks like that one to me.

Ah, yes I see 'details-action' but it on the icon element, not the button:

<i class="fa fa-list details-action"></i>
Comment 9 Martin Renvoize 2021-05-18 15:40:58 UTC
Created attachment 121118 [details] [review]
Bug 28346: (QA follow-up) Correct silly mistake with details-action

This patch moves the details-action class introduced in the first
commit from the font-icon to the button where it should be.
Comment 10 Martin Renvoize 2021-05-18 15:41:40 UTC
Clearly I was having a bad day when I wrote this patch!.. Two simple follow-ups later, hopefully I've got it this time!
Comment 11 Lucas Gass 2021-05-18 15:58:36 UTC
Created attachment 121120 [details] [review]
Bug 28346: Add classes to account action buttons

This patch adds identifiable classes to each action button that may be
displayed next to an accountline on the borrower account page.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 12 Lucas Gass 2021-05-18 15:58:40 UTC
Created attachment 121121 [details] [review]
Bug 28346: (QA follow-up) Correct silly mistake with cancel-action

This patch moves the cancel-action class introduced in the previous
commit from the font-icon to the button where it should be.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 13 Lucas Gass 2021-05-18 15:58:43 UTC
Created attachment 121122 [details] [review]
Bug 28346: (QA follow-up) Correct silly mistake with details-action

This patch moves the details-action class introduced in the first
commit from the font-icon to the button where it should be.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 14 Katrin Fischer 2021-07-03 22:22:00 UTC
Just wondering: if they are supposed to be and remain unique, should they not be ids?
Comment 15 Martin Renvoize 2021-08-05 09:51:35 UTC
(In reply to Katrin Fischer from comment #14)
> Just wondering: if they are supposed to be and remain unique, should they
> not be ids?

I've reworded the bug title.. I didn't mean ID's.. I meant Classes, one per action type.
Comment 16 Tomás Cohen Arazi 2021-08-17 13:24:14 UTC
Could we squash this patches? In this tiny change it makes sense to have a single patch. Also

- payout-amount
- invoice-print
- receipt-print

=> should have the -action suffix for consistency.
Comment 17 Martin Renvoize 2021-08-18 08:46:43 UTC
Created attachment 123923 [details] [review]
Bug 28346: Add classes to account action buttons

This patch adds identifiable classes to each action button that may be
displayed next to an accountline on the borrower account page.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 18 Martin Renvoize 2021-08-18 08:46:47 UTC
Created attachment 123924 [details] [review]
Bug 28346: (QA follow-up) Add  `-action` for consistency
Comment 19 Martin Renvoize 2021-08-18 08:48:22 UTC
Patches squashed and follow-up added to address -action consistency concerns.

Feel free to squash the follow-up on push.. I was not entirely sure about updating the pre-existing 'void' class and hence kept the patch distinct for now.
Comment 20 Katrin Fischer 2021-08-18 12:40:47 UTC
Created attachment 123953 [details] [review]
Bug 28346: Add classes to account action buttons

This patch adds identifiable classes to each action button that may be
displayed next to an accountline on the borrower account page.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Katrin Fischer 2021-08-18 12:40:53 UTC
Created attachment 123954 [details] [review]
Bug 28346: (QA follow-up) Add `-action` for consistency

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Jonathan Druart 2021-08-18 13:36:17 UTC
Pushed to master for 21.11, thanks to everybody involved!