Permalink
Browse files

Fix bug in khan-exercise.js where appropriate modules are not loaded …

…as users navigate to new exercises via tutorial-nav

Summary:
This fixes two bugs:
    1. First, with khan-exercise.js. Essentially, the modules of all loaded exercises, instead of the current exercise, were being used in runModules(), which executes the functions of the form $.fn["module"], $.fn["moduleLoad"], and $.fn["moduleCleanup"]. As a result, the current exercise was compiled using modules for all loaded exercises. This bug had not been found earlier b/c matrix-input is pretty unique right now in visibly modifying the answer area & b/c the matrices tutorial contains both exercises that use and don't use it. This bug was probably slowing down fairly lightweight exercises that were in the same tutorial as heavy exercises (ex: using the word-problems package), since they were running $.fn["word-problemsLoad"] on every newProblem event.
    2. Make sure matrix-input events are not attached again when each hint is requested.

There is room for improvement for whoever is brave enough to dive into this crazy mess of code.

Test Plan:
Devappserver:
    1. Visit /math/algebra/algebra-matrices/e/scalar_matrix_multiplication
    2. Navigate to "matrix dimensions" via tutorial nav
    3. Observe that the matrix input braces do not appear in the answer area

Exercise land:
    1. Visit scalar_matrix_multiplication.html and matrix_dimensions.html
    2. Observe that nothing's visibly changed

Reviewers: eater

Reviewed By: eater

CC: marcia, desmond

Differential Revision: http://phabricator.khanacademy.org/D938
  • Loading branch information...
1 parent ebfcd74 commit 8a389e547033ca33ee8221ab08fcc05ff5ef61da Stephanie H. Chang committed Nov 21, 2012
Showing with 114 additions and 26 deletions.
  1. +6 −4 exercises/matrix_inverse_2x2.html
  2. +6 −4 exercises/matrix_inverse_3x3.html
  3. +46 −9 khan-exercise.js
  4. +32 −3 utils/matrix-input.js
  5. +24 −6 utils/matrix.js
@@ -8,10 +8,12 @@
<body>
<div class="exercise">
- <div class="vars" data-ensure="DET !== 0">
- <var id="DIM">2</var>
- <var id="MAT">makeMatrix(randRange(-2, 5, DIM, DIM))</var>
- <var id="DET">matrixDet(MAT)</var>
+ <div class="vars">
+ <div data-ensure="DET !== 0">
+ <var id="DIM">2</var>
+ <var id="MAT">makeMatrix(randRange(-2, 5, DIM, DIM))</var>
+ <var id="DET">matrixDet(MAT)</var>
+ </div>
<var id="SOLN_MAT">matrixInverse(MAT)</var>
<var id="PADDED_SOLN_MAT">matrixPad(SOLN_MAT, 3, 3)</var>
<var id="PRETTY_SOLN_MAT">
@@ -8,10 +8,12 @@
<body>
<div class="exercise">
- <div class="vars" data-ensure="DET !== 0">
- <var id="DIM">3</var>
- <var id="MAT">makeMatrix(randRange(-2, 5, DIM, DIM))</var>
- <var id="DET">matrixDet(MAT)</var>
+ <div class="vars">
+ <div data-ensure="DET !== 0">
+ <var id="DIM">3</var>
+ <var id="MAT">makeMatrix(randRange(-2, 5, DIM, DIM))</var>
+ <var id="DET">matrixDet(MAT)</var>
+ </div>
<var id="SOLN_MAT">matrixInverse(MAT)</var>
<var id="PADDED_SOLN_MAT">matrixPad(SOLN_MAT, 3, 3)</var>
<var id="PRETTY_SOLN_MAT">
View
@@ -302,8 +302,20 @@ var Khan = (function() {
// The main Khan Module
var Khan = {
+
+ // Modules currently in use
modules: {},
+ // Map from exercise filename to a string of required modules
+ // (data-require). These module names are used in runModules(), where
+ // $.fn["module-name"], $.fn["module-nameLoad"], and
+ // $.fn["module-nameCleanup"] are called.
+ exerciseModulesMap: {},
+
+ // Used to retrieve required modules list for current exercise from
+ // exerciseModulesMap
+ currExerciseFilename: "",
+
// So modules can use file paths properly
urlBase: urlBase,
@@ -353,6 +365,26 @@ var Khan = (function() {
warn("You should " + enableFontDownload + " to improve the appearance of math expressions.", true);
},
+ resetModules: function(modules) {
+ Khan.modules = {};
+
+ if (testMode) {
+ Khan.require(["../jquery-ui", "../jquery.qtip"]);
+ }
+
+ // Base modules required for every problem
+ Khan.require(["answer-types", "tmpl", "underscore", "jquery.adhesion", "hints", "calculator"]);
+
+ if (modules) {
+ Khan.require(modules);
+ }
+
+ if (testMode && !modules) {
+ modules = document.documentElement.getAttribute("data-require");
+ Khan.require(modules);
+ }
+ },
+
require: function(mods) {
if (mods == null) {
return;
@@ -743,14 +775,7 @@ var Khan = (function() {
// Actually load the scripts. This is getting evaluated when the file is loaded.
Khan.loadScripts(scripts, function() {
- if (testMode) {
- Khan.require(["../jquery-ui", "../jquery.qtip"]);
- }
-
- // Base modules required for every problem
- Khan.require(["answer-types", "tmpl", "underscore", "jquery.adhesion", "hints", "calculator"]);
-
- Khan.require(document.documentElement.getAttribute("data-require"));
+ Khan.resetModules();
// Initialize to an empty jQuery set
exercises = jQuery();
@@ -963,6 +988,10 @@ var Khan = (function() {
exerciseFile = exerciseId + ".html";
}
+ // Store the filename of the current exercise for use as an
+ // index for setting/getting values from the exerciseModulesMap
+ Khan.currExerciseFilename = exerciseFile;
+
function finishRender() {
// Get all problems of this exercise type...
@@ -1206,6 +1235,9 @@ var Khan = (function() {
debugLog("added inline styles");
+ // Reset modules to only those required by the current exercise
+ Khan.resetModules(Khan.exerciseModulesMap[Khan.currExerciseFilename]);
+
// Run the main method of any modules
problem.runModules(problem, "Load");
debugLog("done with runModules Load");
@@ -2959,7 +2991,12 @@ var Khan = (function() {
requires = requires[2];
}
- Khan.require(requires);
+ // Store the module requirements in exerciseModulesMap
+ Khan.exerciseModulesMap[fileName] = requires;
+
+ // Calling resetModules here is necessary for populating
+ // Khan.modules immediately so that required scripts can be fetched
+ Khan.resetModules(requires);
// Extract contents from various tags and save them up for parsing when
// actually showing this particular exercise.
View
@@ -34,6 +34,8 @@ $.extend(KhanUtil, {
matrixInput: {
+ eventsAttached: false,
+
containerEl: null,
bracketEls: null,
cells: null,
@@ -92,6 +94,12 @@ $.extend(KhanUtil, {
this.bracketEls = [left, right];
},
+ removeBrackets: function() {
+ _.each(this.bracketEls, function(bracketEl) {
+ $(bracketEl).remove();
+ });
+ },
+
indexToRow: function(i) {
return Math.floor(i / this.COLS);
},
@@ -306,18 +314,39 @@ $.extend(KhanUtil, {
render: function() {
this.positionBrackets();
+ },
+
+ cleanup: function() {
+ this.removeBrackets();
}
}
});
-$.fn["matrix-input"] = function() {
+$.fn["matrix-inputLoad"] = function() {
+ if (KhanUtil.matrixInput.eventsAttached) {
+ return;
+ }
- $(Khan).on("newProblem", function() {
+ $(Khan).on("newProblem.matrix-input", function() {
KhanUtil.matrixInput.init();
});
- $(Khan).on("showGuess", function() {
+ $(Khan).on("showGuess.matrix-input", function() {
KhanUtil.matrixInput.setMaxValsFromScratch();
KhanUtil.matrixInput.render();
});
+
+ KhanUtil.matrixInput.eventsAttached = true;
+};
+
+$.fn["matrix-inputCleanup"] = function() {
+ if (!KhanUtil.matrixInput.eventsAttached) {
+ return;
+ }
+
+ KhanUtil.matrixInput.cleanup();
+ $(Khan).off("newProblem.matrix-input");
+ $(Khan).off("showGuess.matrix-input");
+
+ KhanUtil.matrixInput.eventsAttached = false;
};
View
@@ -1,14 +1,24 @@
$.extend(KhanUtil, {
// To add two 2-dimensional matrices, use
// deepZipWith(2, function(a, b) { return a + b; }, matrixA, matrixB);
- deepZipWith: function deepZipWith(depth, fn) {
+ deepZipWith: function(depth, fn) {
var arrays = [].slice.call(arguments, 2);
+ // if any of the "array" arguments to deepZipWith are null, return null
+ var hasNullValue = _.any(arrays, function(array) {
+ if (array === null) {
+ return true;
+ }
+ });
+ if (hasNullValue) {
+ return null;
+ }
+
if (depth === 0) {
return fn.apply(null, arrays);
} else {
return _.map(_.zip.apply(_, arrays), function(els) {
- return deepZipWith.apply(this, [depth - 1, fn].concat(els));
+ return KhanUtil.deepZipWith.apply(this, [depth - 1, fn].concat(els));
});
}
},
@@ -73,6 +83,10 @@ $.extend(KhanUtil, {
var args = Array.prototype.slice.call(arguments);
mat = KhanUtil.deepZipWith.apply(this, [2].concat(args));
+ if (!mat) {
+ return null;
+ }
+
var table = _.map(mat, function(row, i) {
return row.join(" & ");
}).join(" \\\\ ");
@@ -209,7 +223,7 @@ $.extend(KhanUtil, {
var c = mat.r;
if (!r || !c) {
- return undefined;
+ return null;
}
var matT = [];
@@ -237,7 +251,7 @@ $.extend(KhanUtil, {
// determinant is only defined for a square matrix
if (mat.r !== mat.c) {
- return undefined;
+ return null;
}
var a, b, c, d, e, f, g, h, k, det;
@@ -344,13 +358,13 @@ $.extend(KhanUtil, {
// if determinant is undefined or 0, inverse does not exist
if (!det) {
- return undefined;
+ return null;
}
var adj = KhanUtil.matrixAdj(mat);
if (!adj) {
- return undefined;
+ return null;
}
var inv = KhanUtil.deepZipWith(2, function(val) {
@@ -376,6 +390,10 @@ $.extend(KhanUtil, {
* @return {result of makeMatrix}
*/
matrixPad: function(mat, rows, cols, padVal) {
+ if (!mat) {
+ return null;
+ }
+
mat = KhanUtil.makeMatrix(mat);
matP = KhanUtil.matrixCopy(mat);

0 comments on commit 8a389e5

Please sign in to comment.