Bug 20658 - Move template JavaScript to the footer: Installer and onboarding
Summary: Move template JavaScript to the footer: Installer and onboarding
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Owen Leonard
QA Contact: Josef Moravec
URL:
Keywords:
: 22309 (view as bug list)
Depends on:
Blocks: 17858 22451
  Show dependency treegraph
 
Reported: 2018-04-25 16:23 UTC by Owen Leonard
Modified: 2020-01-06 20:14 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20658: Move template JavaScript to the footer: Installer and onboarding (23.23 KB, patch)
2018-04-25 17:50 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20658: Move template JavaScript to the footer: Installer and onboarding (23.71 KB, patch)
2018-09-15 22:38 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20658: Move template JavaScript to the footer: Installer and onboarding (23.77 KB, patch)
2018-09-15 22:54 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 20658: (follow-up) Fix for QA: Add missing html filters (3.12 KB, patch)
2018-09-24 12:16 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20658: (follow-up) Fix for QA: Add missing html filters (3.13 KB, patch)
2018-10-29 14:13 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20658: Move template JavaScript to the footer: Installer and onboarding (23.82 KB, patch)
2019-02-20 18:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20658: (follow-up) Fix for QA: Add missing html filters (3.19 KB, patch)
2019-02-20 18:43 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2018-04-25 16:23:10 UTC
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.
Comment 1 Owen Leonard 2018-04-25 17:50:39 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2018-09-15 22:38:13 UTC Comment hidden (obsolete)
Comment 3 Claire Gravely 2018-09-15 22:54:55 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2018-09-22 13:48:06 UTC
html filters have been removed in the include file.
Comment 5 Owen Leonard 2018-09-24 12:16:56 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2018-10-27 20:53:10 UTC
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
Comment 7 Owen Leonard 2018-10-29 14:13:15 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2019-02-11 16:56:40 UTC
*** Bug 22309 has been marked as a duplicate of this bug. ***
Comment 9 Josef Moravec 2019-02-20 18:43:42 UTC
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>
Comment 10 Josef Moravec 2019-02-20 18:43:46 UTC
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>
Comment 11 Nick Clemens 2019-02-27 13:45:11 UTC
I still see the QA errors related to paths, can we not use Asset here?
Comment 12 Josef Moravec 2019-02-27 14:41:05 UTC
(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)
Comment 13 Jonathan Druart 2019-02-27 14:58:34 UTC
(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?
Comment 14 Josef Moravec 2019-02-27 15:07:06 UTC
(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...
Comment 15 Jonathan Druart 2019-03-04 18:52:05 UTC
Opened bug 22451 and switched back to PQA.
Comment 16 Nick Clemens 2019-03-15 19:57:44 UTC
Awesome work all!

Pushed to master for 19.05
Comment 17 Martin Renvoize 2019-03-19 10:59:47 UTC
Pushed to 18.11.x for 18.11.04
Comment 18 Lucas Gass 2019-03-26 23:47:24 UTC
cant apply to 18.05.x if this is need please rebase. 


thanks!
Comment 19 Owen Leonard 2019-03-27 11:51:27 UTC
(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.
Comment 20 Fridolin Somers 2019-04-24 07:49:32 UTC
Indeed, its really minor