diff --git a/corsscripts/stacksortable.js b/corsscripts/stacksortable.js index 88a0d2fa2fdd08319ee3d1f351dc1009596ece4f..8ac0f910cc609c86a3a986856dd67faafb10b5d5 100644 --- a/corsscripts/stacksortable.js +++ b/corsscripts/stacksortable.js @@ -33,6 +33,11 @@ export const SUPPORTED_CALLBACK_FUNCTIONS = [ * 2. It validates the structure of the Parson's JSON using `_validate_parsons_JSON`. * 3. If the Parsons JSON is of depth two with a valid set of top-level keys it separates them. * 4. If `steps` is a Maxima string (after separation), it converts it to an object. + * + * NB: there is a rare case in which this causes issues: if the author happens to only use a subset of + * ["steps", "options", "headers", "available_header", "index"] as the keys inside their `steps` Question variable. + * This will cause improper validation in the call of `_validate_parsons_JSON` but it will also cause + * functional issues in the question because we will extract the values of those keys from the object. * * @param {Object|string} steps - The steps object or string representation of steps. * @param {Object} sortableUserOpts - Options for the sortable plugin. @@ -119,6 +124,12 @@ function _stackstring_objectify(stackjson_array_string) { * 2. If the JSON has depth 2, the top-level keys should be a subset of * `["steps", "options", "headers", "index", "available_header"]`, and must contain `"steps"`. * The value for "steps" should be a valid flat JSON. Options are not validated here. + * + * NB: The separation of depth 1 and depth 2 cases only really works when raw JSON are written by the author. + * This does not work in cases where they are pulled through from Maxima variables using `{# ... #}`. This is + * because we check JSON depth by checking if any of the values is an object, and in the Maxima case, this isn't true as the + * item causing "depth 2" is now a string containing a two-dimensional array. Not clear how to circumvent this, + * it just means there's a gap in validation currently but does not break any functionality. * * @param {Object} steps - The Parson's JSON to be validated. * @returns {boolean} - Returns true if the provided Parsons JSON follows the expected structure, false otherwise. @@ -371,8 +382,7 @@ export const stack_sortable = class stack_sortable { this.availableId = this.ids.available; this.usedId = this.ids.used; this.clone = clone; - - this.defaultOptions = {used: {animation: 50, cancel: ".header"}, available: {animation: 50, cancel: ".header"}}; + this.defaultOptions = {used: {animation: 50}, available: {animation: 50}}; // Merges user options and default, overwriting default with user options if they clash this.userOptions = this._set_user_options(options); diff --git a/corsscripts/stacksortable.min.js b/corsscripts/stacksortable.min.js index d4441c5f434dda336d5908ef9b8a62f3e8940c9e..58a4c92f906078e5561c8515a5d3bb7294101b89 100644 --- a/corsscripts/stacksortable.min.js +++ b/corsscripts/stacksortable.min.js @@ -15,7 +15,7 @@ return Object.values(steps).every((val)=>typeof(val)=="string");} function _validate_top_level_keys_JSON(JSON,validKeys,requiredKeys){const keys=Object.keys(JSON);const missingRequiredKeys=requiredKeys.filter((key)=>!keys.includes(key));const invalidKeys=keys.filter((key)=>!validKeys.includes(key));return invalidKeys.length===0&&missingRequiredKeys.length===0;} export function get_iframe_height(){return document.documentElement.offsetHeight;} export const stack_sortable=class stack_sortable{constructor(steps,inputId=null,options=null,clone=false,columns=1,rows=null,orientation="col",index="",grid=false,item_height=null,item_width=null){this.steps=steps;this.inputId=inputId;this.orientation=orientation;this.columns=((this.orientation==="col")?columns:rows);this.rows=((this.orientation==="col")?rows:columns);this.index=index;this.use_index=this.index!=="";this.grid=grid;this.item_class=(this.grid?(this.orientation==="row"?"grid-item-rigid":"grid-item"):"list-group-item");this.item_height_width={"style":""};for(const[key,val]of[["height",item_height],["width",item_width]]){if(val!==""){this.item_height_width["style"]+=`${key}:${val}px;`};};this.item_height_width=(this.item_height_width["style"]==="")?{}:this.item_height_width;this.container_height_width=(this.item_height_width["style"]!=="")?{"style":this.item_height_width["style"]+"margin: 12px;"}:{};this.state=this._generate_state(this.steps,inputId,Number(this.columns),Number(this.rows));if(inputId!==null){this.input=document.getElementById(this.inputId);this.submitted=this.input.getAttribute("readonly")==="readonly"} -this.ids=this._create_ids(this.rows,this.columns);this.availableId=this.ids.available;this.usedId=this.ids.used;this.clone=clone;this.defaultOptions={used:{animation:50,cancel:".header"},available:{animation:50,cancel:".header"}};this.userOptions=this._set_user_options(options);this.options=this._set_ghostClass_group_and_disabled_options();} +this.ids=this._create_ids(this.rows,this.columns);this.availableId=this.ids.available;this.usedId=this.ids.used;this.clone=clone;this.defaultOptions={used:{animation:50},available:{animation:50}};this.userOptions=this._set_user_options(options);this.options=this._set_ghostClass_group_and_disabled_options();} add_dblclick_listeners(newUsed,newAvailable){this.available.addEventListener("dblclick",(e)=>{if(this._double_clickable(e.target)){var li=this._get_moveable_parent_li(e.target);li=(this.clone==="true")?li.cloneNode(true):this.available.removeChild(li);this.used[0][0].append(li);this.update_state(newUsed,newAvailable);}});this.used[0][0].addEventListener("dblclick",(e)=>{if(this._double_clickable(e.target)){var li=this._get_moveable_parent_li(e.target);this.used[0][0].removeChild(li);if(this.clone!=="true"){this.available.insertBefore(li,this.available.children[1]);} this.update_state(newUsed,newAvailable);}});} add_delete_all_listener(buttonId,newUsed,newAvailable){const button=document.getElementById(buttonId);button.addEventListener("click",()=>{this._delete_all_from_used();this.update_state(newUsed,newAvailable);});} diff --git a/lang/en/qtype_stack.php b/lang/en/qtype_stack.php index 2472e204f78fd3fc9c1ec5613df096f8c5deccbf..a8fd89d82d832fbbd3f521f5c526f3b31b4554c2 100644 --- a/lang/en/qtype_stack.php +++ b/lang/en/qtype_stack.php @@ -949,9 +949,20 @@ $string['stackBlock_parsons_unknown_named_version'] = 'The Parson\'s block only $string['stackBlock_parsons_unknown_mathjax_version'] = 'The Parson\'s block only supports MathJax versions {$a->mjversion} for the mathjax parameter.'; $string['stackBlock_parsons_ref'] = 'The Parson\'s block only supports referencing inputs present in the same CASText section \'{$a->var}\' does not exist here.'; $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 JSON of the form {#stackjson_stringify(proof_steps)#}. If you are passing custom objects then the Parson\'s block contents should be a JSON of the form {steps: {#stackjson_stringify(proof_steps)#}, options: {JSON containing Sortable options}}. Alternatively, the contents of the Parsons block may contain raw JSON equivalents. Make sure that the proof_steps Maxima variable is of the correct format. Note that all proof steps must be strings. See the documentation for details.'; +$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_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_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_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\'.'; +$string['stackBlock_parsons_underdefined_grid'] = 'When defining `rows` for a Parson\'s block one must also define `columns`.'; +$string['stackBlock_proof_mode_index'] = 'The use of \'index\' is not supported when using the Parson\'s block for proof assessment.'; +$string['stackBlock_proof_incorrect_header_length'] = 'Headers should be an array containing a single header; use \'available_header\' to update the header for the available list.'; // Define the stackBlock GeoGebra strings. $string['stackBlock_geogebra_width'] = 'The width of a GeoGebra Applet must use a known CSS-length unit.'; diff --git a/stack/cas/castext2/blocks/parsons.block.php b/stack/cas/castext2/blocks/parsons.block.php index cdb2c9eda859d8a02b925054ad09ce747e4a51d6..0da9c3b06259a91dcb793dc3ef6c7c67ea9b31b4 100644 --- a/stack/cas/castext2/blocks/parsons.block.php +++ b/stack/cas/castext2/blocks/parsons.block.php @@ -163,9 +163,10 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block { $columns = $proofmode ? '1' : $columns; // Add correctly oriented container divs for the proof lists to be accessed by sortable. $orientation = $transpose ? 'row' : 'col'; - $tmprows = $transpose ? $columns : $rows; - $columns = $transpose ? $rows : $columns; - $rows = $tmprows; + $ogcolumns = $columns; + $ogrows = $rows; + $columns = $transpose ? $ogrows : $ogcolumns; + $rows = $transpose ? $ogcolumns : $ogrows; $r->items[] = new MP_String("<button type='button' class='parsons-button' id='resize'> <i class='fa fa-expand'></i></button>"); @@ -229,9 +230,43 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block { $code .= '[proofSteps, sortableUserOpts, headers, available_header, index, valid] = preprocess_steps(proofSteps, sortableUserOpts, headers, available_header, index);' . "\n"; - // If the author's JSON has invalid format throw an error. + // If the author's JSON has invalid structure throw an error. $code .= 'if (valid === false) {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)) + {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. + // Error is different for proof vs. matching + $code .= 'if (headers.length !== ' . $ogcolumns . ') {stack_js.display_error("'; + if ($proofmode) { + $code .= stack_string('stackBlock_proof_incorrect_header_length') . '");}' . "\n"; + } else { + $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"; + + // If index is passed then it should be an array + $code .= 'if (index !== undefined && !Array.isArray(index)) + {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 + if ($ogrows !== null) { + $code .= 'if (index !== undefined && index.length !== ' . $ogrows + 1 . ') + {stack_js.display_error("' . stack_string('stackBlock_incorrect_index_length') . '");}' . "\n"; + } + + // Index cannot be used in proof mode due to styling issues + if ($proofmode) { + $code .= 'if (index !== undefined) + {stack_js.display_error("' . stack_string('stackBlock_proof_mode_index') . '");}' . "\n"; + } // Link up to STACK inputs. if (count($inputs) > 0) { @@ -332,11 +367,6 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block { &$errors = [], $options = [] ): bool { - // TO-DO : matching validations: (1) check values of transpose ('true' or 'false'); (2) check values of rows/columns - // (only string containing ints); (3) check rows is not passed without columns; (4) check length of headers - // matches columns (think this has to be in js); (5) check index length matches length of rows if rows passed - // (again js probably). - // Basically, check that the dimensions have units we know. // Also that the references make sense. $valid = true; @@ -344,8 +374,7 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block { $height = array_key_exists('height', $this->params) ? $this->params['height'] : '400px'; // NOTE! List ordered by length. For the trimming logic. - $validunits = [ - + $validunits = [ 'vmin', 'vmax', 'rem', 'em', 'ex', 'px', 'cm', 'mm', 'in', 'pt', 'pc', 'ch', 'vh', 'vw', '%', ]; @@ -423,6 +452,36 @@ class stack_cas_castext2_parsons extends stack_cas_castext2_block { } } + // Check value of transpose is only "true" or "false" + if (array_key_exists('transpose', $this->params)) { + if (!in_array($this->params['transpose'], ['true', 'false'])) { + $valid = false; + $err[] = stack_string('stackBlock_parsons_unknown_transpose_value'); + } + } + + // Check value of columns is a string containing a numeric integer + if (array_key_exists("columns", $this->params)) { + if (!(preg_match('/^\d+$/', $this->params["columns"]) && intval($this->params["columns"]) > 0)) { + $valid = false; + $err[] = stack_string("stackBlock_parsons_invalid_columns_value"); + } + } + + // Check value of rows is a string containing a numeric integer + if (array_key_exists("rows", $this->params)) { + if (!(preg_match('/^\d+$/', $this->params["rows"]) && intval($this->params["rows"]) > 0)) { + $valid = false; + $err[] = stack_string("stackBlock_parsons_invalid_rows_value"); + } + } + + // Check we cannot have rows specified without columns + if (array_key_exists("rows", $this->params) && !array_key_exists("columns", $this->params)) { + $valid = false; + $err[] = stack_string("stackBlock_parsons_underdefined_grid"); + } + // Check that only valid parameters are passed to block header. $valids = null; foreach ($this->params as $key => $value) {