In order to conform to best practices for performance staff client template javascript should be in the footer instead of the header. This patch will do so for the installer and onboarding templates.
Created attachment 74851 [details] [review] Bug 20658: Move template JavaScript to the footer: Installer and onboarding This patch updates the installer and onboarding templates so that JavaScript is included in the footer instead of the header. A new include file is added, installer-intranet-bottom.inc, so that only required JS is being loaded. Some markup changes are included to improve validity. To test, apply the patch and start with a blank Koha database. Run through the installer and onboarding processes and confirm that there are no problems. Check especially JS-related interactions like datepickers and fields which are validated via JS, like the password inputs. NOTE: The qa tool will flag this patch for not using the Asset plugin, but it doesn't work correctly for installer files.
Created attachment 78910 [details] [review] Bug 20658: Move template JavaScript to the footer: Installer and onboarding This patch updates the installer and onboarding templates so that JavaScript is included in the footer instead of the header. A new include file is added, installer-intranet-bottom.inc, so that only required JS is being loaded. Some markup changes are included to improve validity. To test, apply the patch and start with a blank Koha database. Run through the installer and onboarding processes and confirm that there are no problems. Check especially JS-related interactions like datepickers and fields which are validated via JS, like the password inputs. NOTE: The qa tool will flag this patch for not using the Asset plugin, but it doesn't work correctly for installer files.
Created attachment 78912 [details] [review] Bug 20658: Move template JavaScript to the footer: Installer and onboarding This patch updates the installer and onboarding templates so that JavaScript is included in the footer instead of the header. A new include file is added, installer-intranet-bottom.inc, so that only required JS is being loaded. Some markup changes are included to improve validity. To test, apply the patch and start with a blank Koha database. Run through the installer and onboarding processes and confirm that there are no problems. Check especially JS-related interactions like datepickers and fields which are validated via JS, like the password inputs. NOTE: The qa tool will flag this patch for not using the Asset plugin, but it doesn't work correctly for installer files. Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
html filters have been removed in the include file.
Created attachment 79300 [details] [review] Bug 20658: (follow-up) Fix for QA: Add missing html filters This patch adds missing filters to the installer header and footer include files.
QA script is quite unhappy, can you please check? OK koha-tmpl/intranet-tmpl/prog/en/includes/installer-doc-head-close.inc OK filters OK forbidden patterns OK git manipulation OK spelling OK tt_valid OK valid_template FAIL koha-tmpl/intranet-tmpl/prog/en/includes/installer-intranet-bottom.inc OK filters FAIL forbidden patterns forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 4) forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 1) forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 2) forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 3) forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 5) OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/installer/auth.tt OK filters OK forbidden patterns OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/installer/step1.tt OK filters OK forbidden patterns OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/installer/step2.tt OK filters OK forbidden patterns OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt OK filters OK forbidden patterns OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep1.tt OK filters OK forbidden patterns OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep2.tt OK filters FAIL forbidden patterns forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 132) OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep3.tt OK filters FAIL forbidden patterns forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 116) OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep4.tt OK filters FAIL forbidden patterns forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 52) OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found FAIL koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep5.tt OK filters FAIL forbidden patterns forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 139) OK git manipulation OK spelling OK tt_valid FAIL valid_template : filter not found
Created attachment 81504 [details] [review] Bug 20658: (follow-up) Fix for QA: Add missing html filters This patch adds missing filters to the installer header and footer include files.
*** Bug 22309 has been marked as a duplicate of this bug. ***
Created attachment 85404 [details] [review] Bug 20658: Move template JavaScript to the footer: Installer and onboarding This patch updates the installer and onboarding templates so that JavaScript is included in the footer instead of the header. A new include file is added, installer-intranet-bottom.inc, so that only required JS is being loaded. Some markup changes are included to improve validity. To test, apply the patch and start with a blank Koha database. Run through the installer and onboarding processes and confirm that there are no problems. Check especially JS-related interactions like datepickers and fields which are validated via JS, like the password inputs. NOTE: The qa tool will flag this patch for not using the Asset plugin, but it doesn't work correctly for installer files. Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85405 [details] [review] Bug 20658: (follow-up) Fix for QA: Add missing html filters This patch adds missing filters to the installer header and footer include files. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
I still see the QA errors related to paths, can we not use Asset here?
(In reply to Nick Clemens from comment #11) > I still see the QA errors related to paths, can we not use Asset here? I don't think so, Asset needs to read version from system preference, but can't because database does not exist yet (at least in first steps)
(In reply to Josef Moravec from comment #12) > (In reply to Nick Clemens from comment #11) > > I still see the QA errors related to paths, can we not use Asset here? > > I don't think so, Asset needs to read version from system preference, but > can't because database does not exist yet (at least in first steps) Is this correct? Why not Koha::version() instead?
(In reply to Jonathan Druart from comment #13) > (In reply to Josef Moravec from comment #12) > > (In reply to Nick Clemens from comment #11) > > > I still see the QA errors related to paths, can we not use Asset here? > > > > I don't think so, Asset needs to read version from system preference, but > > can't because database does not exist yet (at least in first steps) > > Is this correct? Why not Koha::version() instead? I agree, but it's for another bug report I think...
Opened bug 22451 and switched back to PQA.
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.04
cant apply to 18.05.x if this is need please rebase. thanks!
(In reply to Lucas Gass from comment #18) > cant apply to 18.05.x if this is need please rebase. I don't think it's necessary to backport this.
Indeed, its really minor