We have duplicate JS for the cashup summary modals and this leads to double print dialogues.
Created attachment 136488 [details] [review] Bug 31039: Remove duplicate modal printing JS This patch removes the erroneos duplication of the modal printing js. Test plan 1) Create some transactions and perform a cashup. 2) Open the cashup summary modal 3) Click 'Print' 4) Cancel the print dialogue 5) Note that the dialogue re-appears 6) Apply the patch 7) Repeat and note the dialogue closes first time now.
I attempted to test, but I am not seeing this problem: 1. Print 2. Opens print dialogue 3. Cancel 4. Goes back to cashup summary modal Tested using Ubuntu 18.04 with Firefox and Google Chrome.
Weird.. it was consistent for Firefox and Chrome for me.. even got Owen to test it on IRC before I reported the bug and he agreed. Hmm.. wonder what's going on...
If it's any consolation, I didn't notice anything different after applying the patch. I use Firefox Developer Edition for testing (with the history/caching deleted each time it starts up). Maybe it only shows up on other operating systems?
Created attachment 136532 [details] [review] Bug 31039: Remove duplicate modal printing JS This patch removes the erroneos duplication of the modal printing js. Test plan 1) Create some transactions and perform a cashup. 2) Open the cashup summary modal 3) Click 'Print' 4) Cancel the print dialogue 5) Note that the dialogue re-appears 6) Apply the patch 7) Repeat and note the dialogue closes first time now. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 136533 [details] [review] Bug 31039: (follow-up) Wrap jQuery in $(document).ready() cashup_modal.js consists only of jQuery code, so the whole thing should be contained in a $(document).ready() function. This may or may not be contributing to the behavior this bug is trying to fix. Please note that this patch contains whitespace changes, so diff accordingly.
Created attachment 137046 [details] [review] Bug 31039: (follow-up) Wrap jQuery in $(document).ready() cashup_modal.js consists only of jQuery code, so the whole thing should be contained in a $(document).ready() function. This may or may not be contributing to the behavior this bug is trying to fix. Please note that this patch contains whitespace changes, so diff accordingly. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I couldn't replicate the doubled up printing dialogues, but could also not find any regression and the changes look good.
Argument "" isn't numeric in multiplication (*) at /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line 86. Argument "" isn't numeric in multiplication (*) at /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line 86. Argument "" isn't numeric in multiplication (*) at /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line 87. Argument "" isn't numeric in multiplication (*) at /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line 87. Argument "" isn't numeric in multiplication (*) at /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line 88. Argument "" isn't numeric in multiplication (*) at /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line 158. Argument "" isn't numeric in multiplication (*) at /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line 288.
Created attachment 137367 [details] [review] Bug 31039: Remove duplicate modal printing JS This patch removes the erroneos duplication of the modal printing js. Test plan 1) Create some transactions and perform a cashup. 2) Open the cashup summary modal 3) Click 'Print' 4) Cancel the print dialogue 5) Note that the dialogue re-appears 6) Apply the patch 7) Repeat and note the dialogue closes first time now. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 137368 [details] [review] Bug 31039: (follow-up) Wrap jQuery in $(document).ready() cashup_modal.js consists only of jQuery code, so the whole thing should be contained in a $(document).ready() function. This may or may not be contributing to the behavior this bug is trying to fix. Please note that this patch contains whitespace changes, so diff accordingly. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Sorry, please rebase
(In reply to Marcel de Rooy from comment #9) > Argument "" isn't numeric in multiplication (*) at > /usr/share/koha/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt line > 86. ... What is the purpose of this comment?
I'll rebase on Monday, and take a look at the highlighted warnings at the same time.
(In reply to Martin Renvoize from comment #14) > I'll rebase on Monday, and take a look at the highlighted warnings at the > same time. I filed the warnings already when looking at one of the other cash register patches: Bug 31125 - Fix warnings from register.tt
Created attachment 137514 [details] [review] Bug 31039: Remove duplicate modal printing JS This patch removes the erroneos duplication of the modal printing js. Test plan 1) Create some transactions and perform a cashup. 2) Open the cashup summary modal 3) Click 'Print' 4) Cancel the print dialogue 5) Note that the dialogue re-appears 6) Apply the patch 7) Repeat and note the dialogue closes first time now. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 137515 [details] [review] Bug 31039: (follow-up) Wrap jQuery in $(document).ready() cashup_modal.js consists only of jQuery code, so the whole thing should be contained in a $(document).ready() function. This may or may not be contributing to the behavior this bug is trying to fix. Please note that this patch contains whitespace changes, so diff accordingly. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Rebased
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.05
thx pushed to 21.11.x for 21.11.12
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved.