Moodle
  1. Moodle
  2. MDL-28259

Individual answers Feedback not shown in Cloze questions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.1.2
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      Create a Cloze question with this question text:

      The capital of France is

      {1:SHORTANSWER:%100%Paris#Congratulations, Paris is the capital of France!~%50%Marseille#No, that is the second largest city in France (after Paris).~*#Wrong answer. The capital of France is Paris, of course.}

      .

      Go to Preview question.
      In the "How questions behave" list, Select any behaviour that displays immediate feedback, e.g. Adaptive mode OR Immediate feedback.
      Enter "Paris" and check.
      Hovering the mouse over the response displays:

      Correct
      The correct answer is: Paris.
      Mark 1.00 out of 1.00

      it does not display the individual feedback message "Congratulations, Paris is the capital of France!" as expected!

      Show
      Create a Cloze question with this question text: The capital of France is {1:SHORTANSWER:%100%Paris#Congratulations, Paris is the capital of France!~%50%Marseille#No, that is the second largest city in France (after Paris).~*#Wrong answer. The capital of France is Paris, of course.} . Go to Preview question. In the "How questions behave" list, Select any behaviour that displays immediate feedback, e.g. Adaptive mode OR Immediate feedback. Enter "Paris" and check. Hovering the mouse over the response displays: Correct The correct answer is: Paris. Mark 1.00 out of 1.00 it does not display the individual feedback message "Congratulations, Paris is the capital of France!" as expected!
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      17960

      Description

      Individual answers Feedback not shown in Cloze questions when hovering over students' responses.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          This was working for me at the point when I thought I had finished the the Cloze questiontype, so I don't know what might have gone wrong.

          Show
          Tim Hunt added a comment - This was working for me at the point when I thought I had finished the the Cloze questiontype, so I don't know what might have gone wrong.
          Hide
          Pierre Pichet added a comment -

          The feedbacktext has been forgotten

              protected function feedback_popup(question_graded_automatically $subq,
                      $fraction, $feedbacktext, $rightanswer, question_display_options $options) {
          
                  if (!$options->feedback) {
                      return '';
                  }
          
                  $feedback = array();
                  if ($options->correctness) {
                      if (is_null($fraction)) {
                          $state = question_state::$gaveup;
                      } else {
                          $state = question_state::graded_state_for_fraction($fraction);
                      }
                      $feedback[] = $state->default_string(true);
                  if ($options->rightanswer) {
                      $feedback[] = get_string('correctansweris', 'qtype_shortanswer', $rightanswer);
                  }
          
                  $subfraction = '';
                  if ($options->marks >= question_display_options::MARK_AND_MAX && $subq->maxmark > 0) {
                      $a = new stdClass();
                      $a->mark = format_float($fraction * $subq->maxmark, $options->markdp);
                      $a->max =  format_float($subq->maxmark, $options->markdp);
                      $feedback[] = get_string('markoutofmax', 'question', $a);
                  }
          
                  return html_writer::tag('span', implode('<br />', $feedback),
                          array('class' => 'feedbackspan accesshide'));
          
          Show
          Pierre Pichet added a comment - The feedbacktext has been forgotten protected function feedback_popup(question_graded_automatically $subq, $fraction, $feedbacktext, $rightanswer, question_display_options $options) { if (!$options->feedback) { return ''; } $feedback = array(); if ($options->correctness) { if (is_null($fraction)) { $state = question_state::$gaveup; } else { $state = question_state::graded_state_for_fraction($fraction); } $feedback[] = $state->default_string(true); if ($options->rightanswer) { $feedback[] = get_string('correctansweris', 'qtype_shortanswer', $rightanswer); } $subfraction = ''; if ($options->marks >= question_display_options::MARK_AND_MAX && $subq->maxmark > 0) { $a = new stdClass(); $a->mark = format_float($fraction * $subq->maxmark, $options->markdp); $a->max = format_float($subq->maxmark, $options->markdp); $feedback[] = get_string('markoutofmax', 'question', $a); } return html_writer::tag('span', implode('<br />', $feedback), array('class' => 'feedbackspan accesshide'));
          Hide
          Joseph Rézeau added a comment -

          Thanks, Pierre.
          If we insert the missing feedbacktext here, it works fine:

          $feedback = array();
                  $feedback[] = $feedbacktext;
          
          Show
          Joseph Rézeau added a comment - Thanks, Pierre. If we insert the missing feedbacktext here, it works fine: $feedback = array(); $feedback[] = $feedbacktext;
          Hide
          Joseph Rézeau added a comment -

          Pierre, Tim, any progress on this bug?

          Show
          Joseph Rézeau added a comment - Pierre, Tim, any progress on this bug?
          Hide
          Joseph Rézeau added a comment -

          Any update on this one-month old bug with a very easy fix?
          Actually, it would be better to check for existence of a feedback text, so I suggest this fix:

          file question/type/multianswer/renderer.php line 125
          $feedback = array();
          if ($feedbacktext) {
              $feedback[] = $feedbacktext;
          }
          Show
          Joseph Rézeau added a comment - Any update on this one-month old bug with a very easy fix? Actually, it would be better to check for existence of a feedback text, so I suggest this fix: file question/type/multianswer/renderer.php line 125 $feedback = array(); if ($feedbacktext) { $feedback[] = $feedbacktext; }
          Hide
          Pierre Pichet added a comment -

          Tim return next week from his holidays.
          However if you can prepare a commit and tests results (even for such a little code change), this should help.

          Show
          Pierre Pichet added a comment - Tim return next week from his holidays. However if you can prepare a commit and tests results (even for such a little code change), this should help.
          Hide
          Joseph Rézeau added a comment -

          Now that Tim is back, I hope he will find the time to commit this very easy fix. It is really needed for the Cloze question type to display feedback.

          Show
          Joseph Rézeau added a comment - Now that Tim is back, I hope he will find the time to commit this very easy fix. It is really needed for the Cloze question type to display feedback.
          Hide
          Pierre Pichet added a comment -

          A proposal can be found at
          https://github.com/ppichet/moodle/commit/eff62bb3ecec297521db262ef60dfa88a96727b7

          Seems OK tested on the three variants.
          However the feedback display although similar to new engine regular question types does not clearly identify the feedback part as in preceeding versions which is apparently the new engine standards.

          Show
          Pierre Pichet added a comment - A proposal can be found at https://github.com/ppichet/moodle/commit/eff62bb3ecec297521db262ef60dfa88a96727b7 Seems OK tested on the three variants. However the feedback display although similar to new engine regular question types does not clearly identify the feedback part as in preceeding versions which is apparently the new engine standards.
          Hide
          Pierre Pichet added a comment -

          If the $feedbacktext != '' test is not done, there is an empty line when the feedback is null.

          Show
          Pierre Pichet added a comment - If the $feedbacktext != '' test is not done, there is an empty line when the feedback is null.
          Hide
          Joseph Rézeau added a comment -

          Thank you Pierre, Actually this question of displaying the specific feedback in the Cloze sub-questions (in Moodle 2.1) is more tricky than I thought at first. I am conducting exhaustive tests and will report in this discussion as soon as possible.

          Show
          Joseph Rézeau added a comment - Thank you Pierre, Actually this question of displaying the specific feedback in the Cloze sub-questions (in Moodle 2.1) is more tricky than I thought at first. I am conducting exhaustive tests and will report in this discussion as soon as possible.
          Hide
          Joseph Rézeau added a comment -

          Note: tested in teacher Preview question window. With Pierre's patch applied:

          if ($feedbacktext != ('')) {
              $feedback[] = $feedbacktext;
          }
          
          1.- displaying individual Feedback

          case : sub-question answer does not contain a feedback message
          settings: Behaviour: any;
          Whether correct: Not shown; Marks: Not shown; Specific feedback: Shown; Right answer: Not shown;
          bug: hovering shows small empty tooltip rather than nothing at all.
          suggested fix: check that the feedback() array is not empty before returning function data:
          line 150 add

          if (empty($feedback)) {
                      return "";
                  }
                  return html_writer::tag('span', implode('<br />', $feedback),
                          array('class' => 'feedbackspan accesshide'));
          
          2.- display bug in MCQ sub-questions only
          a. Behaviour: Adaptive

          Whether correct: shown;
          action: Check button
          tootlip feedback is displayed when hovering over dropdown list input field: OK (i.e. hovering over the student's actual answer text, as in all other question types)

          b. Behaviour: any

          Whether correct: shown;
          action: Submit and finish button
          tootlip feedback is NOT displayed when hovering over dropdown list input field but hovering over the green (correct) or red (incorrect) icon.This behaviour is disconcerting.

          c. Behaviour: any

          action: Submit and finish button
          Whether correct: Not shown; Marks: Not shown; Specific feedback: Shown; Right answer: Not shown;
          action: Submit and finish button
          tootlip feedback can NOT be displayed at all for the MCQ sub-questions, since the green (correct) or red (incorrect) icons are not being displayed in this scenario.

          I do not know well enough the code of the Cloze question in M 2.1 to suggest a fix for this display bug.

          Show
          Joseph Rézeau added a comment - Note: tested in teacher Preview question window. With Pierre's patch applied: if ($feedbacktext != ('')) { $feedback[] = $feedbacktext; } 1.- displaying individual Feedback case : sub-question answer does not contain a feedback message settings: Behaviour: any; Whether correct: Not shown; Marks: Not shown; Specific feedback: Shown; Right answer: Not shown; bug: hovering shows small empty tooltip rather than nothing at all. suggested fix: check that the feedback() array is not empty before returning function data: line 150 add if (empty($feedback)) { return ""; } return html_writer::tag('span', implode('<br />', $feedback), array('class' => 'feedbackspan accesshide')); 2.- display bug in MCQ sub-questions only a. Behaviour: Adaptive Whether correct: shown; action: Check button tootlip feedback is displayed when hovering over dropdown list input field: OK (i.e. hovering over the student's actual answer text, as in all other question types) b. Behaviour: any Whether correct: shown; action: Submit and finish button tootlip feedback is NOT displayed when hovering over dropdown list input field but hovering over the green (correct) or red (incorrect) icon.This behaviour is disconcerting. c. Behaviour: any action: Submit and finish button Whether correct: Not shown; Marks: Not shown; Specific feedback: Shown; Right answer: Not shown; action: Submit and finish button tootlip feedback can NOT be displayed at all for the MCQ sub-questions, since the green (correct) or red (incorrect) icons are not being displayed in this scenario. I do not know well enough the code of the Cloze question in M 2.1 to suggest a fix for this display bug.
          Hide
          Tim Hunt added a comment -

          I'll try to look at the fix this week.

          Show
          Tim Hunt added a comment - I'll try to look at the fix this week.
          Hide
          Pierre Pichet added a comment -

          Thanks Joseph for the more complete tests that I have done.
          Should find time to work on it tonight.

          Show
          Pierre Pichet added a comment - Thanks Joseph for the more complete tests that I have done. Should find time to work on it tonight.
          Hide
          Pierre Pichet added a comment -

          When the question is finished the select element is disabled

          <select id="q10:1_sub2_answer" disabled="disabled" class="select incorrect" name="q10:1_sub2_answer">
          

          The hovering is not working in this case.

          P.S. the () are not necessary in $feedbacktext != (''). they where just a leftover from a previous code using is_null() . There are some inconvenients to work to late

          Show
          Pierre Pichet added a comment - When the question is finished the select element is disabled <select id="q10:1_sub2_answer" disabled="disabled" class="select incorrect" name="q10:1_sub2_answer"> The hovering is not working in this case. P.S. the () are not necessary in $feedbacktext != (''). they where just a leftover from a previous code using is_null() . There are some inconvenients to work to late
          Hide
          Pierre Pichet added a comment -

          As Joseph noted, the test for empty line 150 should be added.

          Show
          Pierre Pichet added a comment - As Joseph noted, the test for empty line 150 should be added.
          Hide
          Joseph Rézeau added a comment -

          @ Pierre
          1.- I've just noticed that this problem is navigator-dependent. Firefox and Chrome will refuse to display anything pertaining to an input field which is disabled, whereas "good old" Internet Explorer will accept to do it!

          2.- With or without the parenteses, this condition alone does not solve the problem, Only the empty array test I suggest will work.

          Joseph

          Show
          Joseph Rézeau added a comment - @ Pierre 1.- I've just noticed that this problem is navigator-dependent. Firefox and Chrome will refuse to display anything pertaining to an input field which is disabled, whereas "good old" Internet Explorer will accept to do it! 2.- With or without the parenteses, this condition alone does not solve the problem, Only the empty array test I suggest will work. Joseph
          Show
          Joseph Rézeau added a comment - see http://stackoverflow.com/questions/2034820/firefox-does-not-show-tooltips-on-disabled-input-fields
          Hide
          Pierre Pichet added a comment -

          "Only the empty array test I suggest will work."
          I agree with you that your final test (line 150) is necessary if nothing has been added to $feedback at this point.
          The first test (line 125) is necessary to not have an empty line if there is no feedback defined.

          Should we or not disabled the select element in Cloze, the corresponding shortanswer and numerical are not disabled as far as I know on Firefox.

          Show
          Pierre Pichet added a comment - "Only the empty array test I suggest will work." I agree with you that your final test (line 150) is necessary if nothing has been added to $feedback at this point. The first test (line 125) is necessary to not have an empty line if there is no feedback defined. Should we or not disabled the select element in Cloze, the corresponding shortanswer and numerical are not disabled as far as I know on Firefox.
          Hide
          Pierre Pichet added a comment -

          The text input elements (numerical or shortanswer) are "readonly" when the question is finished.
          The tool-tip remains useable.

          Select element in Cloze must stay in-line with the text which limits the possibilities.

          If we put before and after the select element a small transparent image, this allows the display of the tooltip message when the checkmark is not shown.

          Show
          Pierre Pichet added a comment - The text input elements (numerical or shortanswer) are "readonly" when the question is finished. The tool-tip remains useable. Select element in Cloze must stay in-line with the text which limits the possibilities. If we put before and after the select element a small transparent image, this allows the display of the tooltip message when the checkmark is not shown.
          Hide
          Pierre Pichet added a comment -

          More simply just put a space before and after the select element can do the job.

                  $output = '';
                  $output .= html_writer::start_tag('label', array('class' => 'subq'));
                  $output .='&nbsp;';
                  $output .= $select;
                  $output .= $feedbackimg.'&nbsp;';
                  $output .= $feedbackpopup;
          

          Although there will be a space between the text it will stay inline.
          This could change a little the display when the select element is used to complete a word or specify the punctuation.
          So perhaps we should not add it before the select.
          The space after could be used only if $feedbackimg is null.
          In this case a transparent feedback image can be used.

          Show
          Pierre Pichet added a comment - More simply just put a space before and after the select element can do the job. $output = ''; $output .= html_writer::start_tag('label', array('class' => 'subq')); $output .='&nbsp;'; $output .= $select; $output .= $feedbackimg.'&nbsp;'; $output .= $feedbackpopup; Although there will be a space between the text it will stay inline. This could change a little the display when the select element is used to complete a word or specify the punctuation. So perhaps we should not add it before the select. The space after could be used only if $feedbackimg is null. In this case a transparent feedback image can be used.
          Hide
          Tim Hunt added a comment -

          Pierre's fix is almost right, but it misses out a check for $options->feedback. I have done a revised fix.

          Show
          Tim Hunt added a comment - Pierre's fix is almost right, but it misses out a check for $options->feedback. I have done a revised fix.
          Hide
          Tim Hunt added a comment -

          By the way, the 'disabled select menus in Firefox don't show any feedback' problem is a separate bug. I think someone may already have reported it. Anyway, please can you move discussion of that issue there. Thanks.

          Show
          Tim Hunt added a comment - By the way, the 'disabled select menus in Firefox don't show any feedback' problem is a separate bug. I think someone may already have reported it. Anyway, please can you move discussion of that issue there. Thanks.
          Hide
          Joseph Rézeau added a comment -

          Tim "Pierre's fix is almost right, but it misses out a check for $options->feedback. I have done a revised fix."
          1.- But that test is already done line 122:

          if (!$options->feedback) {
                      return '';
                  }
          

          2.- My remark 13/Sep/11 7:30 PM is still valid:
          .- displaying individual Feedback

          case : sub-question answer does not contain a feedback message
          settings: Behaviour: any;
          Whether correct: Not shown; Marks: Not shown; Specific feedback: Shown; Right answer: Not shown;
          bug: hovering shows small empty tooltip rather than nothing at all.
          and my suggested fix should be added to avoid display of small empty tooltip.
          add line 150

          if (empty($feedback)) {
                      return "";
                  }
                  return html_writer::tag('span', implode('<br />', $feedback),
                          array('class' => 'feedbackspan accesshide'));
          

          Joseph

          Show
          Joseph Rézeau added a comment - Tim "Pierre's fix is almost right, but it misses out a check for $options->feedback. I have done a revised fix." 1.- But that test is already done line 122: if (!$options->feedback) { return ''; } 2.- My remark 13/Sep/11 7:30 PM is still valid: .- displaying individual Feedback case : sub-question answer does not contain a feedback message settings: Behaviour: any; Whether correct: Not shown; Marks: Not shown; Specific feedback: Shown; Right answer: Not shown; bug: hovering shows small empty tooltip rather than nothing at all. and my suggested fix should be added to avoid display of small empty tooltip. add line 150 if (empty($feedback)) { return ""; } return html_writer::tag('span', implode('<br />', $feedback), array('class' => 'feedbackspan accesshide')); Joseph
          Hide
          Tim Hunt added a comment -

          Doh! Thanks Joseph. I was clearly too tired and careless to be reviewing code. I'll fix it tomorrow.

          Show
          Tim Hunt added a comment - Doh! Thanks Joseph. I was clearly too tired and careless to be reviewing code. I'll fix it tomorrow.
          Hide
          Pierre Pichet added a comment - - edited

          I reworked my proposal YESTERDAY but did not add the new references here.

          @@ -123,7 +123,8 @@ abstract class qtype_multianswer_subq_renderer_base extends qtype_renderer {
          123	123	
                   }
          124	124	
           
          125	125	
                   $feedback = array();
          126	 	
          -        if ($feedbacktext != ('')) {
           	126	
          +
           	127	
          +        if ($feedbacktext != '') {
          127	128	
                       $feedback[] = $feedbacktext;
          128	129	
                   }
          129	130	
           
          ...	...	
          @@ -147,7 +148,10 @@ abstract class qtype_multianswer_subq_renderer_base extends qtype_renderer {
          147	148	
                       $a->max =  format_float($subq->maxmark, $options->markdp);
          148	149	
                       $feedback[] = get_string('markoutofmax', 'question', $a);
          149	150	
                   }
          150	 	
          -
           	151	
          +        
           	152	
          +        if (empty($feedback)) {
           	153	
          +            return "";
           	154	
          +        }
          151	155	
                   return html_writer::tag('span', implode('<br />', $feedback),
          152	156	
                           array('class' => 'feedbackspan accesshide'));
          153	157	
               }
          

          So we all agree for almost the same thing
          with the exception that there is no necessary test for feedback line 135

          if ($options->feedback && $feedbacktext != ('')) {
          

          as this is already tested line 121
          We need however to test $feedback line 150 as $feedback could be null and if nothing else is displayed then there is a small empty square that will remain.

          Should we put the feedback before or after the correctness is not clear for me given that the $feedbackimg is already displayed before the user move his mouse to see the details.

          P.S.
          the reference is

          https://github.com/ppichet/moodle/blob/9e6fbeb601936002854fa6e9b784ea448040d04e/question/type/multianswer/renderer.php

          As this was my first merge, perhaps things are not so clear ...

          Show
          Pierre Pichet added a comment - - edited I reworked my proposal YESTERDAY but did not add the new references here. @@ -123,7 +123,8 @@ abstract class qtype_multianswer_subq_renderer_base extends qtype_renderer { 123 123 } 124 124 125 125 $feedback = array(); 126 - if ($feedbacktext != ('')) { 126 + 127 + if ($feedbacktext != '') { 127 128 $feedback[] = $feedbacktext; 128 129 } 129 130 ... ... @@ -147,7 +148,10 @@ abstract class qtype_multianswer_subq_renderer_base extends qtype_renderer { 147 148 $a->max = format_float($subq->maxmark, $options->markdp); 148 149 $feedback[] = get_string('markoutofmax', 'question', $a); 149 150 } 150 - 151 + 152 + if (empty($feedback)) { 153 + return ""; 154 + } 151 155 return html_writer::tag('span', implode('<br />', $feedback), 152 156 array('class' => 'feedbackspan accesshide')); 153 157 } So we all agree for almost the same thing with the exception that there is no necessary test for feedback line 135 if ($options->feedback && $feedbacktext != ('')) { as this is already tested line 121 We need however to test $feedback line 150 as $feedback could be null and if nothing else is displayed then there is a small empty square that will remain. Should we put the feedback before or after the correctness is not clear for me given that the $feedbackimg is already displayed before the user move his mouse to see the details. P.S. the reference is https://github.com/ppichet/moodle/blob/9e6fbeb601936002854fa6e9b784ea448040d04e/question/type/multianswer/renderer.php As this was my first merge, perhaps things are not so clear ...
          Hide
          Tim Hunt added a comment -

          OK. I revised my commit. Thank you both for your peer-review.

          Show
          Tim Hunt added a comment - OK. I revised my commit. Thank you both for your peer-review.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Sam Hemelryk added a comment -

          Thanks everyone this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks everyone this has been integrated now
          Hide
          Ankit Agarwal added a comment -

          test passed.
          Thanks

          Show
          Ankit Agarwal added a comment - test passed. Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And this code has been spread to all Moodle git and cvs repositories. Many thanks! Closing.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And this code has been spread to all Moodle git and cvs repositories. Many thanks! Closing. Ciao

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: