Bug 31766

Summary: Add 'page-section' to notices and slips page
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Staff interfaceAssignee: Lucas Gass (lukeg) <lucas>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: david, gmcharlt, koha-bugs, lucas, oleonard, testopia
Version: MainKeywords: Academy
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:
22.11.00
Circulation function:
Bug Depends on: 30952    
Bug Blocks: 31757    
Attachments: page-section class directly on the table
page-section class on wrapper around table
Bug 31766: Add page-section wrapper to notice table
Bug 31766: Add page-section wrapper to notice table
Bug 31766: Add page-section wrapper to notice table

Description Martin Renvoize (ashimema) 2022-10-13 12:16:00 UTC
We need the page-section div adding to the notices and slips page to ensure proper contrast for even/odd rows on the tables.
Comment 1 Lucas Gass (lukeg) 2022-10-13 15:30:24 UTC
Created attachment 141827 [details]
page-section class directly on the table
Comment 2 Lucas Gass (lukeg) 2022-10-13 15:31:40 UTC
Created attachment 141828 [details]
page-section class on wrapper around table
Comment 3 Lucas Gass (lukeg) 2022-10-13 15:37:01 UTC
I started to look at this and wasn't sure how to proceed. I added the page-section class directly to the table ( see attached )

Then I added the page-section class as a wrapper around the table. ( see attached )

When we use a wrapper I like that the dataTables tools are part of the "page-section", making it obvious the tools go with the table. 

However, when page-section is applied directly to the table I like the look of the table header's better. I don't like how that table header's just run right into the page-section in the 2nd example.
Comment 4 Lucas Gass (lukeg) 2022-10-13 15:57:20 UTC
Created attachment 141829 [details] [review]
Bug 31766: Add page-section wrapper to notice table

To test:
1. Apply patch
2. Look at the noitce page and notice how the table, and it's tools, now are wrapped with the page-section element.
3. Is it better than it was before?

Feel free to osboslete these patch and replace it with your own.
Comment 5 David Nind 2022-10-13 21:51:33 UTC
Created attachment 141848 [details] [review]
Bug 31766: Add page-section wrapper to notice table

To test:
1. Apply patch
2. Look at the noitce page and notice how the table, and it's tools, now are wrapped with the page-section element.
3. Is it better than it was before?

Feel free to osboslete these patch and replace it with your own.

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2022-10-13 21:57:54 UTC
It is better than it was before, so I've signed off.

However, I like the border around the table and table header (in the first screenshot).

For me:
- the header/table blends in too much with the background without the borders
- with the border, it makes it much easer to scan the table column headers and rows
Comment 7 Lucas Gass (lukeg) 2022-10-13 22:14:55 UTC
(In reply to David Nind from comment #6)
> It is better than it was before, so I've signed off.
> 
> However, I like the border around the table and table header (in the first
> screenshot).
> 
> For me:
> - the header/table blends in too much with the background without the borders
> - with the border, it makes it much easer to scan the table column headers
> and rows

David, I agree. The header/table blends in too much, this is happening on this page and elsewhere. I think we'll need to address that.
Comment 8 Martin Renvoize (ashimema) 2022-10-14 07:35:37 UTC
Thanks both, I'm happy to be persuaded about the table borders.. we worked a fair bit on refining the tables already, but I'm sure there are further refinements we can make.. any chance one of you could open a new bug for it so we don't lose it in here?

For reference, I think the majority of prior discussion aroung this was on github issue https://github.com/jajm/koha-staff-interface-redesign/issues/22
Comment 9 Lucas Gass (lukeg) 2022-10-14 15:59:38 UTC
(In reply to Martin Renvoize from comment #8)
> Thanks both, I'm happy to be persuaded about the table borders.. we worked a
> fair bit on refining the tables already, but I'm sure there are further
> refinements we can make.. any chance one of you could open a new bug for it
> so we don't lose it in here?
> 
> For reference, I think the majority of prior discussion aroung this was on
> github issue https://github.com/jajm/koha-staff-interface-redesign/issues/22

Do we need a separate bug to talk about table headings/borders?
Comment 10 Katrin Fischer 2022-10-15 07:53:18 UTC
Created attachment 141914 [details] [review]
Bug 31766: Add page-section wrapper to notice table

To test:
1. Apply patch
2. Look at the noitce page and notice how the table, and it's tools, now are wrapped with the page-section element.
3. Is it better than it was before?

Feel free to osboslete these patch and replace it with your own.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2022-10-15 07:54:09 UTC
(In reply to Lucas Gass from comment #9)
> (In reply to Martin Renvoize from comment #8)
> > Thanks both, I'm happy to be persuaded about the table borders.. we worked a
> > fair bit on refining the tables already, but I'm sure there are further
> > refinements we can make.. any chance one of you could open a new bug for it
> > so we don't lose it in here?
> > 
> > For reference, I think the majority of prior discussion aroung this was on
> > github issue https://github.com/jajm/koha-staff-interface-redesign/issues/22
> 
> Do we need a separate bug to talk about table headings/borders?

Might be good :) But not sure if there is one yet already, still diving in.
Comment 12 Tomás Cohen Arazi (tcohen) 2022-10-17 11:08:50 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!