Right now, the system preferences OPACUserCSS and opacuserjs affect the SCO system. Some libraries would like to add css/js that specifically for the SCO module, but it would negatively impact the OPAC.
Created attachment 13251 [details] [review] Bug 9009 - Add separate user js and css of SCO module
Created attachment 13252 [details] [review] Bug 9009 - Add separate user js and css of SCO module
Created attachment 13253 [details] [review] Bug 9009 - Add separate user js and css of SCO module
Created attachment 13254 [details] [review] Bug 9009 - Add separate user js and css of SCO module
Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Add something arbitrary to the new sysprefs SCOUserCSS and SCOUserJS ( such as TestCSS and TestJS ). 4) Load the SCO module in a browser, and view the HTML, verify the CSS and JS values in the system preferences have been included.
Created attachment 13255 [details] [review] Bug 9009 - Add separate user js and css for SCO module
Small but useful enhancement! However, patch does not apply, sorry: Applying: Bug 9009 - Add separate user js and css for SCO module error: patch failed: installer/data/mysql/sysprefs.sql:386 error: installer/data/mysql/sysprefs.sql: patch does not apply error: patch failed: installer/data/mysql/updatedatabase.pl:6020 error: installer/data/mysql/updatedatabase.pl: patch does not apply error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:454 error: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref: patch does not apply
Created attachment 13871 [details] [review] Bug 9009 - Add separate user js and css for SCO module
Created attachment 13889 [details] [review] [SIGNED-OFF] Bug 9009 - Add separate user js and css for SCO module Signed-off-by: Marc Veron <veron@veron.ch>
Addendum: I tested with an alert('hello') as JavaScript and some background-color for the CSS. Worked as expected.
I think it is a problem that this patch doesn't include a commit message describing what it does, and doesn't include a test plan (even though the test plan is in a comment).
Created attachment 13890 [details] [review] [SIGNED-OFF] Bug 9009 - Add separate user js and css for SCO module Add system preferences SCOUserCSS and SCOUserJS to define separate CSS and JavaScript for the Self Checkout Module. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Add something arbitrary to the new sysprefs SCOUserCSS and SCOUserJS ( such as TestCSS and TestJS ). 4) Load the SCO module in a browser, and view the HTML, verify the CSS and JS values in the system preferences have been included. Signed-off-by: Marc Veron <veron@veron.ch> I tested with an alert('hello') as JavaScript and some background-color for the CSS. Worked as expected.
I always include my test plan in a comment so we don't have them mucking up the git log. I also didn't include a commit message because I thought the title was self explanatory. (In reply to comment #11) > I think it is a problem that this patch doesn't include a commit message > describing what it does, and doesn't include a test plan (even though the > test plan is in a comment).
(In reply to comment #13) > I always include my test plan in a comment so we don't have them mucking up > the git log. I also didn't include a commit message because I thought the > title was self explanatory. If the test plan on the bug is clearly labelled "test plan:" or "to test:" I will probably use it. However, including the test plan in the commit message does increase the chances of the patch getting pushed to Master quickly. Marc's commit message is absolutely perfect, because it tells me in one second exactly what the patch does (adds two sysprefs), how to test it, and what tests Marc performed.
> If the test plan on the bug is clearly labelled "test plan:" or "to test:" I > will probably use it. However, including the test plan in the commit message > does increase the chances of the patch getting pushed to Master quickly. > Marc's commit message is absolutely perfect, because it tells me in one > second exactly what the patch does (adds two sysprefs), how to test it, and > what tests Marc performed. That's good to know. I'll follow that procedure from now on.
QA Comments: The patch is clean and it works as expected (tested some js and css). Marked as Passed QA.
Created attachment 14288 [details] [review] Bug 9009 - Add separate user js and css for SCO module Add system preferences SCOUserCSS and SCOUserJS to define separate CSS and JavaScript for the Self Checkout Module. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Add something arbitrary to the new sysprefs SCOUserCSS and SCOUserJS ( such as TestCSS and TestJS ). 4) Load the SCO module in a browser, and view the HTML, verify the CSS and JS values in the system preferences have been included. Signed-off-by: Marc Veron <veron@veron.ch> I tested with an alert('hello') as JavaScript and some background-color for the CSS. Worked as expected. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
This patch has been pushed to master.