I have made modifications to the template file. It added a text link to remove the SQL statement from the print view. Thus saving the need to use CSV/TSV exports. The print text will not show on the print out as is. I added a little .noprint class to the template file. Then I added my text with a print dialog and class change JS. Please see: http://pastebin.com/LWR3BqiW for the changes specifically lines 22-26 & line 501
Created attachment 8025 [details] [review] Modified the template to have a JS link to change the print media class and launch a print dialog. You can disregard the pastebin post, this patch replaces it.
When I apply this patch and attempt to test it, the guided reports page (/cgi-bin/koha/reports/guided_reports.pl?phase=Use%20saved) has no content in the <body> tag.
i see my mistake i will correct and re-submit. Thanks!
Created attachment 8106 [details] [review] Added missing </style> First time rebasing, so I am hoping this merged(squashed) correctly. Setting to In Discussion until I cna see the "diff"
Created attachment 8108 [details] [review] should be correct this time...I hope... Failed on the previous git squash, hoping I did it right now.
Created attachment 8109 [details] [review] complete <style></style> tag this time.. I think I needed to re-make the patch file after the rebase...crosses fingers.
I tested it on the sandbox http://pro.test9.biblibre.com/ the link appeared on the top an SQL generated report, it launched my print box, and when I previewed the print out it did not have the SQL visable.
Dropping back to Needs Signoff, since signoff by someone other than the author is required. This looks to be a pretty easy one to test, but I'm about to switch trains, so I'll leave it up to another community member. Should pass QA quickly once signed off.
Created attachment 8273 [details] [review] Bug 7646: Add "do not print SQL" link to reports I have made modifications to the template file. It added a text link to remove the SQL statement from the print view. Thus saving the need to use CSV/TSV exports. The print text will not show on the print out as is. I added a little .noprint class to the template file. Then I added my text with a print dialog and class change JS. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Easy to read template only change. Marking Passed QA
QA comment: All pages include a <link rel="stylesheet" type="text/css" media="print" href="/intranet-tmpl/prog/en/css/print.css" /> for printing stylesheet that already has a .noprint class: div#breadcrumbs, div#toplevelnav, div#login, div#header_search, div#toolbar, div#changelanguage, div#menu, div.yui-b, .noprint, form#sortbyform, #cartDetails, fieldset.action, .list-actions, .print { display: none; } So this code: +<style type="text/css" media="print"> +/* ### FIX ME, PUT ME IN MAIN CSS IF USED OFTEN! */ +.noprint { +display: none; +} +</style> in your patch is useless please resubmit with this duplicate class fixed
Created attachment 37779 [details] [review] Bug 7646: Add "do not print SQL" link to reports I redid the patch without the superfluos CSS part for class noprint. To test: Go to Home > Reports > Guided reports wizard > Saved reports Select a report and run it You will find a new link "Print w/o SQL" above the title Hit this link. The report (this page) will print without the SQL statement. The original patch was QAed in 2012, I think it is better to have put it back to "Needs Sign-off"
Patch applied cleanly but can't seem to see the print link? Setting as Failed QA
Created attachment 37790 [details] Screenshot showing position of link after applying patch I re-tested, the link appears as intended by the original patch. See screenshot. Setting back to "Needs signoff".
Created attachment 37926 [details] [review] Bug 7646 - Printing w/o the SQL Code showing This patch provides the same function but adds a 'Print results' button rather than the link. Looks a little cleaner to my eye and there doesn't seem to be an option to print with SQL. To test: Go to Home > Reports > Guided reports wizard > Saved reports Select a report and run it You will find a new link "Print w/o SQL" above the title Hit this link. The report (this page) will print without the SQL statement.
I tested, but the link looked very odd to me, added a patch to put a 'print results' button with the other action buttons. I did not obsolete the old patch so anyone can test both and see which they prefer
Created attachment 37945 [details] [review] [Signed-off] Bug 7646 - Printing w/o the SQL Code showing This patch provides the same function but adds a 'Print results' button rather than the link. Looks a little cleaner to my eye and there doesn't seem to be an option to print with SQL. To test: Go to Home > Reports > Guided reports wizard > Saved reports Select a report and run it You will find a new link "Print w/o SQL" above the title Hit this link. The report (this page) will print without the SQL statement. Works as expected. Print function is displayed as Button. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 38043 [details] [review] Bug 7646 [QA Followup] - Tweak button to match other Koha print buttons
A few issues: 1) Javascript embedded in html. Javascript should be moved outside the html, include the template, and include, or a js file, whichever is best suited. 2) The noprint class is added when clicking the print button, but not removed afterward 3) Only the visible results are printed, but to me the print button implies that it will print all results. If you don't want to go this far, add a tooltip to the print button so it shows "Print visible results" when you hover over it.
Created attachment 41093 [details] screenshot-applied-patch I printed to PDF and under the report name i see the comment, the message »Total number of rows matching the (unlimited) query is 1.« and »Rows per page« including the dropdown menu and the »Update« button. Is that expected? I attached a screenshot. I agree with Kyle that I'd expect all results to be printed, not only the first page.
I just tested the print layout on master, I think the only bit to remove now is the 'rows per page'. I've filed bug 20679 for this.