Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-28259

Individual answers Feedback not shown in Cloze questions

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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:

      Description

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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt 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
            timhunt 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
            ppichet 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
            ppichet 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
            rezeau Joseph Rézeau added a comment -

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

            $feedback = array();
                    $feedback[] = $feedbacktext;

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

            Pierre, Tim, any progress on this bug?

            Show
            rezeau Joseph Rézeau added a comment - Pierre, Tim, any progress on this bug?
            Hide
            rezeau 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
            rezeau 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
            ppichet 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
            ppichet 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
            rezeau 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
            rezeau 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
            ppichet 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
            ppichet 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
            ppichet Pierre Pichet added a comment -

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

            Show
            ppichet Pierre Pichet added a comment - If the $feedbacktext != '' test is not done, there is an empty line when the feedback is null.
            Hide
            rezeau 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
            rezeau 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
            rezeau 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
            rezeau 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
            timhunt Tim Hunt added a comment -

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

            Show
            timhunt Tim Hunt added a comment - I'll try to look at the fix this week.
            Hide
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet Pierre Pichet added a comment -

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

            Show
            ppichet Pierre Pichet added a comment - As Joseph noted, the test for empty line 150 should be added.
            Hide
            rezeau 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
            rezeau 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
            rezeau Joseph Rézeau added a comment - see http://stackoverflow.com/questions/2034820/firefox-does-not-show-tooltips-on-disabled-input-fields
            Hide
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            timhunt 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
            timhunt 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
            timhunt 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
            timhunt 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
            rezeau 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
            rezeau 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
            timhunt 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
            timhunt 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
            ppichet 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
            ppichet 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
            timhunt Tim Hunt added a comment -

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

            Show
            timhunt Tim Hunt added a comment - OK. I revised my commit. Thank you both for your peer-review.
            Hide
            stronk7 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
            stronk7 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks everyone this has been integrated now

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

            test passed.
            Thanks

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

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

            Ciao

            Show
            stronk7 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:
                  Fix Release Date:
                  10/Oct/11