I see areas in self-checkout which break if JavaScript is turned off. I think an argument could be made that self-checkout can require JavaScript because the library has control over the machine it is running on. However, the problematic JavaScript dependencies I'm seeing shouldn't really require JS so I think a change is warranted.
Created attachment 13022 [details] [review] Bug 8955 - Self checkout should degrade gracefully with JavaScript turned off Several changes in this patch, the largest of which is that the renew/return dialogs no longer require JavaScript in order to properly alter and submit the form. Instead each button uses a separate form. To test, log in to self checkout and submit a barcode which is checked out and can be renewd, and a barcode which is checked out but has reached the checkout limit. On the resulting dialogs each button should function properly. Also changed: I removed some useless JavaScript processing related to a bogus "valid_session" variable which was unused. Similarly removed is template logic based on a "timedout" variable which was not set by the script. Note that the script contains NO server- side handling of timeout. Timeout is dependent on JavaScript. To test these changes, confirm that with JavaScript enabled you are automatically logged out after the time specified in the SelfCheckTimeout preference. Other minor changes: Terminology and capitalization corrections, minor style tweaks.
String changes. Will hold for post-3.10.0.
Created attachment 13231 [details] [review] [SIGNED-OFF] Bug 8955 - Self checkout should degrade gracefully with JavaScript turned off Several changes in this patch, the largest of which is that the renew/return dialogs no longer require JavaScript in order to properly alter and submit the form. Instead each button uses a separate form. To test, log in to self checkout and submit a barcode which is checked out and can be renewd, and a barcode which is checked out but has reached the checkout limit. On the resulting dialogs each button should function properly. Also changed: I removed some useless JavaScript processing related to a bogus "valid_session" variable which was unused. Similarly removed is template logic based on a "timedout" variable which was not set by the script. Note that the script contains NO server- side handling of timeout. Timeout is dependent on JavaScript. To test these changes, confirm that with JavaScript enabled you are automatically logged out after the time specified in the SelfCheckTimeout preference. Other minor changes: Terminology and capitalization corrections, minor style tweaks. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Renew and cancel buttons work with javascript disabled when scanning the barcode to renew. The renewal button on the main page does not work with javascript disabled. The renewal and checkout limits do not have any 'back' buttons, but they don't in master either. The timeout still works with javascript enabled. I think this is passing based on the test plan provided.
(In reply to comment #4) > The renewal button on the main page does not work with > javascript disabled. I missed that one. So my patch helps but more work is needed to be able to close this bug.
(In reply to comment #5) > (In reply to comment #4) > > The renewal button on the main page does not work with > > javascript disabled. > > I missed that one. So my patch helps but more work is needed to be able to > close this bug. Yes, it does help. You can still scan a barcode to renew, so the ability to renew isn't completely lost without javascript, but the button should at least be hidden if it won't function with javascript disabled.
Created attachment 13268 [details] [review] Bug 8955 [Follow-up] Self checkout should degrade gracefully with JavaScript turned off This follow-up fixes some instance of unnecessary JavaScript which I missed the first time around: The renew and check-in buttons on the list of checkouts. The functionality can be easily accomplished without JS. Also corrected: - Capitalization error - Miss-named variable causing the "too many checkouts" not to be displayed - Misplaced [% END %] tags hidding "Return to account" button on some dialogs. - Corrected variable scope error which prevented the "return" button from appearing. To test, log into self checkout using an account which has checkouts. Testing renewing items both by scanning barcodes and by clicking the "renew" button in the list of checkouts. Renew something until it reaches its limit and test checking it out by scanning the barcode. Test the "check in" button.
QA comment: * passes koha-qa.pl * I can't see a difference between before and after when I'm js=ON (as expected) * I see that the follow-up has not been signed-off, so back to "need signoff". Kyle can you test this follow-up please ? I'll pass QA once you've done. PS: My feeling is that it's a (minor) enhancement, not a "normal" bugfix. Your opinion ? PS2: on my setup, I don't see the koha logo in "Powered by" (there's nothing) Is it because I'm using the -recent- ccsr theme ? what does the sco-main page look like for you ?
> PS2: on my setup, I don't see the koha logo in "Powered by" (there's > nothing) Is it because I'm using the -recent- ccsr theme ? Yes, the CCSR theme includes lots of errors like this in features which I assume were not deemed necessary by the original sponsors.
Created attachment 13962 [details] [review] Bug 8955 - Self checkout should degrade gracefully with JavaScript turned off Several changes in this patch, the largest of which is that the renew/return dialogs no longer require JavaScript in order to properly alter and submit the form. Instead each button uses a separate form. To test, log in to self checkout and submit a barcode which is checked out and can be renewd, and a barcode which is checked out but has reached the checkout limit. On the resulting dialogs each button should function properly. Also changed: I removed some useless JavaScript processing related to a bogus "valid_session" variable which was unused. Similarly removed is template logic based on a "timedout" variable which was not set by the script. Note that the script contains NO server- side handling of timeout. Timeout is dependent on JavaScript. To test these changes, confirm that with JavaScript enabled you are automatically logged out after the time specified in the SelfCheckTimeout preference. Other minor changes: Terminology and capitalization corrections, minor style tweaks. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 13963 [details] [review] Bug 8955 [Follow-up] Self checkout should degrade gracefully w/o JavaScript This follow-up fixes some instance of unnecessary JavaScript which I missed the first time around: The renew and check-in buttons on the list of checkouts. The functionality can be easily accomplished without JS. Also corrected: - Capitalization error - Miss-named variable causing the "too many checkouts" not to be displayed - Misplaced [% END %] tags hidding "Return to account" button on some dialogs. - Corrected variable scope error which prevented the "return" button from appearing. To test, log into self checkout using an account which has checkouts. Testing renewing items both by scanning barcodes and by clicking the "renew" button in the list of checkouts. Renew something until it reaches its limit and test checking it out by scanning the barcode. Test the "check in" button. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
(In reply to comment #8) > QA comment: > * passes koha-qa.pl > * I can't see a difference between before and after when I'm js=ON (as > expected) > * I see that the follow-up has not been signed-off, so back to "need > signoff". Kyle can you test this follow-up please ? I'll pass QA once you've > done. Thanks chris for the signoff on the follow-up passed QA
Created attachment 14247 [details] [review] Bug 8955 - Self checkout should degrade gracefully with JavaScript turned off Several changes in this patch, the largest of which is that the renew/return dialogs no longer require JavaScript in order to properly alter and submit the form. Instead each button uses a separate form. To test, log in to self checkout and submit a barcode which is checked out and can be renewd, and a barcode which is checked out but has reached the checkout limit. On the resulting dialogs each button should function properly. Also changed: I removed some useless JavaScript processing related to a bogus "valid_session" variable which was unused. Similarly removed is template logic based on a "timedout" variable which was not set by the script. Note that the script contains NO server- side handling of timeout. Timeout is dependent on JavaScript. To test these changes, confirm that with JavaScript enabled you are automatically logged out after the time specified in the SelfCheckTimeout preference. Other minor changes: Terminology and capitalization corrections, minor style tweaks. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 14248 [details] [review] Bug 8955 [Follow-up] Self checkout should degrade gracefully w/o JavaScript This follow-up fixes some instance of unnecessary JavaScript which I missed the first time around: The renew and check-in buttons on the list of checkouts. The functionality can be easily accomplished without JS. Also corrected: - Capitalization error - Miss-named variable causing the "too many checkouts" not to be displayed - Misplaced [% END %] tags hidding "Return to account" button on some dialogs. - Corrected variable scope error which prevented the "return" button from appearing. To test, log into self checkout using an account which has checkouts. Testing renewing items both by scanning barcodes and by clicking the "renew" button in the list of checkouts. Renew something until it reaches its limit and test checking it out by scanning the barcode. Test the "check in" button. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
This patch has been pushed to master.