From ef204535d0cb1ce3e858bed24b4fd78d781cf312 Mon Sep 17 00:00:00 2001 From: Maximilian Haye Date: Thu, 14 Sep 2023 15:52:23 +0200 Subject: [PATCH] feat: replace HTML validation attrs with soft validation We mustn't prevent form submission, but we still want to give some feedback to the user. Closes #57 --- amd/build/view_question.min.js | 3 + amd/build/view_question.min.js.map | 1 + amd/src/view_question.js | 101 ++++++++++++++++++ classes/question_metadata.php | 31 +++++- classes/question_ui_renderer.php | 53 +++++++++ question.php | 13 ++- renderer.php | 14 ++- styles.css | 4 + tests/question_ui_renderer_test.php | 72 ++++++++++--- tests/question_uis/ids_and_names.xhtml | 27 +++++ .../{inputs.xhtml => metadata.xhtml} | 4 +- tests/question_uis/parameters.xhtml | 17 --- tests/question_uis/validations.xhtml | 9 ++ 13 files changed, 312 insertions(+), 37 deletions(-) create mode 100644 amd/build/view_question.min.js create mode 100644 amd/build/view_question.min.js.map create mode 100644 amd/src/view_question.js create mode 100644 tests/question_uis/ids_and_names.xhtml rename tests/question_uis/{inputs.xhtml => metadata.xhtml} (79%) create mode 100644 tests/question_uis/validations.xhtml diff --git a/amd/build/view_question.min.js b/amd/build/view_question.min.js new file mode 100644 index 00000000..192e388f --- /dev/null +++ b/amd/build/view_question.min.js @@ -0,0 +1,3 @@ +define("qtype_questionpy/view_question",["exports","jquery","theme_boost/bootstrap/popover"],(function(_exports,_jquery,_popover){var obj;function markInvalid(element){let ariaInvalid=!(arguments.length>1&&void 0!==arguments[1])||arguments[1];element.classList.add("qpy-invalid"),ariaInvalid?element.setAttribute("aria-invalid","true"):element.removeAttribute("aria-invalid"),element.dataset.toggle="popover",element.dataset.trigger="hover",element.dataset.content="TODO: strings",(0,_jquery.default)(element).popover()}function checkConditions(element){if(void 0!==element.dataset.qpy_required&&(null===element.value||""===element.value))return void markInvalid(element,!1);const pattern=element.dataset.qpy_pattern;if(void 0!==pattern&&null!==element.value&&""!==element.value&&!element.value.match("^(?:".concat(pattern,")$")))return void markInvalid(element);const minLength=element.dataset.qpy_minlength;if(void 0!==minLength&&null!==element.value&&""!==element.value&&element.value.lengthparseInt(maxLength)?markInvalid(element):function(element){element.classList.remove("qpy-invalid"),element.removeAttribute("aria-invalid"),delete element.dataset.toggle,delete element.dataset.trigger,delete element.dataset.content,(0,_jquery.default)(element).popover("dispose")}(element)}Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.init=function(){document.querySelectorAll("[data-qpy_required], [data-qpy_pattern], [data-qpy_minlength], [data-qpy_maxlength]").forEach((element=>{checkConditions(element),element.addEventListener("change",(event=>checkConditions(event.target)))}))},_jquery=(obj=_jquery)&&obj.__esModule?obj:{default:obj}})); + +//# sourceMappingURL=view_question.min.js.map \ No newline at end of file diff --git a/amd/build/view_question.min.js.map b/amd/build/view_question.min.js.map new file mode 100644 index 00000000..db1ddeb0 --- /dev/null +++ b/amd/build/view_question.min.js.map @@ -0,0 +1 @@ +{"version":3,"file":"view_question.min.js","sources":["../src/view_question.js"],"sourcesContent":["/*\n * This file is part of the QuestionPy Moodle plugin - https://questionpy.org\n *\n * Moodle is free software: you can redistribute it and/or modify\n * it under the terms of the GNU General Public License as published by\n * the Free Software Foundation, either version 3 of the License, or\n * (at your option) any later version.\n *\n * Moodle is distributed in the hope that it will be useful,\n * but WITHOUT ANY WARRANTY; without even the implied warranty of\n * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n * GNU General Public License for more details.\n *\n * You should have received a copy of the GNU General Public License\n * along with Moodle. If not, see .\n */\n\nimport $ from \"jquery\";\nimport \"theme_boost/bootstrap/popover\";\n\n/**\n *\n * @param {HTMLInputElement} element\n * @param {boolean} ariaInvalid\n */\nfunction markInvalid(element, ariaInvalid = true) {\n element.classList.add(\"qpy-invalid\");\n if (ariaInvalid) {\n element.setAttribute(\"aria-invalid\", \"true\");\n } else {\n element.removeAttribute(\"aria-invalid\");\n }\n\n // See https://getbootstrap.com/docs/4.0/components/popovers/.\n element.dataset.toggle = \"popover\";\n element.dataset.trigger = \"hover\";\n element.dataset.content = \"TODO: strings\";\n $(element).popover();\n}\n\n/**\n *\n * @param {HTMLInputElement} element\n */\nfunction unmarkInvalid(element) {\n element.classList.remove(\"qpy-invalid\");\n element.removeAttribute(\"aria-invalid\");\n\n delete element.dataset.toggle;\n delete element.dataset.trigger;\n delete element.dataset.content;\n $(element).popover(\"dispose\");\n}\n\n/**\n * Softly (i.e. without preventing form submission) validates required and/or pattern conditions on the given element.\n *\n * @param {HTMLInputElement} element\n */\nfunction checkConditions(element) {\n if (element.dataset.qpy_required !== undefined) {\n if (element.value === null || element.value === \"\") {\n markInvalid(element, false);\n return;\n }\n }\n\n const pattern = element.dataset.qpy_pattern;\n if (pattern !== undefined && element.value !== null && element.value !== \"\"\n && !element.value.match(`^(?:${pattern})$`)) {\n markInvalid(element);\n return;\n }\n\n const minLength = element.dataset.qpy_minlength;\n if (minLength !== undefined && element.value !== null && element.value !== \"\"\n && element.value.length < parseInt(minLength)) {\n markInvalid(element);\n return;\n }\n\n const maxLength = element.dataset.qpy_maxlength;\n if (maxLength !== undefined && element.value !== null && element.value !== \"\"\n && element.value.length > parseInt(maxLength)) {\n markInvalid(element);\n return;\n }\n\n unmarkInvalid(element);\n}\n\n/**\n * Adds change event handlers for soft validation.\n */\nexport function init() {\n document.querySelectorAll(\"[data-qpy_required], [data-qpy_pattern], [data-qpy_minlength], [data-qpy_maxlength]\")\n .forEach(element => {\n checkConditions(element);\n element.addEventListener(\"change\", event => checkConditions(event.target));\n });\n}\n"],"names":["markInvalid","element","ariaInvalid","classList","add","setAttribute","removeAttribute","dataset","toggle","trigger","content","popover","checkConditions","undefined","qpy_required","value","pattern","qpy_pattern","match","minLength","qpy_minlength","length","parseInt","maxLength","qpy_maxlength","remove","unmarkInvalid","document","querySelectorAll","forEach","addEventListener","event","target"],"mappings":"mJAyBSA,YAAYC,aAASC,uEAC1BD,QAAQE,UAAUC,IAAI,eAClBF,YACAD,QAAQI,aAAa,eAAgB,QAErCJ,QAAQK,gBAAgB,gBAI5BL,QAAQM,QAAQC,OAAS,UACzBP,QAAQM,QAAQE,QAAU,QAC1BR,QAAQM,QAAQG,QAAU,oCACxBT,SAASU,mBAsBNC,gBAAgBX,iBACgBY,IAAjCZ,QAAQM,QAAQO,eACM,OAAlBb,QAAQc,OAAoC,KAAlBd,QAAQc,mBAClCf,YAAYC,SAAS,SAKvBe,QAAUf,QAAQM,QAAQU,oBAChBJ,IAAZG,SAA2C,OAAlBf,QAAQc,OAAoC,KAAlBd,QAAQc,QACvDd,QAAQc,MAAMG,oBAAaF,2BAC/BhB,YAAYC,eAIVkB,UAAYlB,QAAQM,QAAQa,sBAChBP,IAAdM,WAA6C,OAAlBlB,QAAQc,OAAoC,KAAlBd,QAAQc,OAC1Dd,QAAQc,MAAMM,OAASC,SAASH,uBACnCnB,YAAYC,eAIVsB,UAAYtB,QAAQM,QAAQiB,mBAChBX,IAAdU,WAA6C,OAAlBtB,QAAQc,OAAoC,KAAlBd,QAAQc,OAC1Dd,QAAQc,MAAMM,OAASC,SAASC,WACnCvB,YAAYC,kBAxCGA,SACnBA,QAAQE,UAAUsB,OAAO,eACzBxB,QAAQK,gBAAgB,uBAEjBL,QAAQM,QAAQC,cAChBP,QAAQM,QAAQE,eAChBR,QAAQM,QAAQG,4BACrBT,SAASU,QAAQ,WAqCnBe,CAAczB,0FAOd0B,SAASC,iBAAiB,uFACrBC,SAAQ5B,UACLW,gBAAgBX,SAChBA,QAAQ6B,iBAAiB,UAAUC,OAASnB,gBAAgBmB,MAAMC"} \ No newline at end of file diff --git a/amd/src/view_question.js b/amd/src/view_question.js new file mode 100644 index 00000000..f89f1f02 --- /dev/null +++ b/amd/src/view_question.js @@ -0,0 +1,101 @@ +/* + * This file is part of the QuestionPy Moodle plugin - https://questionpy.org + * + * Moodle is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Moodle is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Moodle. If not, see . + */ + +import $ from "jquery"; +import "theme_boost/bootstrap/popover"; + +/** + * + * @param {HTMLInputElement} element + * @param {boolean} ariaInvalid + */ +function markInvalid(element, ariaInvalid = true) { + element.classList.add("qpy-invalid"); + if (ariaInvalid) { + element.setAttribute("aria-invalid", "true"); + } else { + element.removeAttribute("aria-invalid"); + } + + // See https://getbootstrap.com/docs/4.0/components/popovers/. + element.dataset.toggle = "popover"; + element.dataset.trigger = "hover"; + element.dataset.content = "TODO: strings"; + $(element).popover(); +} + +/** + * + * @param {HTMLInputElement} element + */ +function unmarkInvalid(element) { + element.classList.remove("qpy-invalid"); + element.removeAttribute("aria-invalid"); + + delete element.dataset.toggle; + delete element.dataset.trigger; + delete element.dataset.content; + $(element).popover("dispose"); +} + +/** + * Softly (i.e. without preventing form submission) validates required and/or pattern conditions on the given element. + * + * @param {HTMLInputElement} element + */ +function checkConditions(element) { + if (element.dataset.qpy_required !== undefined) { + if (element.value === null || element.value === "") { + markInvalid(element, false); + return; + } + } + + const pattern = element.dataset.qpy_pattern; + if (pattern !== undefined && element.value !== null && element.value !== "" + && !element.value.match(`^(?:${pattern})$`)) { + markInvalid(element); + return; + } + + const minLength = element.dataset.qpy_minlength; + if (minLength !== undefined && element.value !== null && element.value !== "" + && element.value.length < parseInt(minLength)) { + markInvalid(element); + return; + } + + const maxLength = element.dataset.qpy_maxlength; + if (maxLength !== undefined && element.value !== null && element.value !== "" + && element.value.length > parseInt(maxLength)) { + markInvalid(element); + return; + } + + unmarkInvalid(element); +} + +/** + * Adds change event handlers for soft validation. + */ +export function init() { + document.querySelectorAll("[data-qpy_required], [data-qpy_pattern], [data-qpy_minlength], [data-qpy_maxlength]") + .forEach(element => { + checkConditions(element); + element.addEventListener("change", event => checkConditions(event.target)); + }); +} diff --git a/classes/question_metadata.php b/classes/question_metadata.php index 0737b26c..74586552 100644 --- a/classes/question_metadata.php +++ b/classes/question_metadata.php @@ -38,15 +38,44 @@ class question_metadata { */ public array $expecteddata = []; + /** + * @var string[] an array of required field names + * @see \question_manually_gradable::is_complete_response() + * @see \question_manually_gradable::is_gradable_response() + */ + public array $requiredfields = []; + + /** + * @var int[] a mapping of field names to minimum string length + * @see \question_manually_gradable::is_gradable_response() + * @link https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/minlength + */ + public array $minlengths = []; + + /** + * @var int[] a mapping of field names to maximum string length + * @see \question_manually_gradable::is_gradable_response() + * @link https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/maxlength + */ + public array $maxlengths = []; + /** * Initializes a new instance. * * @param array|null $correctresponse if known, an array of `name => correct_value` entries for the expected * response fields * @param array $expecteddata an array of `name => PARAM_X` entries for the expected response fields + * @param string[] $requiredfields an array of required field names + * @param int[] $minlengths a mapping of field names to minimum string length + * @param int[] $maxlengths a mapping of field names to maximum string length */ - public function __construct(?array $correctresponse = null, array $expecteddata = []) { + public function __construct(?array $correctresponse = null, array $expecteddata = [], + array $requiredfields = [], array $minlengths = [], array $maxlengths = []) + { $this->correctresponse = $correctresponse; $this->expecteddata = $expecteddata; + $this->requiredfields = $requiredfields; + $this->minlengths = $minlengths; + $this->maxlengths = $maxlengths; } } diff --git a/classes/question_ui_renderer.php b/classes/question_ui_renderer.php index 7eec55ee..118d21c8 100644 --- a/classes/question_ui_renderer.php +++ b/classes/question_ui_renderer.php @@ -182,6 +182,20 @@ public function get_metadata(): question_metadata { $name = $element->getAttribute("name"); if ($name) { $this->metadata->expecteddata[$name] = PARAM_RAW; + + if ($element->hasAttribute("required")) { + $this->metadata->requiredfields[] = $name; + } + + $minlength = $element->getAttribute("minlength"); + if (is_numeric($minlength)) { + $this->metadata->minlengths[$name] = intval($minlength); + } + + $maxlength = $element->getAttribute("maxlength"); + if (is_numeric($maxlength)) { + $this->metadata->maxlengths[$name] = intval($maxlength); + } } } } @@ -215,6 +229,7 @@ private function render_part(DOMNode $part, question_attempt $qa, ?question_disp try { $this->hide_unwanted_feedback($xpath, $options); $this->set_input_values_and_readonly($xpath, $qa, $options); + $this->soften_validation($xpath); $this->shuffle_contents($xpath); $this->mangle_ids_and_names($xpath, $qa); $this->clean_up($xpath); @@ -449,4 +464,42 @@ private function resolve_placeholders(DOMXPath $xpath): void { } } } + + /** + * Replaces the HTML attributes `pattern`, `required`, `minlength`, `maxlength` so that submission is not prevented. + * + * The standard attributes are replaced with `data-qpy_X`, which are then evaluated in JS. + * + * @param DOMXPath $xpath + * @return void + */ + private function soften_validation(DOMXPath $xpath): void + { + /** @var DOMElement $element */ + foreach ($xpath->query("//xhtml:input[@pattern]") as $element) { + $pattern = $element->getAttribute("pattern"); + $element->removeAttribute("pattern"); + $element->setAttribute("data-qpy_pattern", $pattern); + } + + foreach ($xpath->query("(//xhtml:input | //xhtml:select | //xhtml:textarea)[@required]") as $element) { + $element->removeAttribute("required"); + $element->setAttribute("data-qpy_required", "data-qpy_required"); + $element->setAttribute("aria-required", "true"); + } + + foreach ($xpath->query("(//xhtml:input | //xhtml:textarea)[@minlength or @maxlength]") as $element) { + $minlength = $element->getAttribute("minlength"); + if ($minlength !== "") { + $element->removeAttribute("minlength"); + $element->setAttribute("data-qpy_minlength", $minlength); + } + + $maxlength = $element->getAttribute("maxlength"); + if ($maxlength !== "") { + $element->removeAttribute("maxlength"); + $element->setAttribute("data-qpy_maxlength", $maxlength); + } + } + } } diff --git a/question.php b/question.php index e3b4609b..2b8ecd28 100644 --- a/question.php +++ b/question.php @@ -83,12 +83,13 @@ public function __construct(string $packagehash, string $questionstate) { public function start_attempt(question_attempt_step $step, $variant): void { $attempt = $this->api->start_attempt($this->packagehash, $this->questionstate, $variant); + $step->set_qt_var(self::QT_VAR_ATTEMPT_STATE, $attempt->attemptstate); + // We generate a fixed seed to be used during every render of the attempt, to keep shuffles deterministic. $mtseed = mt_rand(); $step->set_qt_var(self::QT_VAR_MT_SEED, $mtseed); $this->ui = new question_ui_renderer($attempt->ui->content, $attempt->ui->parameters, $mtseed); - $step->set_qt_var(self::QT_VAR_ATTEMPT_STATE, $attempt->attemptstate); } /** @@ -154,7 +155,13 @@ public function get_correct_response(): ?array { * {@see question_attempt_step::get_qt_data()}. * @return bool whether this response is a complete answer to this question. */ - public function is_complete_response(array $response) { + public function is_complete_response(array $response): bool + { + foreach ($this->ui->get_metadata()->requiredfields as $requiredfield) { + if (!isset($response[$requiredfield]) || $response[$requiredfield] === "") { + return false; + } + } return true; } @@ -165,7 +172,7 @@ public function is_complete_response(array $response) { * * @param array $prevresponse the responses previously recorded for this question, * as returned by {@see question_attempt_step::get_qt_data()} - * @param array $newresponse the new responses, in the same format. + * @param array $newresponse the new responses, in the same format. * @return bool whether the two sets of responses are the same - that is * whether the new set of responses can safely be discarded. */ diff --git a/renderer.php b/renderer.php index ca5aa416..73891e3f 100644 --- a/renderer.php +++ b/renderer.php @@ -30,13 +30,25 @@ */ class qtype_questionpy_renderer extends qtype_renderer { + /** + * Return any HTML that needs to be included in the page's when this + * question is used. + * @param question_attempt $qa the question attempt that will be displayed on the page. + * @return string HTML fragment. + */ + public function head_code(question_attempt $qa) { + global $PAGE; + $PAGE->requires->js_call_amd("qtype_questionpy/view_question", "init"); + return parent::head_code($qa); + } + /** * Generate the display of the formulation part of the question. This is the * area that contains the quetsion text, and the controls for students to * input their answers. Some question types also embed bits of feedback, for * example ticks and crosses, in this area. * - * @param question_attempt $qa the question attempt to display. + * @param question_attempt $qa the question attempt to display. * @param question_display_options $options controls what should and should not be displayed. * @return string HTML fragment. * @throws coding_exception diff --git a/styles.css b/styles.css index 7929cab8..a997b660 100644 --- a/styles.css +++ b/styles.css @@ -73,3 +73,7 @@ .qpy-repetition-remove { margin: .5em; } + +.qpy-invalid { + border-color: red; +} diff --git a/tests/question_ui_renderer_test.php b/tests/question_ui_renderer_test.php index 6916c893..82e51285 100644 --- a/tests/question_ui_renderer_test.php +++ b/tests/question_ui_renderer_test.php @@ -36,7 +36,7 @@ class question_ui_renderer_test extends \advanced_testcase { * @covers \qtype_questionpy\question_metadata */ public function test_should_extract_correct_metadata() { - $input = file_get_contents(__DIR__ . "/question_uis/inputs.xhtml"); + $input = file_get_contents(__DIR__ . "/question_uis/metadata.xhtml"); $ui = new question_ui_renderer($input, [], mt_rand()); $metadata = $ui->get_metadata(); @@ -51,8 +51,14 @@ public function test_should_extract_correct_metadata() { "my_select" => PARAM_RAW, "my_radio" => PARAM_RAW, "my_text" => PARAM_RAW, - "my_button" => PARAM_RAW - ]), $metadata); + "my_button" => PARAM_RAW, + "only_lowercase_letters" => PARAM_RAW, + "between_5_and_10_chars" => PARAM_RAW + ], + ["my_number"], + ["between_5_and_10_chars" => 5], + ["between_5_and_10_chars" => 10] + ), $metadata); } /** @@ -211,7 +217,7 @@ public function test_should_throw_when_formulation_is_missing() { * @covers \qtype_questionpy\question_ui_renderer */ public function test_should_mangle_names() { - $input = file_get_contents(__DIR__ . "/question_uis/inputs.xhtml"); + $input = file_get_contents(__DIR__ . "/question_uis/ids_and_names.xhtml"); $ui = new question_ui_renderer($input, [], mt_rand()); $qa = $this->createStub(\question_attempt::class); @@ -224,16 +230,26 @@ public function test_should_mangle_names() { $this->assertXmlStringEqualsXmlString(<< - - - One - Two -