From fb651434e0716db7c41bdae645cfb2f7124195e5 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Fri, 17 Feb 2023 11:56:26 +0800 Subject: [PATCH] MDL-76849 qtype: Add a questionidentifier instance variable * Add an instance variable to question_display_options to store the identifier associated with the question being rendered. * This information can be used by question type plugins to improve the accessibility of the answer fields being rendered by adding the question identifier to the answer fields' labels. * Adding the question identifier to the label can be achieved by using question_display_options::add_question_identifier_to_label(). Co-authored-by: Tim Hunt --- question/engine/lib.php | 46 +++++++++ question/engine/renderer.php | 29 +++++- .../tests/question_display_options_test.php | 97 +++++++++++++++++++ question/engine/upgrade.txt | 10 ++ 4 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 question/engine/tests/question_display_options_test.php diff --git a/question/engine/lib.php b/question/engine/lib.php index 0b06d492005..3824dce2324 100644 --- a/question/engine/lib.php +++ b/question/engine/lib.php @@ -639,6 +639,20 @@ class question_display_options { */ public $userinfoinhistory = self::HIDDEN; + /** + * This identifier should be added to the labels of all input fields in the question. + * + * This is so people using assistive technology can easily tell which input belong to + * which question. The helper {@see self::add_question_identifier_to_label() makes this easier. + * + * If not set before the question is rendered, then it defaults to 'Question N'. + * (lang string) + * + * @var string The identifier that the question being rendered is associated with. + * E.g. The question number when it is rendered on a quiz. + */ + public $questionidentifier = null; + /** * Set all the feedback-related fields {@link $feedback}, {@link generalfeedback}, * {@link rightanswer} and {@link manualcomment} to @@ -669,6 +683,38 @@ class question_display_options { } return $options; } + + /** + * Helper to add the question identify (if there is one) to the label of an input field in a question. + * + * @param string $label The plain field label. E.g. 'Answer 1' + * @param bool $sridentifier If true, the question identifier, if added, will be wrapped in a sr-only span. Default false. + * @param bool $addbefore If true, the question identifier will be added before the label. + * @return string The amended label. For example 'Answer 1, Question 1'. + */ + public function add_question_identifier_to_label(string $label, bool $sridentifier = false, bool $addbefore = false): string { + if (!$this->has_question_identifier()) { + return $label; + } + $identifier = $this->questionidentifier; + if ($sridentifier) { + $identifier = html_writer::span($identifier, 'sr-only'); + } + $fieldlang = 'fieldinquestion'; + if ($addbefore) { + $fieldlang = 'fieldinquestionpre'; + } + return get_string($fieldlang, 'question', (object)['fieldname' => $label, 'questionindentifier' => $identifier]); + } + + /** + * Whether a question number has been provided for the question that is being displayed. + * + * @return bool + */ + public function has_question_identifier(): bool { + return $this->questionidentifier !== null && trim($this->questionidentifier) !== ''; + } } diff --git a/question/engine/renderer.php b/question/engine/renderer.php index b2f77b7e5f9..fe78e0099ae 100644 --- a/question/engine/renderer.php +++ b/question/engine/renderer.php @@ -80,6 +80,12 @@ class core_question_renderer extends plugin_renderer_base { public function question(question_attempt $qa, qbehaviour_renderer $behaviouroutput, qtype_renderer $qtoutput, question_display_options $options, $number) { + // If not already set, record the questionidentifier. + $options = clone($options); + if (!$options->has_question_identifier()) { + $options->questionidentifier = $this->question_number_text($number); + } + $output = ''; $output .= html_writer::start_tag('div', array( 'id' => $qa->get_outer_question_div_unique_id(), @@ -152,16 +158,35 @@ class core_question_renderer extends plugin_renderer_base { if (trim($number ?? '') === '') { return ''; } - $numbertext = ''; if (trim($number) === 'i') { $numbertext = get_string('information', 'question'); } else { $numbertext = get_string('questionx', 'question', - html_writer::tag('span', $number, array('class' => 'qno'))); + html_writer::tag('span', s($number), array('class' => 'qno'))); } return html_writer::tag('h3', $numbertext, array('class' => 'no')); } + /** + * Get the question number as a string. + * + * @param string|null $number e.g. '123' or 'i'. null or '' means do not display anything number-related. + * @return string e.g. 'Question 123' or 'Information' or ''. + */ + protected function question_number_text(?string $number): string { + $number = $number ?? ''; + // Trim the question number of whitespace, including  . + $trimmed = trim(html_entity_decode($number), " \n\r\t\v\x00\xC2\xA0"); + if ($trimmed === '') { + return ''; + } + if (trim($number) === 'i') { + return get_string('information', 'question'); + } else { + return get_string('questionx', 'question', s($number)); + } + } + /** * Add an invisible heading like 'question text', 'feebdack' at the top of * a section's contents, but only if the section has some content. diff --git a/question/engine/tests/question_display_options_test.php b/question/engine/tests/question_display_options_test.php new file mode 100644 index 00000000000..1923c7bc27d --- /dev/null +++ b/question/engine/tests/question_display_options_test.php @@ -0,0 +1,97 @@ +. + +namespace core_question; + +/** + * Unit tests for {@see \question_display_options}. + * + * @coversDefaultClass \question_display_options + * @package core_question + * @category test + * @copyright 2023 Jun Pataleta + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_display_options_test extends \advanced_testcase { + + /** + * Data provider for {@see self::test_has_question_identifier()} + * + * @return array[] + */ + public function has_question_identifier_provider(): array { + return [ + 'Empty string' => ['', false], + 'Empty space' => [' ', false], + 'Null' => [null, false], + 'Non-empty string' => ["Hello!", true], + ]; + } + + /** + * Tests for {@see \question_display_options::has_question_identifier} + * + * @covers ::has_question_identifier + * @dataProvider has_question_identifier_provider + * @param string|null $identifier The question identifier + * @param bool $expected The expected return value + * @return void + */ + public function test_has_question_identifier(?string $identifier, bool $expected): void { + $options = new \question_display_options(); + $options->questionidentifier = $identifier; + $this->assertEquals($expected, $options->has_question_identifier()); + } + + /** + * Data provider for {@see self::test_add_question_identifier_to_label() + * + * @return array[] + */ + public function add_question_identifier_to_label_provider(): array { + return [ + 'Empty string identifier' => ['Hello', '', false, false, "Hello"], + 'Null identifier' => ['Hello', null, false, false, "Hello"], + 'With identifier' => ['Hello', 'World', false, false, "Hello World"], + 'With identifier, sr-only' => ['Hello', 'World', true, false, 'Hello World'], + 'With identifier, prepend' => ['Hello', 'World', false, true, "World Hello"], + ]; + } + + /** + * Tests for {@see \question_display_options::add_question_identifier_to_label()} + * + * @covers ::add_question_identifier_to_label + * @dataProvider add_question_identifier_to_label_provider + * @param string $label The label string. + * @param string|null $identifier The question identifier. + * @param bool $sronly Whether to render the question identifier in a sr-only container + * @param bool $addbefore Whether to render the question identifier before the label. + * @param string $expected The expected return value. + * @return void + */ + public function test_add_question_identifier_to_label( + string $label, + ?string $identifier, + bool $sronly, + bool $addbefore, + string $expected + ): void { + $options = new \question_display_options(); + $options->questionidentifier = $identifier; + $this->assertEquals($expected, $options->add_question_identifier_to_label($label, $sronly, $addbefore)); + } +} diff --git a/question/engine/upgrade.txt b/question/engine/upgrade.txt index 357485fb19d..28811c88a32 100644 --- a/question/engine/upgrade.txt +++ b/question/engine/upgrade.txt @@ -1,5 +1,15 @@ This files describes API changes for the core question engine. +=== 4.2 === + +* A `$questionidentifier` property has been added to `\question_display_options` to enable question type plugins to associate the +question number to the question that is being rendered. This can be used to improve the accessibility of rendered +questions and can be especially be helpful for screen reader users as adding the question number on the answer field(s) labels +will allow them to distinguish between answer fields as they navigate through a quiz. +* Question type plugins can use \question_display_options::add_question_identifier_to_label() to add the question number to the +label of the answer field(s) of the question that is being rendered. The question number may be redundant when displayed, so the +function allows for it to be enclosed within an sr-only container. + === 4.0 === 1) A new optional parameter $extraselect has been added as a part of load_questions_usages_where_question_in_state() method in question/engine/datalib.php, anything passed here will be added to the SELECT list, use this to return extra data.