Bug 28187 - rowGroup headings are getting their styles overriden
Summary: rowGroup headings are getting their styles overriden
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 13985
  Show dependency treegraph
 
Reported: 2021-04-21 07:42 UTC by Martin Renvoize (ashimema)
Modified: 2022-06-06 20:24 UTC (History)
7 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.05.00,20.11.06
Circulation function:


Attachments
Screenshot highlighting problem (58.63 KB, image/png)
2021-04-21 07:44 UTC, Martin Renvoize (ashimema)
Details
Bug 28187: Make debit/credit row headers correct color (1.30 KB, patch)
2021-04-21 13:47 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 28187: Make debit/credit row headers correct color (1.35 KB, patch)
2021-04-22 11:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28187: Exclude dtrg-group from row striping. (1.79 KB, patch)
2021-05-14 10:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28187: Exclude dtrg-group from row striping. (1.84 KB, patch)
2021-05-14 13:26 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 28187: Exclude dtrg-group from row striping. (1.92 KB, patch)
2021-05-18 12:12 UTC, Kyle M Hall (khall)
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-04-21 07:42:24 UTC
The rowGroup datatables plugin used in the register details page for transaction groups is having it's header row background styling overridden by local css for the case where the row is 'odd'.
Comment 1 Martin Renvoize (ashimema) 2021-04-21 07:44:18 UTC
Created attachment 119936 [details]
Screenshot highlighting problem
Comment 2 Owen Leonard 2021-04-21 12:16:26 UTC
How is it supposed to look?
Comment 3 Martin Renvoize (ashimema) 2021-04-21 13:03:13 UTC
Ooops.. I cut of the text in my screenshot.. the first and second arrow rows are incorrect.. the third arrow points out the correct darker background colour.

i.e. we should continue to do our even/odd row highlighting.. but on top of that if it's a 'header row for a group' we should allow that to take precedence and take the darker third shade.
Comment 4 Lucas Gass (lukeg) 2021-04-21 13:47:53 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2021-04-22 11:59:21 UTC
Created attachment 119998 [details] [review]
Bug 28187: Make debit/credit row headers correct color

To test:
-Enable POS
-Create a register and make some transactions.
-Have both debits and credits
-Have some older tranactions so you can look at the past_sales table.
-Apply patch and regenerate the staff CSS: (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_interface)
-Look at the sales and past_sales table.
-Make sure each row header is the darker shade of gray (#e0e0e0)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Martin Renvoize (ashimema) 2021-04-22 12:21:48 UTC
Whilst this does fix the immediate issue, I was hoping to fix it at a higher level so subsequent uses of the rowGrouping plugin don't suffer the same problem.

The problem I was having is that I couldn't find where in our css source files the background color was coming from.
Comment 7 Martin Renvoize (ashimema) 2021-04-22 12:40:07 UTC
Indeed.. still stuck

How does the following rule get generated:

tbody tr:nth-child(odd) td:not(.bg-danger):not(.bg-warning):not(.bg-info):not(.bg-success):not(.bg-primary) {
    background-color: #f9f9f9;
}

and why does it take precedence over

table.dataTable tr.dtrg-group td {
    background-color: #e0e0e0;
}
Comment 8 Martin Renvoize (ashimema) 2021-04-22 12:41:25 UTC
The rowGroup plugin is also used for checkouts I believe.. which is a shared include in a number of templates.. so yeah.. I do still feel we need a higher level fix.

Sorry Lucas, thanks for working on it.
Comment 9 Martin Renvoize (ashimema) 2021-05-14 10:22:23 UTC
Created attachment 120956 [details] [review]
Bug 28187: Exclude dtrg-group from row striping.

The odd row lowlighting stripe effect added to datatales wasn't
accounting for classes introduced by the rowGroup plugin to denote
grouping header rows. This patch adds the required exclusion rule to
properly highlight rowGroup grouping rows.

Test plan
1/ Enable POS
2/ Create a register and make some transactions.
3/ Have both debits and credits
4/ Have some older tranactions so you can look at the past_sales table.
5/ Apply patch and regenerate the staff CSS:
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_interface)
6/ Look at the sales and past_sales table.
7/ Make sure each row header is the darker shade of gray (#e0e0e0)
7a/ You will need to have made some transactions with even and odd
numbers of items in the sale highlight the problem before applying the
patch
7b/ The rowGroup plugin is also used for the checkouts table and
this patch should also fix that case.
Comment 10 Martin Renvoize (ashimema) 2021-05-14 10:23:25 UTC
I've come up with an alternate solution now that catches the cases more widely.. back to NSO.. not sure why it took me so long to wrap my brain around that!
Comment 11 Lucas Gass (lukeg) 2021-05-14 13:26:09 UTC
Created attachment 120962 [details] [review]
Bug 28187: Exclude dtrg-group from row striping.

The odd row lowlighting stripe effect added to datatales wasn't
accounting for classes introduced by the rowGroup plugin to denote
grouping header rows. This patch adds the required exclusion rule to
properly highlight rowGroup grouping rows.

Test plan
1/ Enable POS
2/ Create a register and make some transactions.
3/ Have both debits and credits
4/ Have some older tranactions so you can look at the past_sales table.
5/ Apply patch and regenerate the staff CSS:
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_interface)
6/ Look at the sales and past_sales table.
7/ Make sure each row header is the darker shade of gray (#e0e0e0)
7a/ You will need to have made some transactions with even and odd
numbers of items in the sale highlight the problem before applying the
patch
7b/ The rowGroup plugin is also used for the checkouts table and
this patch should also fix that case.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 12 Victor Grousset/tuxayo 2021-05-18 09:59:09 UTC
> 7b/ The rowGroup plugin is also used for the checkouts table and
> this patch should also fix that case.

Where is that?

http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=XXX
http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=XXXX

?

Is there anything wrong?
https://wtf.roflcopter.fr/pics/gallery#vzlK8k2u/lgCojF0C.png,pjbLuxYq/d6NnmVoS.png
Comment 13 Victor Grousset/tuxayo 2021-05-18 10:04:00 UTC
Meanwhile, the main stuff works :D
Comment 14 Kyle M Hall (khall) 2021-05-18 12:12:26 UTC
Created attachment 121104 [details] [review]
Bug 28187: Exclude dtrg-group from row striping.

The odd row lowlighting stripe effect added to datatales wasn't
accounting for classes introduced by the rowGroup plugin to denote
grouping header rows. This patch adds the required exclusion rule to
properly highlight rowGroup grouping rows.

Test plan
1/ Enable POS
2/ Create a register and make some transactions.
3/ Have both debits and credits
4/ Have some older tranactions so you can look at the past_sales table.
5/ Apply patch and regenerate the staff CSS:
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_interface)
6/ Look at the sales and past_sales table.
7/ Make sure each row header is the darker shade of gray (#e0e0e0)
7a/ You will need to have made some transactions with even and odd
numbers of items in the sale highlight the problem before applying the
patch
7b/ The rowGroup plugin is also used for the checkouts table and
this patch should also fix that case.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Jonathan Druart 2021-05-19 13:08:12 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 16 Victor Grousset/tuxayo 2021-06-08 18:48:42 UTC
note: was backported to oldstable (20.11.x)
Comment 17 Victor Grousset/tuxayo 2021-06-08 18:49:05 UTC
Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed.