Bug 31758 - Add 'page-section' to system preferences page
Summary: Add 'page-section' to system preferences page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 30952
Blocks: 31757 31886
  Show dependency treegraph
 
Reported: 2022-10-13 08:40 UTC by Martin Renvoize
Modified: 2023-12-28 20:42 UTC (History)
3 users (show)

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


Attachments
Bug 31758: OPTION 1 - Add page-section to prefs-tab div (1.29 KB, patch)
2022-10-13 08:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31758: OPTION 2 - Add page-section div around subsections (1.77 KB, patch)
2022-10-13 08:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31758: OPTION 2 (follow-up) Add padding between adjacent sections (790 bytes, patch)
2022-10-13 10:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 31758: OPTION 2 - Add page-section div around subsections (1.83 KB, patch)
2022-10-13 16:10 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 31758: OPTION 2 (follow-up) Add padding between adjacent sections (845 bytes, patch)
2022-10-13 16:10 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 31758: (follow-up) Remove fat margin-top (1.10 KB, patch)
2022-10-13 16:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31758: OPTION 2 - Add page-section div around subsections (1.88 KB, patch)
2022-10-15 08:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31758: OPTION 2 (follow-up) Add padding between adjacent sections (903 bytes, patch)
2022-10-15 08:03 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 08:40:21 UTC
We need the page-section divs adding to the system preferences page to ensure proper contrast for even/odd rows on the tables.

See https://github.com/jajm/koha-staff-interface-redesign/issues/55 for discusion.
Comment 1 Martin Renvoize 2022-10-13 08:57:43 UTC
Created attachment 141769 [details] [review]
Bug 31758: OPTION 1 - Add page-section to prefs-tab div

This patch simply adds the new page-section class to the existing
prefs-tab div encasing the whole tab content inside a 'card' display.
Comment 2 Martin Renvoize 2022-10-13 08:58:07 UTC
Created attachment 141770 [details] [review]
Bug 31758: OPTION 2 - Add page-section div around subsections

An alternative that puts adds a new page-section div around each
preference subsection rather than around the whole tab section.
Comment 3 Martin Renvoize 2022-10-13 08:58:30 UTC
Needs some input.. calling Owen :)
Comment 4 Martin Renvoize 2022-10-13 09:00:19 UTC
Option 1 just works.. Option 2 splits the content up more and 'might' be nicer.. but needs a little refinement still if people like it..
Comment 5 Owen Leonard 2022-10-13 10:52:54 UTC
Created attachment 141793 [details] [review]
Bug 31758: OPTION 2 (follow-up) Add padding between adjacent sections
Comment 6 Owen Leonard 2022-10-13 10:53:40 UTC
My preference is option 2, with some padding between the sections.
Comment 7 Martin Renvoize 2022-10-13 11:17:02 UTC
Excellent follow-up Owen, thanks.. I think I prefer option 2 too... 

However, what do you think to what it does to the look of search results when you search for a system preference?.. It's not terrible, but also doesn't look quite right to me yet either... contemplating.
Comment 8 Martin Renvoize 2022-10-13 11:24:05 UTC
Also... if I do a search for a preference, say 'UserJS'.. if I then click through using the 'View all X preferences' link on the right of the section.. I end up with a larger top margin on the section I'm looking at.. seems to come from custom styling on 'h3:target'.. not sure how/what it was/is trying to achieve and whether we're safe to remove it now.
Comment 9 Lucas Gass 2022-10-13 15:08:18 UTC
(In reply to Martin Renvoize from comment #7)
> Excellent follow-up Owen, thanks.. I think I prefer option 2 too... 
> 
> However, what do you think to what it does to the look of search results
> when you search for a system preference?.. It's not terrible, but also
> doesn't look quite right to me yet either... contemplating.

I also prefer option #2. 

When you search for system preferences and return more than 1 in a single category they look a bit "smushed" together. If there was just a tiny margin between the two I think it looks more organized. IMHO.
Comment 10 Lucas Gass 2022-10-13 16:07:15 UTC
(In reply to Lucas Gass from comment #9)
> (In reply to Martin Renvoize from comment #7)
> > Excellent follow-up Owen, thanks.. I think I prefer option 2 too... 
> > 
> > However, what do you think to what it does to the look of search results
> > when you search for a system preference?.. It's not terrible, but also
> > doesn't look quite right to me yet either... contemplating.
> 
> I also prefer option #2. 
> 
> When you search for system preferences and return more than 1 in a single
> category they look a bit "smushed" together. If there was just a tiny margin
> between the two I think it looks more organized. IMHO.

Ignore my comment, this does create a nice margin. I didn't 'yarn build'.
Comment 11 Lucas Gass 2022-10-13 16:10:47 UTC
Created attachment 141830 [details] [review]
Bug 31758: OPTION 2 - Add page-section div around subsections

An alternative that puts adds a new page-section div around each
preference subsection rather than around the whole tab section.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 12 Lucas Gass 2022-10-13 16:10:51 UTC
Created attachment 141831 [details] [review]
Bug 31758: OPTION 2 (follow-up) Add padding between adjacent sections

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 13 Martin Renvoize 2022-10-13 16:31:41 UTC
Created attachment 141834 [details] [review]
Bug 31758: (follow-up) Remove fat margin-top

This removes the increased top margin appearing on subsection headings
after clicking through from search..

I'm not sure if we need it or not now.. the original feature it appeared
to help with doesn't work for me.. scroll to section doesn't happen.
Comment 14 Owen Leonard 2022-10-13 16:53:47 UTC
(In reply to Martin Renvoize from comment #13)
> Created attachment 141834 [details] [review] [review]
> Bug 31758: (follow-up) Remove fat margin-top
> 
> This removes the increased top margin appearing on subsection headings
> after clicking through from search..

The purpose of the extra margin can be seen if you click one of the sidebar subheadings to jump down. Without the extra margin, when you jump to that section the heading is obscured by the floating toolbar.
Comment 15 Katrin Fischer 2022-10-15 08:02:45 UTC
(In reply to Owen Leonard from comment #14)
> (In reply to Martin Renvoize from comment #13)
> > Created attachment 141834 [details] [review] [review] [review]
> > Bug 31758: (follow-up) Remove fat margin-top
> > 
> > This removes the increased top margin appearing on subsection headings
> > after clicking through from search..
> 
> The purpose of the extra margin can be seen if you click one of the sidebar
> subheadings to jump down. Without the extra margin, when you jump to that
> section the heading is obscured by the floating toolbar.

I suggest we leave the last patch off for now, and then we could figure this one out still on a separate bug if we want.
Comment 16 Katrin Fischer 2022-10-15 08:03:53 UTC
Created attachment 141915 [details] [review]
Bug 31758: OPTION 2 - Add page-section div around subsections

An alternative that puts adds a new page-section div around each
preference subsection rather than around the whole tab section.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2022-10-15 08:03:58 UTC
Created attachment 141916 [details] [review]
Bug 31758: OPTION 2 (follow-up) Add padding between adjacent sections

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Martin Renvoize 2022-10-15 11:39:39 UTC
Good call obsoleting the third patch.. we probably should have a big for it though ;)
Comment 19 Katrin Fischer 2022-10-15 14:46:31 UTC
(In reply to Martin Renvoize from comment #18)
> Good call obsoleting the third patch.. we probably should have a big for it
> though ;)

I left it open, because after Owen's explanation I was ok with the current behaviour.
Comment 20 Tomás Cohen Arazi 2022-10-17 11:08:33 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!