When printing the cashup summary modal, if the content is long enough to make the modal scrollable then the print cuts off/duplicates some of the details over the second page.
I was about to file a bug when I found this one. I have a similar, but not exactly the same, problem. We are noticing that the Cashup summary can be duplicated when the register.pl page prints longer than two pages. From a stackoverflow post: "The problem is that the content underlying the modal determines how many pages are printed and if you are trying to print the content of the modal it will print duplicates equaling the number pages in the underlying document" https://stackoverflow.com/questions/68206205/same-page-prints-twice-when-using-a-bootstrap-full-screen-modal So, to recreate: 1. Have POS on. 2. Have some registers and items for sale. Make some sales to 'Cashup'. 3. If you have enough enough lines in the table so that 'register.pl' would print on 2 or more pages, the summary will be duplicated that same number of time. If you are lazy like me and don't want to make that many transaction you can also add some content to the page like this: for (let i = 0; i < 100; i++) { $('#register').append('<h1>TEST</h1>'); }
Created attachment 158283 [details] [review] Bug 31041: Prevent duplicate display of cashup summary when printing To test: 1. Have POS on. 2. Have some registers and items for sale. Make some sales to 'Cashup'. 3. If you have enough enough lines in the table so that 'register.pl' would print on 2 or more pages, the summary will be duplicated that same number of time. If you are lazy like me and don't want to make that many transaction you can also add some content to the page like this: for (let i = 0; i < 100; i++) { $('#register').append('<h1>TEST</h1>'); } 4. APPLY PATCH, clear browser cahche 5. Try printing again, you should only get one cashup summary when printing.
CC'ing Owen here.
Created attachment 158342 [details] [review] Bug 31041: Prevent duplicate display of cashup summary when printing To test: 1. Have POS on. 2. Have some registers and items for sale. Make some sales to 'Cashup'. 3. If you have enough enough lines in the table so that 'register.pl' would print on 2 or more pages, the summary will be duplicated that same number of time. If you are lazy like me and don't want to make that many transaction you can also add some content to the page like this: for (let i = 0; i < 100; i++) { $('#register').append('<h1>TEST</h1>'); } 4. APPLY PATCH, clear browser cahche 5. Try printing again, you should only get one cashup summary when printing. Signed-off-by: Juliet Heltibridle <jheltibridle@rcplib.org>
I am having some trouble testing here, even with the "cheat" - can you please add some more instructions on how to test and how to apply the JS?
(In reply to Katrin Fischer from comment #5) > I am having some trouble testing here, even with the "cheat" - can you > please add some more instructions on how to test and how to apply the JS? Katrin, are you making sure you are testing this on the register.pl page and not the registers.pl page? The registers.pl page works correctly. To test: 1. Have POS on. 2. Have some registers and items for sale. Make some sales to 'Cashup'. 2b. Record a Cashup 2c. Go to register.pl page (transaction history page) and attempt to print the Cashup summary. 3. If you have enough enough lines in the table so that 'register.pl' would print on 2 or more pages, the summary will be duplicated that same number of time.
This causes the print formatting to change a bit.. I'm not entirely against that, but it is a change. I need to think a bit about how this is implemented. The 'modalprinter' patturn is in use for the bundle confirmation modal as well as this one.. so it would be good to have a standard include for the JS that can be re-used for other printable modals. The basic premise here is.. Open the content of the modal in a new window and then print that.. as apposed the existing "hide everything that's not the modal" method that was in place prior to here.. we could certainly do some cleanup if we decide the 'open new window' approach is prefereable. I think I tried and failed to get the 'hide content' version to work as a fix here in the past so maybe the open window is the best way.
Thx for the help, Juliet :) Since Martin knows this code really well (having written it), I'll leave QA to him.
Created attachment 158589 [details] [review] Bug 31041: Prevent duplicate display of cashup summary when printing To test: 1. Have POS on. 2. Have some registers and items for sale. Make some sales to 'Cashup'. 3. If you have enough enough lines in the table so that 'register.pl' would print on 2 or more pages, the summary will be duplicated that same number of time. If you are lazy like me and don't want to make that many transaction you can also add some content to the page like this: for (let i = 0; i < 100; i++) { $('#register').append('<h1>TEST</h1>'); } 4. APPLY PATCH, clear browser cahche 5. Try printing again, you should only get one cashup summary when printing. Signed-off-by: Juliet Heltibridle <jheltibridle@rcplib.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 158590 [details] [review] Bug 31041: (follow-up) Clean up and generalise This patch cleans out the print media css rules that were originally added for modalprinting. The window.open method is simpler to maintain and will more reliably print modal content as expected. We factor out the printer code into it's own JS asset that we can apply to other printable modals and then use it in the two existing places where such modals have been defined already (cashup summary and bundle confirmation). Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 158591 [details] [review] Bug 31041: DO NOT PUSH - Testing Only This patch adds a script to allow easy population of a cashup using KTD. To use it 1) Start up a KTD 2) Enable UseCashRegisters system preference 3) Add a cash register with the name 'TEST' 4) Run this script from the command line 5) Navigate to the register you created to look at the cashups 6) Click on the summary link and print at the bottom of the modal and confirm that the modal prints as expected accross multiple pages without duplications or data lose. 7) Back on the register page, select to view the past months 'Older transactions'. This will lengthen the page significantly 8) Click on the summary link again and print.. Confirm that again the correct number of pages is printed and contain the correct content. This tests both scenarios.. where the background page spans multiple pages and where the modal content requires a scroll.
Created attachment 158592 [details] [review] Bug 31041: DO NOT PUSH - Testing Only This patch adds a script to allow easy population of a cashup using KTD. To use it 1) Start up a KTD 2) Enable UseCashRegisters system preference 3) Add a cash register with the name 'TEST' 4) Run this script from the command line 5) Navigate to the register you created to look at the cashups 6) Click on the summary link and print at the bottom of the modal and confirm that the modal prints as expected accross multiple pages without duplications or data lose. 7) Back on the register page, select to view the past months 'Older transactions'. This will lengthen the page significantly 8) Click on the summary link again and print.. Confirm that again the correct number of pages is printed and contain the correct content. This tests both scenarios.. where the background page spans multiple pages and where the modal content requires a scroll.
All working well, QA scripts happy. Passing QA
Thanks Martin! This all looks pretty good. However, there is one link in the #registers table ( in the Last cashup column) where the modal will pop-up but when I click 'Print' nothing happens.
Created attachment 158636 [details] [review] Bug 31041: add modal_printer.js asset to registers.tt
Cheers for taking care of the missing inc Lucas, I fell asleep last night before getting to it. :-)
Created attachment 158644 [details] [review] Bug 31041: Prevent duplicate display of cashup summary when printing To test: 1. Have POS on. 2. Have some registers and items for sale. Make some sales to 'Cashup'. 3. If you have enough enough lines in the table so that 'register.pl' would print on 2 or more pages, the summary will be duplicated that same number of time. If you are lazy like me and don't want to make that many transaction you can also add some content to the page like this: for (let i = 0; i < 100; i++) { $('#register').append('<h1>TEST</h1>'); } 4. APPLY PATCH, clear browser cahche 5. Try printing again, you should only get one cashup summary when printing. Signed-off-by: Juliet Heltibridle <jheltibridle@rcplib.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 158645 [details] [review] Bug 31041: (follow-up) Clean up and generalise This patch cleans out the print media css rules that were originally added for modalprinting. The window.open method is simpler to maintain and will more reliably print modal content as expected. We factor out the printer code into it's own JS asset that we can apply to other printable modals and then use it in the two existing places where such modals have been defined already (cashup summary and bundle confirmation). Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 158646 [details] [review] Bug 31041: Add modal_printer.js asset to registers.tt Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 158647 [details] [review] Bug 31041: DO NOT PUSH - Testing Only This patch adds a script to allow easy population of a cashup using KTD. To use it 1) Start up a KTD 2) Enable UseCashRegisters system preference 3) Add a cash register with the name 'TEST' 4) Run this script from the command line 5) Navigate to the register you created to look at the cashups 6) Click on the summary link and print at the bottom of the modal and confirm that the modal prints as expected accross multiple pages without duplications or data lose. 7) Back on the register page, select to view the past months 'Older transactions'. This will lengthen the page significantly 8) Click on the summary link again and print.. Confirm that again the correct number of pages is printed and contain the correct content. This tests both scenarios.. where the background page spans multiple pages and where the modal content requires a scroll.
Tested the follow-up and setting to PQA :)
Pushed to master for 23.11. Nice work everyone, thanks!
It breaks the check-in pop notifying of holds! :o commit ef880e349f959686a830bda67ec5a295e3d76eb4 Bug 31041: (follow-up) Clean up and generalise Opened bug 35295
Pushed to 23.05.x for 23.05.06
(In reply to Fridolin Somers from comment #24) > Pushed to 23.05.x for 23.05.06 This one cause a problem - you will need bug 35295 as well!
Nice work everyone! Pushed to 22.11.x for next release
(In reply to Katrin Fischer from comment #25) > (In reply to Fridolin Somers from comment #24) > > Pushed to 23.05.x for 23.05.06 > > This one cause a problem - you will need bug 35295 as well! Also backported