Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove error ReferenceError: JavascriptLoader is not defined in Zoom Image Tool #35661

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 66 additions & 20 deletions xmodule/templates/html/zooming_image.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,39 @@ data: |
&lt;img alt="<i>Text for screen readers</i>"
src="<i>path to the image you want to display in the unit</i>" /&gt;
&lt;/a>
&lt;div class="script_placeholder"
data-src="<i>path to the jquery.loupeAndLightbox.js JavaScript file in your course</i>"/&gt;
&lt;/div&gt;
&lt;script type="text/javascript"&gt;// &gt;![CDATA[
JavascriptLoader.executeModuleScripts($('.zooming-image-place').eq(0), function() {
$('.loupe').loupeAndLightbox({
width: 350,
height: 350,
lightbox: false
function loadScript(url, callback) {
let script = document.createElement("script");
script.type = "text/javascript";

if (script.readyState) { // IE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer support IE, so this conditional can be removed in favor of script.onload.

script.onreadystatechange = function() {
if (script.readyState === "loaded" || script.readyState === "complete") {
script.onreadystatechange = null;
callback();
}
};
} else { // Other browsers
script.onload = function() {
callback();
};
}

script.src = url;
document.head.appendChild(script);
}

function callback() {
JavascriptLoader.executeModuleScripts($('.zooming-image-place').eq(0), function() {
$('.loupe').loupeAndLightbox({
width: 350,
height: 350,
lightbox: false
});
});
});
}

loadScript("<i>path to jquery.loupeAndLightbox.js JavaScript file to your course</i>", callback);
// ]]&gt;&lt;/script&gt;
&lt;div id="ap_listener_added"&gt;&lt;/div&gt;
</pre>
Expand All @@ -34,24 +56,48 @@ data: |
<li>Replace the value of the link's <strong>href</strong> attribute with the path to the magnified image. Do not change the value of the class attribute.</li>
<li>Replace the value of the image's <strong>src</strong> attribute with the path to the image that will appear in the unit.</li>
<li>Replace the value of the image's <strong>alt</strong> attribute with text that both describes the image and the action or destination of clicking on the image. You <strong>must</strong> include alt text to provide an accessible label.</li>
<li>Replace the value of the div element's <strong>data-src</strong> attribute with the path to the jquery.loupeAndLightbox.js JavaScript file in your course.</li>
<li>Replace the value of the first parameter in <strong>loadScript</strong> function with the path to the jquery.loupeAndLightbox.js JavaScript file in your course.</li>
</ol>
<p>The example below shows a subset of the biochemical reactions that cells carry out. </p>
<p>You can view the chemical structures of the molecules by clicking on them. The magnified view also lists the enzymes involved in each step.</p>
<p class="sr">Press spacebar to open the magnifier.</p>
<div class="zooming-image-place" style="position: relative;">
<a class="loupe" href="https://studio.edx.org/c4x/edX/DemoX/asset/pathways_detail_01.png">
<img alt="magnify" src="https://studio.edx.org/c4x/edX/DemoX/asset/pathways_overview_01.png" />
</a>
<div class="script_placeholder" data-src="https://studio.edx.org/c4x/edX/DemoX/asset/jquery.loupeAndLightbox.js" />
<img alt="magnify" src="https://studio.edx.org/c4x/edX/DemoX/asset/pathways_detail_01.png" />
</a>
</div>
<script type="text/javascript">// <![CDATA[
JavascriptLoader.executeModuleScripts($('.zooming-image-place').eq(0), function() {
$('.loupe').loupeAndLightbox({
width: 350,
height: 350,
lightbox: false
});
});
function loadScript(url, callback) {
let script = document.createElement("script");
script.type = "text/javascript";

if (script.readyState) { // IE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. We can do with only script.onload, now.

script.onreadystatechange = function() {
if (script.readyState === "loaded" || script.readyState === "complete") {
script.onreadystatechange = null;
callback();
}
};
} else { // Other browsers
script.onload = function() {
callback();
};
}

script.src = url;
document.head.appendChild(script);
}

function callback() {
JavascriptLoader.executeModuleScripts($('.zooming-image-place').eq(0), function() {
$('.loupe').loupeAndLightbox({
width: 350,
height: 350,
lightbox: false
});
});
}

loadScript("https://studio.edx.org/c4x/edX/DemoX/asset/jquery.loupeAndLightbox.js", callback);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that this version of the loupe is broken when you use it on a full-width, desktop LMS. The positioning is off:

image

There's no way to fix it except by modifying the source of the script. And here lies the major problem I have with this template: by default it depends on a script that's hosted by edx.org, and a buggy one at that.

// ]]></script>
<div id="ap_listener_added"></div>
Loading