Bug 28346 - Action buttons should have a class per action type
Summary: Action buttons should have a class per action type
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords:
Depends on:
Blocks: 13985
  Show dependency treegraph
 
Reported: 2021-05-13 10:49 UTC by Martin Renvoize (ashimema)
Modified: 2022-06-06 20:25 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Circulation function:


Attachments
Bug 28346: Add classes to account action buttons (7.52 KB, patch)
2021-05-13 10:55 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28346: Add classes to account action buttons (7.52 KB, patch)
2021-05-18 13:00 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28346: (QA follow-up) Correct silly mistake with cancel-action (1.42 KB, patch)
2021-05-18 13:00 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28346: (QA follow-up) Correct silly mistake with details-action (1.97 KB, patch)
2021-05-18 15:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28346: Add classes to account action buttons (7.58 KB, patch)
2021-05-18 15:58 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 28346: (QA follow-up) Correct silly mistake with cancel-action (1.48 KB, patch)
2021-05-18 15:58 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 28346: (QA follow-up) Correct silly mistake with details-action (2.03 KB, patch)
2021-05-18 15:58 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 28346: Add classes to account action buttons (7.68 KB, patch)
2021-08-18 08:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28346: (QA follow-up) Add `-action` for consistency (4.29 KB, patch)
2021-08-18 08:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28346: Add classes to account action buttons (7.75 KB, patch)
2021-08-18 12:40 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 28346: (QA follow-up) Add `-action` for consistency (4.36 KB, patch)
2021-08-18 12:40 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 Martin Renvoize (ashimema) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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 (ashimema) 2021-05-18 13:00:57 UTC
Good catch, seems I attached the class to the wrong element.. Doh!
Comment 6 Lucas Gass (lukeg) 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 (ashimema) 2021-05-18 14:41:18 UTC
Did I miss a case... 'details-action' looks like that one to me.
Comment 8 Lucas Gass (lukeg) 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 (ashimema) 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 (ashimema) 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 (lukeg) 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 (lukeg) 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 (lukeg) 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 (ashimema) 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 (tcohen) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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!