From 604f8265201bd1ff447819c2dce55112383e9536 Mon Sep 17 00:00:00 2001
From: smmercuri <smercuri@ed.ac.uk>
Date: Fri, 17 May 2024 13:28:55 +0100
Subject: [PATCH] Update docs and validation

---
 corsscripts/stacksortable.js                |  8 ++--
 corsscripts/stacksortable.min.js            |  6 +--
 doc/en/Authoring/Matching.md                | 13 +++++++
 doc/en/Authoring/Parsons.md                 | 27 ++++++++++----
 doc/en/Topics/Matching.md                   | 13 +++++++
 doc/en/Topics/Parsons.md                    | 13 +++++++
 lang/en/qtype_stack.php                     |  8 ++--
 stack/cas/castext2/blocks/parsons.block.php | 41 ++++++++++++++++-----
 8 files changed, 101 insertions(+), 28 deletions(-)

diff --git a/corsscripts/stacksortable.js b/corsscripts/stacksortable.js
index 8ac0f910c..248a0a800 100644
--- a/corsscripts/stacksortable.js
+++ b/corsscripts/stacksortable.js
@@ -80,7 +80,7 @@ export function preprocess_steps(steps, sortableUserOpts, headers, available_hea
     // ["steps", "options", "headers", "available_header", "index"], and contains at least "steps".
     // Separate these if they are present.
     if (_validate_top_level_keys_JSON(steps, ["steps", "options", "headers", "index", "available_header"], ["steps"])) {
-        var sortableUserOpts = steps["options"];
+        var sortableUserOpts = {used: steps["options"], available: steps["options"]};
         // only want to replace defaults for headers if they have been provided
         if ("headers" in steps) {
             headers = steps["headers"];
@@ -683,8 +683,8 @@ export const stack_sortable = class stack_sortable {
     _create_header(innerHTML, id, attrs) {
         let i = document.createElement("i");
         i.innerHTML = innerHTML;
-        var addClass = (this.orientation === "col") ?
-            [this.item_class, "header"] : [this.item_class, "index"];
+        var addClass = (this.grid && this.orientation !== "col") ?
+            [this.item_class, "index"] : [this.item_class, "header"];
         i.classList.add(...addClass);
         this._apply_attrs(i, {...{"id" : id}, ...attrs});
         return i;
@@ -800,7 +800,7 @@ export const stack_sortable = class stack_sortable {
         var warningMessage = document.createElement("span");
         warningMessage.textContent = msg;
         warning.append(warningMessage);
-        document.body.insertBefore(warning, document.getElementById("sortableContainer"));
+        document.body.insertBefore(warning, document.getElementById("containerRow"));
     }
 
     /**
diff --git a/corsscripts/stacksortable.min.js b/corsscripts/stacksortable.min.js
index 58a4c92f9..78794e9c2 100644
--- a/corsscripts/stacksortable.min.js
+++ b/corsscripts/stacksortable.min.js
@@ -1,6 +1,6 @@
 
 export const SUPPORTED_CALLBACK_FUNCTIONS=["onChoose","onUnchoose","onStart","onEnd","onAdd","onUpdate","onSort","onRemove","onFilter","onMove","onClone","onChange"];export function preprocess_steps(steps,sortableUserOpts,headers,available_header,index){if(typeof steps==="string"){steps=_stackstring_objectify(steps);}
-var valid=_validate_parsons_JSON(steps);if(_validate_top_level_keys_JSON(steps,["steps","options","headers","index","available_header"],["steps"])){var sortableUserOpts=steps["options"];if("headers"in steps){headers=steps["headers"];}
+var valid=_validate_parsons_JSON(steps);if(_validate_top_level_keys_JSON(steps,["steps","options","headers","index","available_header"],["steps"])){var sortableUserOpts={used:steps["options"],available:steps["options"]};if("headers"in steps){headers=steps["headers"];}
 if("available_header"in steps){available_header=steps["available_header"];}
 index=steps["index"];steps=steps["steps"];}
 if(typeof steps==="string"){steps=_stackstring_objectify(steps);}
@@ -36,7 +36,7 @@ var overwrittenKeys=[];var keysPreserved=true;["ghostClass","group","onSort"].fo
 {keysPreserved=false;overwrittenKeys.push(key);}});if(!keysPreserved){err+=overwrittenErr+overwrittenKeys.join(", ")+".";}
 if(!keysRecognised||!keysPreserved){this._display_warning(err);}}
 _apply_attrs(el,opts){for(const[key,value]of Object.entries(opts)){el.setAttribute(key,value);}}
-_create_header(innerHTML,id,attrs){let i=document.createElement("i");i.innerHTML=innerHTML;var addClass=(this.orientation==="col")?[this.item_class,"header"]:[this.item_class,"index"];i.classList.add(...addClass);this._apply_attrs(i,{...{"id":id},...attrs});return i;}
+_create_header(innerHTML,id,attrs){let i=document.createElement("i");i.innerHTML=innerHTML;var addClass=(this.grid&&this.orientation!=="col")?[this.item_class,"index"]:[this.item_class,"header"];i.classList.add(...addClass);this._apply_attrs(i,{...{"id":id},...attrs});return i;}
 _create_ids(rows,columns){var colIdx=Array.from({length:columns},(_,i)=>i);var rowIdx=Array.from({length:rows},(_,j)=>j);this.colIds=colIdx.map((idx)=>`usedList_${idx}`);this.rowColIds={}
 colIdx.forEach((i)=>this.rowColIds[this.colIds[i]]=rowIdx.map((j)=>`usedList_${j}${i}`));var usedIds=(rows==="")?this.colIds.map((id)=>[id]):Object.values(this.rowColIds);return{used:usedIds,available:"availableList"};}
 _create_index(innerHTML,id,attrs){let i=document.createElement("i");i.innerHTML=innerHTML;var addClass=(this.orientation==="col")?[this.item_class,"index"]:[this.item_class,"header"];i.classList.add(...addClass);this._apply_attrs(i,{...{"id":id},...attrs});return i;}
@@ -44,7 +44,7 @@ _create_li(stepKey,attrs){let li=document.createElement("li");li.innerHTML=this.
 _deletable_li(li){return!li.matches(".header")&&!li.matches(".index")&&!this._is_empty_li(li);}
 _delete_all_from_used(){const lis=document.querySelectorAll(".usedList li[data-id]");lis.forEach(li=>{if(this._deletable_li(li)){this._delete_li(li);}});}
 _delete_li(li){li.parentNode.removeChild(li);}
-_display_warning(msg){var warning=document.createElement("div");warning.className="sortable-warning";var exclamation=document.createElement("i");exclamation.className="icon fa fa-exclamation-circle text-danger fa-fw";warning.append(exclamation);var warningMessage=document.createElement("span");warningMessage.textContent=msg;warning.append(warningMessage);document.body.insertBefore(warning,document.getElementById("sortableContainer"));}
+_display_warning(msg){var warning=document.createElement("div");warning.className="sortable-warning";var exclamation=document.createElement("i");exclamation.className="icon fa fa-exclamation-circle text-danger fa-fw";warning.append(exclamation);var warningMessage=document.createElement("span");warningMessage.textContent=msg;warning.append(warningMessage);document.body.insertBefore(warning,document.getElementById("containerRow"));}
 _double_clickable(item){return!item.matches(".header")&&!item.matches(".index");}
 _flip_orientation(){var addClass=(this.orientation==="row")?["list-group","col"]:["row"];if(this.grid){var removeClass=(this.orientation==="row")?["list-group","row"]:["list-group","col"];var currGridClass=(this.orientation==="row")?"grid-item-rigid":"grid-item";var gridAddClass=(this.orientation==="row")?"grid-item":"grid-item-rigid"
 var gridItems=document.querySelectorAll(`.${currGridClass}`);gridItems.forEach((item)=>{item.classList.remove(currGridClass);item.classList.add(gridAddClass);})
diff --git a/doc/en/Authoring/Matching.md b/doc/en/Authoring/Matching.md
index c11d7af86..91bb5945c 100644
--- a/doc/en/Authoring/Matching.md
+++ b/doc/en/Authoring/Matching.md
@@ -78,6 +78,19 @@ The final JSON key allowed inside the `parsons` block is `"options"` whose value
 
 See [the Parsons authoring guide](Parsons.md#block-parameters) for a full list of supported block parameters.
 
+## Troubleshooting
+
+If your matching problem is not displaying properly, in particular if the all the items are displayed in a single yellow block, then
+double-check that you have spelled the keys of the JSON inside the Parsons block correctly as described below. They should be a subset of 
+```
+{"steps", "options", "headers", "available_header", "index"}
+```
+and a superset of 
+```
+{"steps"}
+```
+For technical reasons this is one error that we are unable to validate currently.
+
 ## State
 
 The state of the problem at any given point in time during question answer takes on the following format:
diff --git a/doc/en/Authoring/Parsons.md b/doc/en/Authoring/Parsons.md
index 6f8e6b3ae..41836413e 100644
--- a/doc/en/Authoring/Parsons.md
+++ b/doc/en/Authoring/Parsons.md
@@ -55,10 +55,8 @@ The `[[parsons]]` block is a wrapper for the javascript library "Sortable.js", o
 ````
 [[parsons input="ans1"]]
 { "steps": {# stackjson_stringify(proof_steps) #},
-  "options": {"header" : ["Custom header for the answer list", "Custom header for the available steps"],
-              "sortable option 1" : value,
-              ...
-              "sortable option n" : value}
+  "options": {"sortable option 1" : value, ..., "sortable option n" : value},
+  "headers" : ["Custom header for the answer list"], 
 }
 [[/parsons]]
 ````
@@ -72,15 +70,28 @@ A list of all Sortable.js options can be found [here](https://github.com/Sortabl
 ````
 Most other Sortable options can be modified, except for `ghostClass`, `group` and `onSort` as these are required to be set for basic functionality.
 
-The only non-Sortable option that may currently be customised is the `header` option. The default for these are:
+Note that if you enter an unknown sortable option or if an attempt to pass `ghostClass`, `group`, or `onSort` is made, then these will simply be ignored. A warning will be displayed on the question page to signify this situation.
+
+The default for "headers" and "available_header" are:
 ````
 {
-    "header": ["Construct your solution here:", "Drag from here:"]
+    "headers": ["Construct your solution here:"], 
+    "available_header": ["Drag from here:"]
 }
 ````
-To modify these pass an array of length two, with first entry corresponding to the header for the answer list and the second entry corresponding to the header for the list of available steps.
 
-Note that if you enter an unknown sortable option or if an attempt to pass `ghostClass`, `group`, or `onSort` is made, then these will simply be ignored. A warning will be displayed on the question page to signify this situation.
+#### Troubleshooting
+
+If your Parson's problem is not displaying properly, in particular if the all the items are displayed in a single yellow block, then
+double-check that you have spelled the keys of the JSON inside the Parsons block correctly as described above. They should be a subset of 
+```
+{"steps", "options", "headers", "available_header"}
+```
+and a superset of 
+```
+{"steps"}
+```
+For technical reasons this is one error that we are unable to validate currently.
 
 ### Block parameters
 
diff --git a/doc/en/Topics/Matching.md b/doc/en/Topics/Matching.md
index c2908c74c..55b80fb0e 100644
--- a/doc/en/Topics/Matching.md
+++ b/doc/en/Topics/Matching.md
@@ -16,6 +16,19 @@ The main difference between them is that **Grid** allows the student to drag any
 of whether the item above it has been filled; **Grouping** on the other hand only allows students to drag items to the 
 end of the list within a column.
 
+## Troubleshooting
+
+If your matching problem is not displaying properly, in particular if the all the items are displayed in a single yellow block, then
+double-check that you have spelled the keys of the JSON inside the Parsons block correctly as described below. They should be a subset of 
+```
+{"steps", "options", "headers", "available_header", "index"}
+```
+and a superset of 
+```
+{"steps"}
+```
+For technical reasons this is one error that we are unable to validate currently.
+
 ## Switching orientation
 
 Parsons blocks will display columns vertically by default. 
diff --git a/doc/en/Topics/Parsons.md b/doc/en/Topics/Parsons.md
index 3733d0539..115802bbf 100644
--- a/doc/en/Topics/Parsons.md
+++ b/doc/en/Topics/Parsons.md
@@ -33,6 +33,19 @@ Notes
 * Lists are a special case of a tree with one root (the list creation function) and an arbitrary number of nodes in order.  Hence our design explicitly includes traditional Parson's problems as a special case.
 * Teachers who do not want to scaffold explicit block structures (e.g. signal types of proof blocks) can choose to restrict students to (i) flat lists, or (ii) lists of lists.
 
+## Troubleshooting
+
+If your Parson's problem is not displaying properly, in particular if the all the items are displayed in a single yellow block, then
+double-check that you have spelled the keys of the JSON inside the Parsons block correctly as described below. They should be a subset of 
+```
+{"steps", "options", "headers", "available_header"}
+```
+and a superset of 
+```
+{"steps"}
+```
+For technical reasons this is one error that we are unable to validate currently.
+
 # Example 1: a minimal Parson's question
 
 The following is a minimal Parson's question where there student is expected to create a list in one and only one order.
diff --git a/lang/en/qtype_stack.php b/lang/en/qtype_stack.php
index a8fd89d82..73917204f 100644
--- a/lang/en/qtype_stack.php
+++ b/lang/en/qtype_stack.php
@@ -951,12 +951,14 @@ $string['stackBlock_parsons_ref']         = 'The Parson\'s block only supports r
 $string['stackBlock_parsons_param']       = 'The Parson\'s block supports only these parameters in this context: \'{$a->param}\'.';
 $string['stackBlock_parsons_contents']    = 'The contents of a Parson\'s block must be a either a JSON of the form {#stackjson_stringify(steps)#}, where `steps` is the two-dimensional Maxima array containing key, value pairs of items, or of the form {\'steps\' : {#stackjson_stringify(steps)#}, \'options\' : {JSON containing Sortable options}, \'header\' : [List of headers], \'available_header\' : \'string containing header for the available list\', \'index\' : [List containing the index]}, where the \'options\', \'header\', \'available_header\', and \'index\' keys are optional. Alternatively, the contents of the Parsons block may contain raw JSON equivalents. Make sure that the `steps` Maxima variable is of the correct format. Note that all steps must be strings. See the https://docs.stack-assessment.org/en/Authoring/Parsons/ for details.';
 $string['stackBlock_incorrect_header_length'] = 'The list of headers should have the same length as the number of columns passed to the block header.';
-$string['stackBlock_incorrect_available_header_length'] = 'The list containing the header for the available list must have only one element.';
+$string['stackBlock_incorrect_available_header_type'] = 'The header for the available list should be passed as a string or a list of length one.';
 $string['stackBlock_incorrect_index_length'] = 'The length of the index should be one more than the number of rows passed to the block header. An item in the top-left corner should always go in the index';
-$string['stackBlock_incorrect_index_type'] = 'Index should be an array.';
-$string['stackBlock_incorrect_header_type'] = 'Headers should be an array.';
+$string['stackBlock_incorrect_index_type'] = 'Index should be an array containing strings.';
+$string['stackBlock_incorrect_header_type'] = 'Headers should be an array containing strings.';
 $string['stackBlock_parsons_invalid_columns_value'] = 'The value of `columns` in the Parson\'s block header should be a string containing a positive integer.';
 $string['stackBlock_parsons_invalid_rows_value'] = 'The value of `rows` in the Parson\'s block header should be a string containing a positive integer.';
+$string['stackBlock_parsons_invalid_item-height_value'] = 'The value of `item-height` in the Parson\'s block header should be a string containing a positive integer.';
+$string['stackBlock_parsons_invalid_item-width_value'] = 'The value of `item-width` in the Parson\'s block header should be a string containing a positive integer.';
 $string['stackBlock_unknown_sortable_option'] = 'Unknown Sortable options found, the following are being ignored: ';
 $string['stackBlock_overwritten_sortable_option'] = 'Unchangeable Sortable options found, the following are being ignored: ';
 $string['stackBlock_parsons_unknown_transpose_value'] = 'Transpose must be one of \'true\' or \'false\'.';
diff --git a/stack/cas/castext2/blocks/parsons.block.php b/stack/cas/castext2/blocks/parsons.block.php
index 0da9c3b06..7b198b0ab 100644
--- a/stack/cas/castext2/blocks/parsons.block.php
+++ b/stack/cas/castext2/blocks/parsons.block.php
@@ -227,6 +227,7 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block {
         // Invalid JSON will be identified by preprocess_steps function.
         $code .= 'var sortableUserOpts = {};' . "\n";
         $code .= 'var valid, index;' . "\n";
+
         $code .= '[proofSteps, sortableUserOpts, headers, available_header, index, valid] =
             preprocess_steps(proofSteps, sortableUserOpts, headers, available_header, index);' . "\n";
 
@@ -235,8 +236,8 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block {
             {stack_js.display_error("' . stack_string('stackBlock_parsons_contents') . '");}' . "\n";
         
         // More specific pieces of validation
-        // Check typing of headers, it should be an array
-        $code .= 'if (!Array.isArray(headers)) 
+        // Check typing of headers, it should be an array containing strings.
+        $code .= 'if (!(Array.isArray(headers) && headers.every((header) => typeof(header) === "string"))) 
             {stack_js.display_error("' . stack_string('stackBlock_incorrect_header_type') . '");}' . "\n";
 
         // If the length of headers does not match the number of columns expected throw an error.
@@ -248,12 +249,16 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block {
             $code .= stack_string('stackBlock_incorrect_header_length') . '");}' . "\n";
         }
 
-        // If the length of the available headers is not equal to one, throw an error
-        $code .= 'if (typeof(available_header) !== "string")
-            {stack_js.display_error("' . stack_string('stackBlock_incorrect_available_header_length') . '");}' . "\n";
+        // Validate available headers. It 
+        // is either a string or an array containing a single string. 
+        $code .= 'if (!(typeof(available_header) === "string" ||
+        (Array.isArray(available_header) && available_header.length === 1 && typeof(available_header[0]) === "string")))
+            {stack_js.display_error("' . stack_string('stackBlock_incorrect_available_header_type') . '");}' . "\n";
+        // Extract available header if it is an array containing a single string
+        $code .= 'if (Array.isArray(available_header)) {available_header = available_header[0]};' . "\n";
 
-        // If index is passed then it should be an array
-        $code .= 'if (index !== undefined && !Array.isArray(index))
+        // If index is passed then it should be an array containing strings.
+        $code .= 'if (index !== undefined && !(Array.isArray(index) && index.every((idx) => typeof(idx) === "string")))
             {stack_js.display_error("' . stack_string('stackBlock_incorrect_index_type') . '");}' . "\n";
 
         // If rows and index are passed then the length of index should match the value of rows + 1
@@ -293,7 +298,7 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block {
         // First, instantiate with default options first in order to extract all possible options for validation.
         $code .= 'var sortableUsed =
         stackSortable.ids.used.map((idList) =>
-            idList.map((usedId) => Sortable.create(document.getElementById(usedId), stackSortable.options.used)));' . "\n";
+            idList.map((usedId) => Sortable.create(document.getElementById(usedId))));' . "\n";
         $code .= 'var possibleOptionKeys = Object.keys(sortableUsed[0][0].options).concat(SUPPORTED_CALLBACK_FUNCTIONS);' . "\n";
         // Now set appropriate options.
 
@@ -460,7 +465,7 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block {
             }
         }
 
-        // Check value of columns is a string containing a numeric integer
+        // Check value of columns is a string containing a numeric positive integer
         if (array_key_exists("columns", $this->params)) {
             if (!(preg_match('/^\d+$/', $this->params["columns"]) && intval($this->params["columns"]) > 0)) {
                 $valid = false;
@@ -468,7 +473,7 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block {
             }
         }
         
-        // Check value of rows is a string containing a numeric integer
+        // Check value of rows is a string containing a numeric positive integer
         if (array_key_exists("rows", $this->params)) {
             if (!(preg_match('/^\d+$/', $this->params["rows"]) && intval($this->params["rows"]) > 0)) {
                 $valid = false;
@@ -482,6 +487,22 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block {
             $err[] = stack_string("stackBlock_parsons_underdefined_grid");
         }
 
+        // Check value of `item-height` is a string containing a positive integer
+        if (array_key_exists("item-height", $this->params)) {
+            if (!(preg_match('/^\d+$/', $this->params["item-height"]) && intval($this->params["item-height"]) > 0)) {
+                $valid = false;
+                $err[] = stack_string("stackBlock_parsons_invalid_item-height_value");
+            }
+        }
+
+        // Check value of `item-width` is a string containing a positive integer
+        if (array_key_exists("item-width", $this->params)) {
+            if (!(preg_match('/^\d+$/', $this->params["item-width"]) && intval($this->params["item-width"]) > 0)) {
+                $valid = false;
+                $err[] = stack_string("stackBlock_parsons_invalid_item-width_value");
+            }
+        }
+
         // Check that only valid parameters are passed to block header.
         $valids = null;
         foreach ($this->params as $key => $value) {
-- 
GitLab