GST: work in progress on refactoring.

common/lib/xmodule/xmodule/gst_module.py

@@ -85,7 +85,7 @@ class GraphicalSliderToolModule(XModule):
         #substitute sliders
         slider_div = '<div class="{element_class}_slider" \
                                    id="{element_id}_slider_{var}" \
-                                   data-var="{var}" data-el_width="{width}"\
+                                   data-var="{var}" data-el_width="{width}">\
                      </div>'
         for var in variables:
             # find $slider var='var' ... $
@@ -151,7 +151,7 @@ class GraphicalSliderToolModule(XModule):
         """
         # root added for interface compatibility with xmltodict.parse
         self.configuration_json = json.dumps(
-                xmltodict.parse('<root>' +
+                xmltodict.parse('<root class="' + self.location.category + '">' +
                 stringify_children(self.definition['configuration'])
                              + '</root>'))
         return self.configuration_json

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/graph.js

@@ -268,7 +268,7 @@ define('Graph', ['logme'], function (logme) {
             }
 
             function addFunction(funcString, color, line, dot, label) {
-                var newFunctionObject, func, constNames;
+                var newFunctionObject, func, paramNames;
 
                 // The main requirement is function string. Without it we can't
                 // create a function, and the series cannot be calculated.
@@ -283,19 +283,19 @@ define('Graph', ['logme'], function (logme) {
                     'dot': false
                 };
 
-                // Get all of the constant names defined by the user in the
+                // Get all of the parameter names defined by the user in the
                 // XML.
-                constNames = state.getAllConstantNames();
+                paramNames = state.getAllParameterNames();
 
                 // The 'x' is always one of the function parameters.
-                constNames.push('x');
+                paramNames.push('x');
 
                 // Must make sure that the function body also gets passed to
                 // the Function constructor.
-                constNames.push(funcString);
+                paramNames.push(funcString);
 
                 // Create the function from the function string, and all of the
-                // available constants + the 'x' variable as it's parameters.
+                // available parameters AND the 'x' variable as it's parameters.
                 // For this we will use the built-in Function object
                 // constructor.
                 //
@@ -303,7 +303,7 @@ define('Graph', ['logme'], function (logme) {
                 // likely the user supplied an invalid JavaScript function body
                 // string. In this case we will not proceed.
                 try {
-                    func = Function.apply(null, constNames);
+                    func = Function.apply(null, paramNames);
                 } catch (err) {
                     // Let's tell the user. He will see a nice red error
                     // message instead of a graph.
@@ -363,9 +363,9 @@ define('Graph', ['logme'], function (logme) {
         }
 
         function generateData() {
-            var c0, functionObj, seriesObj, dataPoints, constValues, x, y;
+            var c0, functionObj, seriesObj, dataPoints, paramValues, x, y;
 
-            constValues = state.getAllConstantValues();
+            paramValues = state.getAllParameterValues();
 
             dataSeries = [];
 
@@ -379,16 +379,16 @@ define('Graph', ['logme'], function (logme) {
                 for (x = xrange.start; x <= xrange.end; x += xrange.step) {
 
                     // Push the 'x' variable to the end of the parameter array.
-                    constValues.push(x);
+                    paramValues.push(x);
 
                     // We call the user defined function, passing all of the
-                    // available constant values. inside this function they
+                    // available parameter values. Inside this function they
                     // will be accessible by their names.
-                    y = functionObj.func.apply(window, constValues);
+                    y = functionObj.func.apply(window, paramValues);
 
-                    // Return the constValues array to how it was before we
+                    // Return the paramValues array to how it was before we
                     // added 'x' variable to the end of it.
-                    constValues.pop();
+                    paramValues.pop();
 
                     // Add the generated point to the data points set.
                     dataPoints.push([x, y]);

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/gst_main.js

@@ -8,26 +8,29 @@ define(
     // Even though it is not explicitly in this module, we have to specify
     // 'GeneralMethods' as a dependency. It expands some of the core JS objects
     // with additional useful methods that are used in other modules.
-    ['State', 'GeneralMethods', 'Sliders', 'Inputs', 'Graph'],
-    function (State, GeneralMethods, Sliders, Inputs, Graph) {
+    ['State', 'GeneralMethods', 'Sliders', 'Inputs', 'Graph', 'logme'],
+    function (State, GeneralMethods, Sliders, Inputs, Graph, logme) {
 
     return GstMain;
 
     function GstMain(gstId) {
-        var config, state;
+        var config, gstClass, state;
 
         // Get the JSON configuration, and parse it, and store as an object.
         config = JSON.parse($('#' + gstId + '_json').html()).root;
 
+        gstClass = config['@class'];
+        logme('gstClass: ' + gstClass);
+
         // Parse the configuration settings for sliders and text inputs, and
         // extract all of the defined constants (their names along with their
         // initial values).
-        state = State(gstId, config);
+        state = State(gstId, gstClass, config);
 
         // Create the sliders and the text inputs, attaching them to
         // approriate constants.
-        Sliders(gstId, config, state);
-        Inputs(gstId, config, state);
+        Sliders(gstId, gstClass, state);
+        Inputs(gstId, gstClass, state);
 
         // Configure and display the loop. Attach event for the graph to be
         // updated on any change of a slider or a text input.

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/inputs.js

@@ -2,108 +2,60 @@
 // define() functions from Require JS available inside the anonymous function.
 (function (requirejs, require, define) {
 
-define('Inputs', [], function () {
+define('Inputs', ['logme'], function (logme) {
     return Inputs;
 
-    function Inputs(gstId, config, state) {
-        var constNamesUsed;
+    function Inputs(gstId, gstClass, state) {
+        var c1, paramName, allParamNames;
 
-        // There should not be more than one text input per a constant. This
-        // just does not make sense. However, nothing really prevents the user
-        // from specifying more than one text input for the same constant name.
-        // That's why we have to track which constant names already have
-        // text inputs for them, and prevent adding further text inputs to
-        // these constants.
-        //
-        // constNamesUsed is an object to which we will add properties having
-        // the name of the constant to which we are adding a text input to.
-        // When creating a new text input, we must consult with this object, to
-        // see if the constant name is not defined as it's property.
-        constNamesUsed = {};
-
-        // We will go thorugh all of the inputs, and those that have a valid
-        // '@var' property will be added to the page as a HTML text input
-        // element.
-        if ((typeof config.inputs !== 'undefined') &&
-            (typeof config.inputs.input !== 'undefined')) {
-            if ($.isArray(config.inputs.input)) {
-
-                // config.inputs.input is an array. For each element, we will
-                // add a text input.
-                for (c1 = 0; c1 < config.inputs.input.length; c1++) {
-                    createInput(config.inputs.input[c1]);
-                }
-            } else if ($.isPlainObject(config.inputs.input)) {
+        allParamNames = state.getAllParameterNames();
 
-                // config.inputs.input is an object. Add a text input for it.
-                createInput(config.inputs.input);
+        console.log(allParamNames);
 
-            }
-        }
+        for (c1 = 0; c1 < allParamNames.length; c1 += 1) {
+            $('#' + gstId).children('.' + gstClass + '_input').each(function (index, value) {
+                var inputDiv, paramName;
 
-        function createInput(obj) {
-            var constName, constValue, spanEl, inputEl, readOnly;
+                paramName = allParamNames[c1];
 
-            // The name of the constant is obj['@var']. If it is not specified,
-            // we will skip creating a text input for this constant.
-            if (typeof obj['@var'] !== 'string') {
-                return;
-            }
-            constName = obj['@var'];
+                inputDiv = $(value);
 
-            // We will not add a text input for a constant which already has a
-            // text input defined for it.
-            //
-            // We will add the constant name to the 'constNamesUsed' object in
-            // the end, when everything went successfully.
-            if (constNamesUsed.hasOwnProperty(constName)) {
-                return;
+                if (paramName === inputDiv.data('var')) {
+                    createInput(inputDiv, paramName);
+                }
+            });
         }
 
-            // Multiple sliders and/or inputs can represent the same constant.
-            // Therefore we will get the most recent const value from the state
-            // object. If it is undefined, we will skip creating a text input
-            // for this constant.
-            constValue = state.getConstValue(constName);
-            if (constValue === undefined) {
         return;
-            }
 
-            // With the constant name, and the constant value being defined,
-            // lets get the element on the page into which the text input will
-            // be inserted.
-            spanEl = $('#' + gstId + '_input_' + constName);
+        function createInput(inputDiv, paramName) {
+            var paramObj, inputWidth, readOnly;
 
-            // If a corresponding element for this constant does not exist on
-            // the page, we will not be making a text input.
-            if (spanEl.length === 0) {
-                return;
+            paramObj = state.getParamObj(paramName);
+
+            // We will define the width of the slider to a sensible default.
+            inputWidth = 400;
+
+            // See if it was specified by the user.
+            if (isFinite(parseInt(inputDiv.data('el_width'))) === true) {
+                inputWidth = parseInt(inputDiv.data('el_width'));
             }
 
-            // Create the text input element.
-            inputEl = $('<input type"text" />');
+            // Set the width of the element.
+            inputDiv.width(inputWidth);
 
-            // Set the current constant to the text input. It will be visible
-            // to the user.
-            inputEl.val(constValue);
+            inputDiv.css('display', 'inline-block');
 
-            // Before binding a 'change' event, we will check if this text
-            // input is specified as 'read only'.
-            //
-            // By default, this setting is false - the user can change the
-            // value in the text input.
-            readonly = false;
-            if (typeof obj['@readonly'] === 'string') {
-                if (obj['@readonly'] === 'true') {
-                    readonly = true;
-                }
+            readOnly = false;
+            if (inputDiv.attr('data-el_readonly').toLowerCase() === 'true') {
+                readOnly = true;
             }
 
-            if (readonly === true) {
+            if (readOnly === true) {
 
                 // In the case of a readonly config option, configure the text
                 // inputit as read-only, and NOT bind an event to it.
-                inputEl.attr('readonly', 'readonly');
+                inputDiv.attr('readonly', 'readonly');
 
             } else { // readonly !== true
 
@@ -111,14 +63,16 @@ define('Inputs', [], function () {
                 // the value of this text input, and presses 'enter' (or clicks
                 // somewhere else on the page), this event will be triggered, and
                 // our callback will be called.
-                inputEl.bind('change', inputOnChange);
+                inputDiv.bind('change', inputOnChange);
 
             }
 
+            inputDiv.val(paramObj.value);
+
             // Lets style the input element nicely. We will use the button()
             // widget for this since there is no native widget for the text
             // input.
-            inputEl.button().css({
+            inputDiv.button().css({
                 'font': 'inherit',
                 'color': 'inherit',
                 'text-align': 'left',
@@ -128,19 +82,22 @@ define('Inputs', [], function () {
                 // 'width': '50px'
             });
 
-            // And finally, publish the text input element to the page.
-            inputEl.appendTo(spanEl);
-
-            // Don't forget to add the constant to the list of used constants.
-            // Next time a slider for this constant will not be created.
-            constNamesUsed[constName] = true;
+            paramObj.inputDivs.push(inputDiv);
 
             return;
 
-            // When the user changes the value of this text input, the 'state'
-            // will be updated, forcing the plot to be redrawn.
+            // Update the 'state' - i.e. set the value of the parameter this
+            // input is attached to to a new value.
+            //
+            // This will cause the plot to be redrawn each time after the user
+            // changes the value in the input. Note that he has to either press
+            // 'Enter', or click somewhere else on the page in order for the
+            // 'change' event to be tiggered.
             function inputOnChange(event) {
-                state.setConstValue(constName, $(this).val());
+                var inputDiv;
+
+                inputDiv = $(this);
+                state.setParameterValue(paramName, inputDiv.val(), inputDiv);
             }
         }
     }

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/state.js

@@ -13,11 +13,22 @@ define('State', ['logme'], function (logme) {
     return State;
 
     // function: State
-    function State(gstId, config) {
-        var parameters, allParameterNames, allParameterValues, plotDiv;
+    function State(gstId, gstClass, config) {
+        var parameters, allParameterNames, allParameterValues,
+            plotDiv;
 
+        // Initially, there are no parameters to track. So, we will instantiate
+        // an empty object.
+        //
+        // As we parse the JSON config object, we will add parameters as
+        // named properties (for example
+        //
+        //     parameters.a = {...};
+        //
+        // for the parameter 'a'.
         parameters = {};
 
+        // Check that the required object is available.
         if (
             (typeof config.parameters !== 'undefined') &&
             (typeof config.parameters.param !== 'undefined')
@@ -28,7 +39,7 @@ define('State', ['logme'], function (logme) {
             if ($.isArray(config.parameters.param) === true) {
                 (function (c1) {
                     while (c1 < config.parameters.param.length) {
-                        addConstFromInput(config.parameters.param[c1]);
+                        processParameter(config.parameters.param[c1]);
                         c1 += 1;
                     }
                 }(0));
@@ -37,44 +48,53 @@ define('State', ['logme'], function (logme) {
             // If config.parameters.param is an object, pass this object to the
             // processor directly.
             else if ($.isPlainObject(config.inputs.input) === true) {
-                addConstFromInput(config.parameters.param);
+                processParameter(config.parameters.param);
             }
 
         }
 
+        // Instead of building these arrays every time when some component
+        // requests them, we will create them in the beginning, and then update
+        // by element when some parameter's value changes.
+        //
+        // Then we can just return the required array, instead of iterating
+        // over all of the properties of the 'parameters' object, and
+        // extracting their names/values one by one.
+        allParameterNames = [];
+        allParameterValues = [];
+
+        generateHelperArrays();
+
+        logme(parameters, allParameterNames, allParameterValues);
+
         // The constructor will return an object with methods to operate on
         // it's private properties.
         return {
             'getParameterValue': getParameterValue,
             'setParameterValue': setParameterValue,
 
+            'getParamObj': getParamObj,
+
             'getAllParameterNames': getAllParameterNames,
             'getAllParameterValues': getAllParameterValues,
 
             'bindUpdatePlotEvent': bindUpdatePlotEvent
         };
 
-        // ####################################################################
-        //
-        // To get all parameter names, you would do:
-        //
-        //     allParamNames = getAllParameterProperties('name');
-        //
-        // To get all parameter values, you would do:
-        //
-        //     allParamValues = getAllParameterProperties('value');
-        //
-        // ####################################################################
-        function getAllParameterProperties(propertyName) {
-            var paramName, allParamProperties;
+        function getAllParameterNames() {
+            return allParameterNames;
+        }
 
-            allParamProperties = [];
+        function getAllParameterValues() {
+            return allParameterValues;
+        }
 
-            for (paramName in parameters) {
-                allParamProperties.push(parameters[paramName][propertyName]);
+        function getParamObj(paramName) {
+            if (parameters.hasOwnProperty(paramName) === false) {
+                return;
             }
 
-            return allParamProperties;
+            return parameters[paramName];
         }
 
         function bindUpdatePlotEvent(newPlotDiv, callback) {
@@ -118,6 +138,8 @@ define('State', ['logme'], function (logme) {
         // the stored value in the parameter with the new value, and also
         // update all of the text inputs and the slider that correspond to this
         // parameter (if any), so that they reflect the new parameter's value.
+        // Finally, the helper array 'allParameterValues' will also be updated
+        // to reflect the change.
         //
         // If something went wrong (for example the new value is outside the
         // allowed range), then we will reset the 'element' to display the
@@ -125,29 +147,50 @@ define('State', ['logme'], function (logme) {
         //
         // ####################################################################
         function setParameterValue(paramName, paramValue, element) {
-            var inputDiv;
+            var paramValueNum, c1;
 
-            if (constants.hasOwnProperty(constName) === false) {
-                // If the name of the constant is not tracked by state, return an
-                // 'undefined' value.
+            // If a parameter with the name specified by the 'paramName'
+            // parameter is not tracked by state, do not do anything.
+            if (parameters.hasOwnProperty(paramName) === false) {
                 return;
             }
 
-            if (isNaN(parseFloat(constValue)) === true) {
-                // We are interested only in valid float values.
+            // Try to convert the passed value to a valid floating-point
+            // number.
+            paramValueNum = parseFloat(paramValue);
+
+            if (
+                // We are interested only in valid float values. NaN, -INF,
+                // +INF we will disregard.
+                (isFinite(paramValueNum) === false) ||
+
+                // If the new parameter's value is valid, but lies outised of
+                // the parameter's allowed range, we will also disregard it.
+                (paramValueNum < parameters[paramName].min) ||
+                (paramValueNum > parameters[paramName].max)
+            ) {
+                // We will also change the element's value back to the current
+                // parameter's value.
+                element.val(parameters[paramName].value);
+
                 return;
             }
 
-            constants[constName] = parseFloat(constValue);
+            parameters[paramName].value = paramValueNum;
 
             if (plotDiv !== undefined) {
                 plotDiv.trigger('update_plot');
             }
 
-            inputDiv = $('#' + gstId + '_input_' + constName).children('input');
-            if (inputDiv.length !== 0) {
-                inputDiv.val(constValue);
+            for (c1 = 0; c1 < parameters[paramName].inputDivs.length; c1 += 1) {
+                parameters[paramName].inputDivs[c1].val(paramValueNum);
             }
+
+            if (parameters[paramName].sliderDiv !== null) {
+                parameters[paramName].sliderDiv.slider('value', paramValueNum);
+            }
+
+            allParameterValues[parameters[paramName].helperArrayIndex] = paramValueNum;
         } // End-of: function setParameterValue
 
         // ####################################################################
@@ -156,7 +199,8 @@ define('State', ['logme'], function (logme) {
         // -------------------------------
         //
         //
-        // This function will be run once for each instance of a GST.
+        // This function will be run once for each instance of a GST when
+        // parsing the JSON config object.
         //
         // 'newParamObj' must be empty from the start for each invocation of
         // this function, that's why we will declare it locally.
@@ -201,6 +245,7 @@ define('State', ['logme'], function (logme) {
             if (
                 (processFloat('@min', 'min') === false) ||
                 (processFloat('@max', 'max') === false) ||
+                (processFloat('@step', 'step') === false) ||
                 (processFloat('@initial', 'value') === false)
             ) {
                 logme('---> Not adding a parameter named "' + paramName + '".');
@@ -208,7 +253,10 @@ define('State', ['logme'], function (logme) {
                 return;
             }
 
-            constants[constName] = constValue;
+            newParamObj.inputDivs = [];
+            newParamObj.sliderDiv = null;
+
+            parameters[paramName] = newParamObj;
 
             return;
 
@@ -234,11 +282,29 @@ define('State', ['logme'], function (logme) {
                     }
                 }
 
-                newParamObj[newAttrName] = paramValue;
+                newParamObj[newAttrName] = attrValue;
 
                 return true;
             } // End-of: function processFloat
         } // End-of: function processParameter
+
+        // Populate 'allParameterNames' and 'allParameterValues' with data.
+        // Link each parameter object with the corresponding helper array via
+        // an index ('helperArrayIndex'). It will be the same for both of the
+        // arrays.
+        function generateHelperArrays() {
+            var paramName, c1;
+
+            c1 = 0;
+            for (paramName in parameters) {
+                allParameterNames.push(paramName);
+                allParameterValues.push(parameters[paramName].value);
+
+                parameters[paramName].helperArrayIndex = c1;
+
+                c1 += 1;
+            }
+        }
     } // End-of: function State
 });