Created attachment 61175 [details] [review] Bug 18283 - Display improvements on report results - hide code and change wording This patch makes small display improvements to the report results page. -Add toggle link for SQL code -Change results count wording -Add 'Notes:' label report notes To test: 1 - Apply patch 2 - Run some reports 3 - Check that results page makes sense 4 - Hide and display code and ensure toggle works
Works great. I would change it to a button and put it with the rest of the buttons at the top of the report. My 2 cents.
Created attachment 61192 [details] [review] Bug 18283 - Display improvements on report results - hide code and change wording This patch makes small display improvements to the report results page. -Add toggle link for SQL code -Change results count wording -Add 'Notes:' label report notes To test: 1 - Apply patch 2 - Run some reports 3 - Check that results page makes sense 4 - Hide and display code and ensure toggle works
I can not toggle code display. It seems that something went lost from previous patch: <a class="toggle_sql" id="toggle_sql_hid" href="#">Show sql code</a> <a class="toggle_sql" id="toggle_sql_vis" href="#" style="display:none;">Hide sql code</a>
Created attachment 61207 [details] [review] Bug 18283 - Display improvements on report results - hide code and change wording This patch makes small display improvements to the report results page. -Add toggle link for SQL code -Change results count wording -Add 'Notes:' label report notes To test: 1 - Apply patch 2 - Run some reports 3 - Check that results page makes sense 4 - Hide and display code and ensure toggle works
Works great. Just need to add space between buttons. The picky part of me wants you to place the button between the New report and Edit buttons. Makes more sense to be there. But the space is a must.
Created attachment 61249 [details] [review] Bug 18283 - Display improvements on report results - hide code and change wording This patch makes small display improvements to the report results page. -Add toggle link for SQL code -Change results count wording -Add 'Notes:' label report notes To test: 1 - Apply patch 2 - Run some reports 3 - Check that results page makes sense 4 - Hide and display code and ensure toggle works
Created attachment 61253 [details] [review] Bug 18283 - Display improvements on report results - hide code and change wording This patch makes small display improvements to the report results page. -Add toggle link for SQL code -Change results count wording -Add 'Notes:' label report notes To test: 1 - Apply patch 2 - Run some reports 3 - Check that results page makes sense 4 - Hide and display code and ensure toggle works Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 61318 [details] [review] [SIGNED OFF] Bug 18283 - Display improvements on report results - hide code and change wording This patch makes small display improvements to the report results page. -Add toggle link for SQL code -Change results count wording -Add 'Notes:' label report notes To test: 1 - Apply patch 2 - Run some reports 3 - Check that results page makes sense 4 - Hide and display code and ensure toggle works Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Why is this supposed to be an improvement? What are the use cases?
(In reply to Jonathan Druart from comment #10) > Why is this supposed to be an improvement? What are the use cases? I can tell you that at our library, we have jquery setup that does something similar. It bugged me the moment we started using Koha that the SQL was showing. Your average staff person doesn't care about it, and depending on the size of your sql, you might have to scroll down a bit to see results. It's ugly. It makes sense to hide it, but be able to reveal it when needed.
Created attachment 61359 [details] [review] Bug 18283 - Display improvements on report results - hide code and change wording This patch makes small display improvements to the report results page. -Add toggle link for SQL code -Change results count wording -Add 'Notes:' label report notes To test: 1 - Apply patch 2 - Run some reports 3 - Check that results page makes sense 4 - Hide and display code and ensure toggle works Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 61360 [details] [review] Bug 18283: 'sql' should be 'SQL' Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 17.05, thanks Nick, Jonathan!
This won't get ported back to 16.11.x as it is an enhancement.