Bug 28187 - rowGroup headings are getting their styles overriden
Summary: rowGroup headings are getting their styles overriden
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 13985
  Show dependency treegraph
 
Reported: 2021-04-21 07:42 UTC by Martin Renvoize
Modified: 2021-06-08 18:49 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06


Attachments
Screenshot highlighting problem (58.63 KB, image/png)
2021-04-21 07:44 UTC, Martin Renvoize
Details
Bug 28187: Make debit/credit row headers correct color (1.30 KB, patch)
2021-04-21 13:47 UTC, Lucas Gass
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
Details | Diff | Splinter Review
Bug 28187: Exclude dtrg-group from row striping. (1.84 KB, patch)
2021-05-14 13:26 UTC, Lucas Gass
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
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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 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 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 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 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 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 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 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 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.