Skip to content

Commit

Permalink
MDL-53304 question behaviour: don't show useless Check buttons
Browse files Browse the repository at this point in the history 
Previously, the Check button was often shown disabled when it
could not be used (e.g. when the question was finished, or when an
interactive question was in the try-again state). Eventually we
realised it was better usability to hide it in these cases.

Note that when a teacher reviews an in-progress quiz attempt, they will
see a disabled Check button if the student doing the quiz can see the
button.
  • Loading branch information
@timhunt
timhunt committed Mar 4, 2016
1 parent 7adc7ef commit 81e47a3
Show file tree
Hide file tree
Showing 19 changed files with 92 additions and 60 deletions.
16 changes: 8 additions & 8 deletions question/behaviour/adaptive/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public function test_adaptive_multichoice2() {
$this->check_current_mark(2);
$this->check_current_output(
$this->get_contains_mark_summary(2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation());
}

Expand Down Expand Up @@ -310,7 +310,7 @@ public function test_adaptive_shortanswer_partially_right() {
$this->check_current_mark(0.8);
$this->check_current_output(
$this->get_contains_mark_summary(0.8),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
Expand Down Expand Up @@ -393,7 +393,7 @@ public function test_adaptive_shortanswer_wrong_right_wrong() {
$this->check_current_mark(4.00);
$this->check_current_output(
$this->get_contains_mark_summary(4.00),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_incorrect_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
Expand Down Expand Up @@ -475,7 +475,7 @@ public function test_adaptive_shortanswer_invalid_after_complete() {
$this->check_current_mark(0.66666667);
$this->check_current_output(
$this->get_contains_mark_summary(0.67),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_incorrect_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
Expand Down Expand Up @@ -532,7 +532,7 @@ public function test_adaptive_shortanswer_zero_penalty() {
$this->check_current_mark(1.0);
$this->check_current_output(
$this->get_contains_mark_summary(1.0),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
Expand Down Expand Up @@ -646,7 +646,7 @@ public function test_adaptive_numerical() {
$this->check_current_mark(1);
$this->check_current_output(
$this->get_contains_mark_summary(1),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_incorrect_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
Expand Down Expand Up @@ -749,7 +749,7 @@ public function test_adaptive_numerical_invalid() {
$this->check_current_mark(0.9);
$this->check_current_output(
$this->get_contains_mark_summary(0.9),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_incorrect_expectation(),
$this->get_does_not_contain_validation_error_expectation(),
$this->get_does_not_contain_disregarded_info_expectation());
Expand Down Expand Up @@ -848,7 +848,7 @@ public function test_adaptive_multianswer() {
$this->check_output_contains_text_input_with_class('sub1_answer', 'correct');
$this->check_current_output(
$this->get_contains_mark_summary(8.00),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
Expand Down
4 changes: 2 additions & 2 deletions question/behaviour/adaptivenopenalty/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public function test_multichoice2() {
$this->check_current_mark(2);
$this->check_current_output(
$this->get_contains_mark_summary(2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation());
}

Expand Down Expand Up @@ -305,7 +305,7 @@ public function test_numerical_invalid() {
$this->check_current_mark(1.0);
$this->check_current_output(
$this->get_contains_mark_summary(1.0),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_incorrect_expectation(),
$this->get_does_not_contain_validation_error_expectation());
}
Expand Down
3 changes: 3 additions & 0 deletions question/behaviour/interactive/renderer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
*/
class qbehaviour_interactive_renderer extends qbehaviour_renderer {
public function controls(question_attempt $qa, question_display_options $options) {
if ($options->readonly === qbehaviour_interactive::READONLY_EXCEPT_TRY_AGAIN) {
return '';
}
return $this->submit_button($qa, $options);
}

Expand Down
12 changes: 6 additions & 6 deletions question/behaviour/interactive/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function test_interactive_feedback_multichoice_right() {
$this->get_contains_mc_radio_expectation($wrongindex, false, true),
$this->get_contains_mc_radio_expectation(($wrongindex + 1) % 3, false, false),
$this->get_contains_mc_radio_expectation(($wrongindex + 1) % 3, false, false),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_try_again_button_expectation(true),
$this->get_does_not_contain_correctness_expectation(),
new question_pattern_expectation('/Tries remaining: 2/'),
Expand Down Expand Up @@ -135,7 +135,7 @@ public function test_interactive_feedback_multichoice_right() {
$this->get_contains_mc_radio_expectation($rightindex, false, true),
$this->get_contains_mc_radio_expectation(($rightindex + 1) % 3, false, false),
$this->get_contains_mc_radio_expectation(($rightindex + 1) % 3, false, false),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_no_hint_visible_expectation());

Expand Down Expand Up @@ -219,7 +219,7 @@ public function test_interactive_finish_when_try_again_showing() {
$this->get_contains_mc_radio_expectation($wrongindex, false, true),
$this->get_contains_mc_radio_expectation(($wrongindex + 1) % 3, false, false),
$this->get_contains_mc_radio_expectation(($wrongindex + 1) % 3, false, false),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_try_again_button_expectation(true),
$this->get_does_not_contain_correctness_expectation(),
new question_pattern_expectation('/Tries remaining: 1/'),
Expand Down Expand Up @@ -283,7 +283,7 @@ public function test_interactive_shortanswer_try_to_submit_blank() {
$this->check_current_mark(null);
$this->check_current_output(
$this->get_contains_marked_out_of_summary(),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_does_not_contain_validation_error_expectation(),
$this->get_contains_try_again_button_expectation(true),
new question_pattern_expectation('/Tries remaining: 2/'),
Expand Down Expand Up @@ -327,7 +327,7 @@ public function test_interactive_shortanswer_try_to_submit_blank() {
$this->check_current_mark(0.6666667);
$this->check_current_output(
$this->get_contains_mark_summary(0.6666667),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_does_not_contain_validation_error_expectation(),
$this->get_no_hint_visible_expectation());
Expand Down Expand Up @@ -380,7 +380,7 @@ public function test_interactive_feedback_multichoice_multiple_reset() {
$this->get_contains_mc_checkbox_expectation($right[1], false, false),
$this->get_contains_mc_checkbox_expectation($wrong[0], false, true),
$this->get_contains_mc_checkbox_expectation($wrong[1], false, false),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_try_again_button_expectation(true),
$this->get_does_not_contain_correctness_expectation(),
new question_pattern_expectation('/Tries remaining: 2/'),
Expand Down
4 changes: 2 additions & 2 deletions question/behaviour/interactivecountback/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public function test_interactive_feedback_match_reset() {
$this->get_contains_select_expectation('sub1', $choices, $orderforchoice[1], false),
$this->get_contains_select_expectation('sub2', $choices, $orderforchoice[1], false),
$this->get_contains_select_expectation('sub3', $choices, $orderforchoice[1], false),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_try_again_button_expectation(true),
$this->get_does_not_contain_correctness_expectation(),
new question_pattern_expectation('/Tries remaining: 2/'),
Expand Down Expand Up @@ -139,7 +139,7 @@ public function test_interactive_feedback_match_reset() {
$this->get_contains_select_expectation('sub1', $choices, $orderforchoice[2], false),
$this->get_contains_select_expectation('sub2', $choices, $orderforchoice[2], false),
$this->get_contains_select_expectation('sub3', $choices, $orderforchoice[1], false),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_does_not_contain_try_again_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_contains_standard_correct_combined_feedback_expectation(),
Expand Down
3 changes: 3 additions & 0 deletions question/behaviour/rendererbase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ public function manual_comment(question_attempt $qa, question_display_options $o
* @return string HTML fragment.
*/
protected function submit_button(question_attempt $qa, question_display_options $options) {
if (!$qa->get_state()->is_active()) {
return '';
}
$attributes = array(
'type' => 'submit',
'id' => $qa->get_behaviour_field_name('submit'),
Expand Down
10 changes: 10 additions & 0 deletions question/behaviour/upgrade.txt
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
This files describes API changes for question behaviour plugins.

=== 3.1 ===

1) The standard behaviours that use a 'Check' button have all been changed so
that they only show the button when the question is active. Your behaviour
may interit this behaviour, because the change was made in the base class,
and this is probably good for consistency. However, if your question behaviour
uses the Check button, your probably want to test it carefully, and you will
probably have to update your unit tests. See MDL-53304 for more details.


=== 2.9 ===

1) There are new methods question_behaviour::can_finish_during_attempt and
Expand Down
16 changes: 16 additions & 0 deletions question/engine/tests/helpers.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1103,11 +1103,27 @@ protected function get_contains_button_expectation($name, $value = null, $enable
return new question_contains_tag_with_attributes('input', $expectedattributes, $forbiddenattributes);
}

/**
* Returns an epectation that a string contains the HTML of a button with
* name {question-attempt prefix}-submit, and eiter enabled or not.
* @param bool $enabled if not null, check the enabled/disabled state of the button. True = enabled.
* @return question_contains_tag_with_attributes an expectation for use with check_current_output.
*/
protected function get_contains_submit_button_expectation($enabled = null) {
return $this->get_contains_button_expectation(
$this->quba->get_field_prefix($this->slot) . '-submit', null, $enabled);
}

/**
* Returns an epectation that a string does not contain the HTML of a button with
* name {question-attempt prefix}-submit.
* @return question_contains_tag_with_attributes an expectation for use with check_current_output.
*/
protected function get_does_not_contain_submit_button_expectation() {
return new question_no_pattern_expectation('/name="' .
$this->quba->get_field_prefix($this->slot) . '-submit"/');
}

protected function get_tries_remaining_expectation($n) {
return new question_pattern_expectation('/' .
preg_quote(get_string('triesremaining', 'qbehaviour_interactive', $n), '/') . '/');
Expand Down
2 changes: 1 addition & 1 deletion question/type/calculated/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public function test_interactive() {
$this->check_current_mark(3);
$this->check_current_output(
$this->get_contains_mark_summary(3),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_does_not_contain_validation_error_expectation(),
$this->get_no_hint_visible_expectation());
Expand Down
2 changes: 1 addition & 1 deletion question/type/calculatedmulti/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public function test_interactive() {
$this->check_current_mark(3);
$this->check_current_output(
$this->get_contains_mark_summary(3),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_does_not_contain_validation_error_expectation(),
$this->get_no_hint_visible_expectation());
Expand Down
2 changes: 1 addition & 1 deletion question/type/calculatedsimple/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public function test_interactive() {
$this->check_current_mark(3);
$this->check_current_output(
$this->get_contains_mark_summary(3),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_does_not_contain_validation_error_expectation(),
$this->get_no_hint_visible_expectation());
Expand Down
14 changes: 7 additions & 7 deletions question/type/ddimageortext/tests/walkthrough_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public function test_interactive_behaviour() {
$this->quba->get_field_prefix($this->slot) . 'p3', '1'),
$this->get_contains_hidden_expectation(
$this->quba->get_field_prefix($this->slot) . 'p4', '2'),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_no_hint_visible_expectation());

Expand Down Expand Up @@ -469,7 +469,7 @@ public function test_interactive_grading() {
$this->get_contains_drag_image_home_expectation(2, 2, 1),
$this->get_contains_drag_image_home_expectation(3, 1, 2),
$this->get_contains_drag_image_home_expectation(4, 2, 2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_try_again_button_expectation(true),
$this->get_does_not_contain_correctness_expectation(),
$this->get_contains_hint_expectation('This is the first hint'),
Expand Down Expand Up @@ -530,7 +530,7 @@ public function test_interactive_grading() {
$this->get_contains_drag_image_home_expectation(2, 2, 1),
$this->get_contains_drag_image_home_expectation(3, 1, 2),
$this->get_contains_drag_image_home_expectation(4, 2, 2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_try_again_button_expectation(true),
$this->get_does_not_contain_correctness_expectation(),
$this->get_contains_hint_expectation('This is the second hint'),
Expand Down Expand Up @@ -592,7 +592,7 @@ public function test_interactive_grading() {
$this->quba->get_field_prefix($this->slot) . 'p3', 1),
$this->get_contains_hidden_expectation(
$this->quba->get_field_prefix($this->slot) . 'p4', 2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_does_not_contain_try_again_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_no_hint_visible_expectation(),
Expand Down Expand Up @@ -648,7 +648,7 @@ public function test_interactive_correct_no_submit() {
$this->get_contains_drag_image_home_expectation(2, 2, 1),
$this->get_contains_drag_image_home_expectation(3, 1, 2),
$this->get_contains_drag_image_home_expectation(4, 2, 2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_correct_expectation(),
$this->get_no_hint_visible_expectation());

Expand Down Expand Up @@ -710,7 +710,7 @@ public function test_interactive_partial_no_submit() {
$this->get_contains_drag_image_home_expectation(2, 2, 1),
$this->get_contains_drag_image_home_expectation(3, 1, 2),
$this->get_contains_drag_image_home_expectation(4, 2, 2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_partcorrect_expectation(),
$this->get_no_hint_visible_expectation());

Expand Down Expand Up @@ -769,7 +769,7 @@ public function test_interactive_no_right_clears() {
$this->get_contains_drag_image_home_expectation(2, 2, 1),
$this->get_contains_drag_image_home_expectation(3, 1, 2),
$this->get_contains_drag_image_home_expectation(4, 2, 2),
$this->get_contains_submit_button_expectation(false),
$this->get_does_not_contain_submit_button_expectation(),
$this->get_contains_hint_expectation('This is the first hint'));

// Do try again.
Expand Down
Loading

0 comments on commit 81e47a3

Please sign in to comment.