ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

make sure jQuery is loaded before account js 

fixes: COREFE-349

this will make it so our javascript, specifically the global “$”
variable from jQuery, is loaded before custom account JS runs

test plan:
* you may have to test this in firefox or some other browser besides
  chrome because I could never reproduce the original problem in chrome
* upload a file as the custom javascript file in the theme editor with
  something like:
  $(function(){console.log(‘it worked’, $)})
* save that and apply it to your account
* you should see the it worked message always logged to your console
  and should not ever see a “$ is undefined” error in the console

Once this is on beta:
* we should specifically test the schools that we know had this problem
  originally and see if it is resolved for them

Change-Id: Ic0f245cb649f3cf0148f4b0c9b263aa3b46aad57
Reviewed-on: https://gerrit.instructure.com/212390
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
This commit is contained in:
Ryan Shaw 2019-10-07 21:16:08 -06:00
parent 69b7443df7
commit 05a477e1c8
2 changed files with 51 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
app/helpers/application_helper.rb
View File

@ -170,6 +170,19 @@ module ApplicationHelper
(use_optimized_js? ? '/dist/webpack-production' : '/dist/webpack-dev').freeze
end
def load_scripts_async_in_order(script_urls)
# this is how you execute scripts in order, in a way that doesn’t block rendering,
# and without having to use 'defer' to wait until the whole DOM is loaded.
# see: https://www.html5rocks.com/en/tutorials/speed/script-loading/
javascript_tag "
;#{script_urls.map{ |url| javascript_path(url)}}.forEach(function(src) {
var s = document.createElement('script')
s.src = src
s.async = false
document.head.appendChild(s)
});"
end
# puts the "main" webpack entry and the moment & timezone files in the <head> of the document
def include_head_js
paths = []
@ -197,17 +210,8 @@ module ApplicationHelper
paths.each { |url| concat preload_link_tag(javascript_path(url)) }
chunk_urls.each { |url| concat preload_link_tag(url) }
# this is how you execute scripts in order, in a way that doesn’t block rendering,
# and without having to use 'defer' to wait until the whole DOM is loaded.
# see: https://www.html5rocks.com/en/tutorials/speed/script-loading/
concat javascript_tag "
;#{(paths + chunk_urls).map{ |url| javascript_path(url)}}.forEach(function(src) {
var s = document.createElement('script')
s.src = src
s.async = false
document.head.appendChild(s)
});
"
concat load_scripts_async_in_order(paths + chunk_urls)
concat include_js_bundles
end
end
@ -727,7 +731,9 @@ module ApplicationHelper
end
if includes.present?
javascript_include_tag(*includes, defer: true)
# Loading them like this puts them in the same queue as our script tags we load in
# include_head_js. We need that because we need them to load _after_ our jquery loads.
load_scripts_async_in_order(includes)
end
end

40
spec/helpers/application_helper_spec.rb
View File

@ -367,14 +367,34 @@ describe ApplicationHelper do
it "should just include domain root account's when there is no context or @current_user" do
output = helper.include_account_js
expect(output).to have_tag 'script'
expect(output).to eq("<script src=\"https://example.com/root/account.js\" defer=\"defer\"></script>")
expect(output).to eq("<script>
//<![CDATA[
;[\"https://example.com/root/account.js\"].forEach(function(src) {
var s = document.createElement('script')
s.src = src
s.async = false
document.head.appendChild(s)
});
//]]>
</script>")
end
it "should load custom js even for high contrast users" do
@current_user = user_factory
user_factory.enable_feature!(:high_contrast)
output = helper.include_account_js
expect(output).to eq("<script src=\"https://example.com/root/account.js\" defer=\"defer\"></script>")
expect(output).to eq("<script>
//<![CDATA[
;[\"https://example.com/root/account.js\"].forEach(function(src) {
var s = document.createElement('script')
s.src = src
s.async = false
document.head.appendChild(s)
});
//]]>
</script>")
end
it "should include granchild, child, and root when viewing the grandchild or any course or group in it" do
@ -382,11 +402,17 @@ describe ApplicationHelper do
group = course.groups.create!
[@grandchild_account, course, group].each do |context|
@context = context
expect(helper.include_account_js).to eq %{
<script src="https://example.com/root/account.js" defer="defer"></script>
<script src="https://example.com/child/account.js" defer="defer"></script>
<script src="https://example.com/grandchild/account.js" defer="defer"></script>
}.strip
expect(helper.include_account_js).to eq("<script>
//<![CDATA[
;[\"https://example.com/root/account.js\", \"https://example.com/child/account.js\", \"https://example.com/grandchild/account.js\"].forEach(function(src) {
var s = document.createElement('script')
s.src = src
s.async = false
document.head.appendChild(s)
});
//]]>
</script>")
end
end
end