Bug 31766 - Add 'page-section' to notices and slips page
Summary: Add 'page-section' to notices and slips page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lucas Gass
QA Contact: Katrin Fischer
URL:
Keywords: Academy
Depends on: 30952
Blocks: 31757
  Show dependency treegraph
 
Reported: 2022-10-13 12:16 UTC by Martin Renvoize
Modified: 2023-12-28 20:42 UTC (History)
6 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:
22.11.00


Attachments
page-section class directly on the table (279.20 KB, image/png)
2022-10-13 15:30 UTC, Lucas Gass
Details
page-section class on wrapper around table (229.83 KB, image/png)
2022-10-13 15:31 UTC, Lucas Gass
Details
Bug 31766: Add page-section wrapper to notice table (12.58 KB, patch)
2022-10-13 15:57 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 31766: Add page-section wrapper to notice table (12.63 KB, patch)
2022-10-13 21:51 UTC, David Nind
Details | Diff | Splinter Review
Bug 31766: Add page-section wrapper to notice table (12.69 KB, patch)
2022-10-15 07:53 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 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 2022-10-13 15:30:24 UTC
Created attachment 141827 [details]
page-section class directly on the table
Comment 2 Lucas Gass 2022-10-13 15:31:40 UTC
Created attachment 141828 [details]
page-section class on wrapper around table
Comment 3 Lucas Gass 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 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 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 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 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 2022-10-17 11:08:50 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!