Bug 34119 - Improve staff interface print stylesheet following redesign
Summary: Improve staff interface print stylesheet following redesign
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Katrin Fischer
URL:
Keywords:
: 29979 35016 (view as bug list)
Depends on: 30952
Blocks:
  Show dependency treegraph
 
Reported: 2023-06-23 19:51 UTC by Phil Ringnalda
Modified: 2023-11-13 13:45 UTC (History)
6 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:
23.11.00,23.05.05,22.11.12


Attachments
Bug 34119 - Use !important to make print.css beat staff-global.css at hiding SQL report cruft (1.16 KB, patch)
2023-06-28 00:31 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 34119: (Preliminary) Run stylelint fixes on print.css (11.13 KB, patch)
2023-10-10 15:27 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34119: Improve staff interface print stylesheet following redesign (2.00 KB, patch)
2023-10-10 15:28 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34119: (Preliminary) Run stylelint fixes on print.css (11.22 KB, patch)
2023-10-11 10:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34119: Improve staff interface print stylesheet following redesign (2.09 KB, patch)
2023-10-11 10:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34119: (Preliminary) Run stylelint fixes on print.css (11.24 KB, patch)
2023-10-14 17:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34119: Improve staff interface print stylesheet following redesign (2.11 KB, patch)
2023-10-14 17:07 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 Phil Ringnalda 2023-06-23 19:51:49 UTC
The fistful of selectors in https://git.koha-community.org/Koha-community/Koha/src/commit/a57278f39b8c5b7a82b21671f266dbf32e2d41a0/koha-tmpl/intranet-tmpl/prog/css/print.css#L289 intend to hide things like the header search and the button to send results to batch modification when SQL reports are printed, but after the redesign they no longer apply. I think that's because staff-global.css, which is referenced after print.css, previously wasn't setting display for them, and now for most it does.

It can be fixed with my favorite CSS hammer, !important, but I've been told that's not the only hammer which exists. Is there something else we should do instead?
Comment 1 Katrin Fischer 2023-06-25 11:44:29 UTC
*** Bug 29979 has been marked as a duplicate of this bug. ***
Comment 2 Phil Ringnalda 2023-06-28 00:31:41 UTC
Created attachment 152775 [details] [review]
Bug 34119 - Use !important to make print.css beat staff-global.css at hiding SQL report cruft

Test plan:
1. Run an SQL report (or create a simple one like "select itemnumber from
   items" if you don't have any). Shift-reload the results, since you
   probably have print.css cached.
2. File - Print
3. Make sure the print preview doesn't show the header search box, or
   the New report, Edit, Run report buttons (the logo, the hamburger
   menu and the help ? icon are a separate bug).
Comment 3 Phil Ringnalda 2023-06-28 00:32:24 UTC
I should know better than to ask a question in any form other than a patch :)
Comment 4 Lucas Gass 2023-07-06 17:56:42 UTC
Should we hide the other elements on the page that would be unwanted when printing? The Koha logo and help link still appear.
Comment 5 Phil Ringnalda 2023-07-06 18:54:47 UTC
At the time I wrote it, this seemed like the simplest possible SO+PQA+backport patch to keep us from wasting a third of a sheet of paper any time someone prints a report, and then I was going to reopen cait's bug 29979 to deal with the problem of .navbar replacing, um, probably several of the selectors in this rule, I'm not sure exactly which, and adding in one for anything new that's not in .navbar.

I'm going to be gone until August 1st, and I've already solved the problem for myself in IntranetUserCSS, so if someone wants to take this bug and sort through what selectors have completely disappeared, not just disappeared from reports, and replace them with the new thing, and also fix !important in the same patch, feel free.
Comment 6 Katrin Fischer 2023-07-07 10:06:23 UTC
(In reply to Lucas Gass from comment #4)
> Should we hide the other elements on the page that would be unwanted when
> printing? The Koha logo and help link still appear.

I'd say so!
Comment 7 Katrin Fischer 2023-07-07 10:06:47 UTC
(In reply to Phil Ringnalda from comment #5)
> At the time I wrote it, this seemed like the simplest possible
> SO+PQA+backport patch to keep us from wasting a third of a sheet of paper
> any time someone prints a report, and then I was going to reopen cait's bug
> 29979 to deal with the problem of .navbar replacing, um, probably several of
> the selectors in this rule, I'm not sure exactly which, and adding in one
> for anything new that's not in .navbar.
> 
> I'm going to be gone until August 1st, and I've already solved the problem
> for myself in IntranetUserCSS, so if someone wants to take this bug and sort
> through what selectors have completely disappeared, not just disappeared
> from reports, and replace them with the new thing, and also fix !important
> in the same patch, feel free.

Can you share your code? Also please don't reopen, almost always better to have a new bug.
Comment 8 Phil Ringnalda 2023-07-07 15:06:02 UTC
I copy-pasted this rule, added !important, and added the selector nav. Do we use a <nav> somewhere that it should be printed? Don't know, didn't test or grep.
Comment 9 Katrin Fischer 2023-08-11 10:07:59 UTC
Better to avoid "In Discussion" unless there is an argument that needs more people to be resolved.
Comment 10 Owen Leonard 2023-10-10 15:00:30 UTC
*** Bug 35016 has been marked as a duplicate of this bug. ***
Comment 11 Owen Leonard 2023-10-10 15:27:58 UTC
Created attachment 156798 [details] [review]
Bug 34119: (Preliminary) Run stylelint fixes on print.css

This patch corrects errors raised by the new stylelint configuration in
print.css. Most of the fixes were made automatically by using the
"--fix" option:

npx stylelint koha-tmpl/intranet-tmpl/prog/css/print.css --fix

Some examples of errors fixed automatically:

- Expected indentation of 4 spaces
- Unexpected whitespace before ":"
- Unexpected unit
- Expected single space before "{"
- Expected newline after ";" in a multi-line declaration block

Errors from use of "!important" have been ignored.
Comment 12 Owen Leonard 2023-10-10 15:28:00 UTC
Created attachment 156799 [details] [review]
Bug 34119: Improve staff interface print stylesheet following redesign

This patch makes some updates to the staff interface print stylesheet in
order to make it work better following the staff interface redesign.

To test, apply the patch and clear your browser cache if necessary.

In the staff client, go to various pages and view the print preview. The
print view should exclude the header navigation, header search, page
toolbars and page sidebars.

- Pages with tabbed displays should show content from all tabs: e.g.
  bibliographic details, MARC editor.
- In Reports, check that report results print well.
- Pages with DataTables should print with table information visible but
  table controls hidden.
Comment 13 Marcel de Rooy 2023-10-11 10:47:46 UTC
Created attachment 156813 [details] [review]
Bug 34119: (Preliminary) Run stylelint fixes on print.css

This patch corrects errors raised by the new stylelint configuration in
print.css. Most of the fixes were made automatically by using the
"--fix" option:

npx stylelint koha-tmpl/intranet-tmpl/prog/css/print.css --fix

Some examples of errors fixed automatically:

- Expected indentation of 4 spaces
- Unexpected whitespace before ":"
- Unexpected unit
- Expected single space before "{"
- Expected newline after ";" in a multi-line declaration block

Errors from use of "!important" have been ignored.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2023-10-11 10:47:48 UTC
Created attachment 156814 [details] [review]
Bug 34119: Improve staff interface print stylesheet following redesign

This patch makes some updates to the staff interface print stylesheet in
order to make it work better following the staff interface redesign.

To test, apply the patch and clear your browser cache if necessary.

In the staff client, go to various pages and view the print preview. The
print view should exclude the header navigation, header search, page
toolbars and page sidebars.

- Pages with tabbed displays should show content from all tabs: e.g.
  bibliographic details, MARC editor.
- In Reports, check that report results print well.
- Pages with DataTables should print with table information visible but
  table controls hidden.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Victor Grousset/tuxayo 2023-10-13 05:50:47 UTC
> - Pages with DataTables should print with table information visible but
>  table controls hidden.

- tested patron page with a few checkouts displayed
- tested order basket
- tested patron search results


---

Are those undesirable table controls?:
https://pic.infini.fr/wk6zBOFT/RP5P0YA0.webp

---

minor findings:
- on record detail page, should the content of the descriptions tab be after the list of items? And same for acquisition details. No reliable idea on what is more relevant when printing, mentioning this just in case.
Comment 16 Katrin Fischer 2023-10-14 17:07:11 UTC
Created attachment 157160 [details] [review]
Bug 34119: (Preliminary) Run stylelint fixes on print.css

This patch corrects errors raised by the new stylelint configuration in
print.css. Most of the fixes were made automatically by using the
"--fix" option:

npx stylelint koha-tmpl/intranet-tmpl/prog/css/print.css --fix

Some examples of errors fixed automatically:

- Expected indentation of 4 spaces
- Unexpected whitespace before ":"
- Unexpected unit
- Expected single space before "{"
- Expected newline after ";" in a multi-line declaration block

Errors from use of "!important" have been ignored.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2023-10-14 17:07:14 UTC
Created attachment 157161 [details] [review]
Bug 34119: Improve staff interface print stylesheet following redesign

This patch makes some updates to the staff interface print stylesheet in
order to make it work better following the staff interface redesign.

To test, apply the patch and clear your browser cache if necessary.

In the staff client, go to various pages and view the print preview. The
print view should exclude the header navigation, header search, page
toolbars and page sidebars.

- Pages with tabbed displays should show content from all tabs: e.g.
  bibliographic details, MARC editor.
- In Reports, check that report results print well.
- Pages with DataTables should print with table information visible but
  table controls hidden.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Katrin Fischer 2023-10-14 17:10:52 UTC
(In reply to Victor Grousset/tuxayo from comment #15)
> > - Pages with DataTables should print with table information visible but
> >  table controls hidden.
> 
> - tested patron page with a few checkouts displayed
> - tested order basket
> - tested patron search results
> 
> 
> ---
> 
> Are those undesirable table controls?:
> https://pic.infini.fr/wk6zBOFT/RP5P0YA0.webp
> 
> ---

I think removing them would be nice, but is probably a page specific change whie this takes care of the general elements. I think maybe we could take a closer look at other pages separately. Also: patron search results has a export > print option on the table that gives nice results :)


> minor findings:
> - on record detail page, should the content of the descriptions tab be after
> the list of items? And same for acquisition details. No reliable idea on
> what is more relevant when printing, mentioning this just in case.

They both appear after the item list for me. And it doesn't matter which tab is active before switching to print layout.
Comment 19 Victor Grousset/tuxayo 2023-10-14 19:39:44 UTC
(In reply to Katrin Fischer from comment #18)
> I think removing them would be nice, but is probably a page specific change
> whie this takes care of the general elements. I think maybe we could take a
> closer look at other pages separately. Also: patron search results has a
> export > print option on the table that gives nice results :)

Alright, let's go with the current proposal then! :)

> > minor findings:
> > - on record detail page, should the content of the descriptions tab be after
> > the list of items? And same for acquisition details. No reliable idea on
> > what is more relevant when printing, mentioning this just in case.
> 
> They both appear after the item list for me. And it doesn't matter which tab
> is active before switching to print layout.

My phrasing was weird. I meant that they are indeed after the item list, but should they?
(It sounded that like I was suggesting that they should be after. But it's already the case)
Comment 20 Katrin Fischer 2023-10-14 21:31:04 UTC
(In reply to Victor Grousset/tuxayo from comment #19)
> (In reply to Katrin Fischer from comment #18)
> > I think removing them would be nice, but is probably a page specific change
> > whie this takes care of the general elements. I think maybe we could take a
> > closer look at other pages separately. Also: patron search results has a
> > export > print option on the table that gives nice results :)
> 
> Alright, let's go with the current proposal then! :)
> 
> > > minor findings:
> > > - on record detail page, should the content of the descriptions tab be after
> > > the list of items? And same for acquisition details. No reliable idea on
> > > what is more relevant when printing, mentioning this just in case.
> > 
> > They both appear after the item list for me. And it doesn't matter which tab
> > is active before switching to print layout.
> 
> My phrasing was weird. I meant that they are indeed after the item list, but
> should they?
> (It sounded that like I was suggesting that they should be after. But it's
> already the case)

That's the sequence in the source code, I think it's correct.
Comment 21 Tomás Cohen Arazi 2023-10-18 13:23:01 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 22 Fridolin Somers 2023-10-25 07:26:58 UTC
Pushed to 23.05.x for 23.05.05
Comment 23 Matt Blenkinsop 2023-11-13 13:45:59 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x