From d574ac363c0530b3f1d8e51342d0f33c71ee38c3 Mon Sep 17 00:00:00 2001 From: Jeff Ericson Date: Tue, 3 Dec 2013 16:08:06 -0800 Subject: [PATCH] Added targeted-feedback option to capa_problem Modified CSS and custom renderer to display targeted feedback boxes --- CHANGELOG.rst | 9 + common/lib/capa/capa/capa_problem.py | 86 ++- common/lib/capa/capa/customrender.py | 38 ++ .../capa/capa/tests/test_targeted_feedback.py | 608 ++++++++++++++++++ .../lib/xmodule/xmodule/css/capa/display.scss | 45 ++ 5 files changed, 784 insertions(+), 2 deletions(-) create mode 100644 common/lib/capa/capa/tests/test_targeted_feedback.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a62b0fb0091e..b8bb9d30d3c6 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -118,6 +118,15 @@ client error are correctly passed through to the client. LMS: Improve performance of page load and thread list load for discussion tab +Studio: Support targeted feedback, which allows for authors to provide explanations for +incorrect choice selections for multiple choice question choices that will automatically +display. These are intended to help steer a student to the correct answer. Thus, they are +best used for quizzes that allow multiple attempts. To provide targeted feedback, add an +element called right before your or , and in +this element, provide a for each feedback. Within +you can specify your text explanation. Both the and should have +the same explanation-id attribute. + LMS: The wiki markup cheatsheet dialog is now accessible to screen readers. (LMS-1303) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 08a223f609b8..283b53064763 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -34,6 +34,7 @@ from pytz import UTC + # dict of tagname, Response Class -- this should come from auto-registering response_tag_dict = dict([(x.response_tag, x) for x in responsetypes.__all__]) @@ -381,11 +382,93 @@ def get_answer_ids(self): answer_ids.append(results.keys()) return answer_ids + def tree_using_targeted_feedback(self, tree): + """ + Allows for problem questions to show targeted feedback, which are choice-level explanations. + Targeted feedback is automatically visible after a student has submitted their answers. + + The tag must have an attribute 'targeted-feedback': + - if so, this method will modify the tree + - if not, this method will not modify the tree + - if the value is 'alwaysShowCorrectChoiceExplanation', then the correct-choice + explanation will be automatically visible too after a student has submitted answers + + Note if the value is 'alwaysShowCorrectChoiceExplanation', you probably want to set + the "Show Answer" setting to "Never" because now there's no need for a "Show Answer" + button because no solution will show up if you were to click the "Show Answer" button + """ + + # Note that if there are no questions with targeted feedback, the body of the for loop is not executed + for mult_choice_response in tree.xpath('//multiplechoiceresponse[@targeted-feedback]'): + show_explanation = mult_choice_response.get('targeted-feedback') == 'alwaysShowCorrectChoiceExplanation' + + # Grab the first choicegroup (there should only be one within each tag) + choicegroup = mult_choice_response.xpath('./choicegroup[@type="MultipleChoice"]')[0] + choices_list = list(choicegroup.iter('choice')) + + # Find the student answer key that matches our id + student_answer = self.student_answers.get(choicegroup.get('id')) + expl_id_for_student_answer = None + + # Keep track of the explanation-id that corresponds to the student's answer + # Also, keep track of the solution-id + solution_id = None + for choice in choices_list: + if choice.get('name') == student_answer: + expl_id_for_student_answer = choice.get('explanation-id') + if choice.get('correct') == 'true': + solution_id = choice.get('explanation-id') + + # Filter out targetedfeedback that doesn't correspond to the answer the student selected + # Note: following-sibling will grab all following siblings, so we just want the first in the list + targetedfeedbackset = mult_choice_response.xpath('./following-sibling::targetedfeedbackset') + if len(targetedfeedbackset) != 0: + targetedfeedbackset = targetedfeedbackset[0] + targetedfeedbacks = targetedfeedbackset.xpath('./targetedfeedback') + for targetedfeedback in targetedfeedbacks: + # Don't show targeted feedback if the student hasn't answer the problem + # or if the target feedback doesn't match the student's (incorrect) answer + if not self.done or targetedfeedback.get('explanation-id') != expl_id_for_student_answer: + targetedfeedbackset.remove(targetedfeedback) + + # Do not displace the solution under these circumstances + if not show_explanation or not self.done: + continue + + # The next element should either be or + next_element = targetedfeedbackset.getnext() + parent_element = tree + solution_element = None + if next_element.tag == 'solution': + solution_element = next_element + elif next_element.tag == 'solutionset': + solutions = next_element.xpath('./solution') + for solution in solutions: + if solution.get('explanation-id') == solution_id: + parent_element = next_element + solution_element = solution + + # If could not find the solution element, then skip the remaining steps below + if solution_element is None: + continue + + # Change our correct-choice explanation from a "solution explanation" to within + # the set of targeted feedback, which means the explanation will render on the page + # without the student clicking "Show Answer" or seeing a checkmark next to the correct choice + parent_element.remove(solution_element) + + # Add our solution instead to the targetedfeedbackset and change its tag name + solution_element.tag = 'targetedfeedback' + targetedfeedbackset.append(solution_element) + def get_html(self): ''' Main method called externally to get the HTML to be rendered for this capa Problem. ''' + + self.tree_using_targeted_feedback(self.tree) html = contextualize_text(etree.tostring(self._extract_html(self.tree)), self.context) + return html def handle_input_ajax(self, data): @@ -562,8 +645,7 @@ def _extract_html(self, problemtree): # private # other than to examine .tag to see if it's a string. :( return - if (problemtree.tag == 'script' and problemtree.get('type') - and 'javascript' in problemtree.get('type')): + if (problemtree.tag == 'script' and problemtree.get('type') and 'javascript' in problemtree.get('type')): # leave javascript intact. return deepcopy(problemtree) diff --git a/common/lib/capa/capa/customrender.py b/common/lib/capa/capa/customrender.py index f7d586c9d55d..e27dc88a9fcc 100644 --- a/common/lib/capa/capa/customrender.py +++ b/common/lib/capa/capa/customrender.py @@ -98,3 +98,41 @@ def get_html(self): return etree.XML(html) registry.register(SolutionRenderer) + +#----------------------------------------------------------------------------- + + +class TargetedFeedbackRenderer(object): + ''' + A targeted feedback is just a ... that is used for displaying an + extended piece of feedback to students if they incorrectly answered a question. + ''' + tags = ['targetedfeedback'] + + def __init__(self, system, xml): + self.system = system + self.xml = xml + + def get_html(self): + """ + Return the contents of this tag, rendered to html, as an etree element. + """ + + html = '
%s
' % ( + etree.tostring(self.xml)) + try: + xhtml = etree.XML(html) + except Exception as err: + if self.system.DEBUG: + msg = '

Error %s

' % ( + str(err).replace('<', '<')) + msg += ('

Failed to construct targeted feedback from 

%s

' % + html.replace('<', '<')) + msg += "
" + log.error(msg) + return etree.XML(msg) + else: + raise + return xhtml + +registry.register(TargetedFeedbackRenderer) diff --git a/common/lib/capa/capa/tests/test_targeted_feedback.py b/common/lib/capa/capa/tests/test_targeted_feedback.py new file mode 100644 index 000000000000..282a71987343 --- /dev/null +++ b/common/lib/capa/capa/tests/test_targeted_feedback.py @@ -0,0 +1,608 @@ +""" +Tests the logic of the "targeted-feedback" attribute for MultipleChoice questions, +i.e. those with the element +""" + +import unittest +import textwrap +from . import test_system, new_loncapa_problem + + +class CapaTargetedFeedbackTest(unittest.TestCase): + ''' + Testing class + ''' + + def setUp(self): + super(CapaTargetedFeedbackTest, self).setUp() + self.system = test_system() + + def test_no_targeted_feedback(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + """) + + problem = new_loncapa_problem(xml_str) + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r"
.*'wrong-1'.*'wrong-2'.*'correct-1'.*'wrong-3'.*
") + self.assertRegexpMatches(without_new_lines, r"feedback1|feedback2|feedback3|feedbackC") + + def test_targeted_feedback_not_finished(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + """) + + problem = new_loncapa_problem(xml_str) + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r"
.*'wrong-1'.*'wrong-2'.*'correct-1'.*'wrong-3'.*
") + self.assertNotRegexpMatches(without_new_lines, r"feedback1|feedback2|feedback3|feedbackC") + + def test_targeted_feedback_student_answer1(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + """) + + problem = new_loncapa_problem(xml_str) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_3'} + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r".*3rd WRONG solution") + self.assertNotRegexpMatches(without_new_lines, r"feedback1|feedback2|feedbackC") + + def test_targeted_feedback_student_answer2(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + """) + + problem = new_loncapa_problem(xml_str) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_0'} + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r".*1st WRONG solution") + self.assertRegexpMatches(without_new_lines, r"
\{.*'1_solution_1'.*\}
") + self.assertNotRegexpMatches(without_new_lines, r"feedback2|feedback3|feedbackC") + + def test_targeted_feedback_show_solution_explanation(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + """) + + problem = new_loncapa_problem(xml_str) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_0'} + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r".*1st WRONG solution") + self.assertRegexpMatches(without_new_lines, r"\{.*'1_solution_1'.*\}") + self.assertNotRegexpMatches(without_new_lines, r"feedback2|feedback3") + + def test_targeted_feedback_no_show_solution_explanation(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + """) + + problem = new_loncapa_problem(xml_str) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_0'} + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r".*1st WRONG solution") + self.assertNotRegexpMatches(without_new_lines, r"\{.*'1_solution_1'.*\}") + self.assertNotRegexpMatches(without_new_lines, r"feedback2|feedback3|feedbackC") + + def test_targeted_feedback_with_solutionset_explanation(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + correct-2 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + +
+

Targeted Feedback

+

Feedback on the other solution...

+
+ + + + + + +
+

Explanation

+

This is the other solution explanation

+

Not much to explain here, sorry!

+
+ + + + + """) + + problem = new_loncapa_problem(xml_str) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_0'} + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r".*1st WRONG solution") + self.assertRegexpMatches(without_new_lines, r"\{.*'1_solution_1'.*\}") + self.assertNotRegexpMatches(without_new_lines, r"feedback2|feedback3") + + def test_targeted_feedback_no_feedback_for_selected_choice1(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + + """) + + problem = new_loncapa_problem(xml_str) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_1'} + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertRegexpMatches(without_new_lines, r"\{.*'1_solution_1'.*\}") + self.assertNotRegexpMatches(without_new_lines, r"feedback1|feedback3") + + def test_targeted_feedback_no_feedback_for_selected_choice2(self): + xml_str = textwrap.dedent(""" + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + + + """) + + problem = new_loncapa_problem(xml_str) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_1'} + + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + + self.assertNotRegexpMatches(without_new_lines, r"\{.*'1_solution_1'.*\}") + self.assertNotRegexpMatches(without_new_lines, r"feedback1|feedback3|feedbackC") diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index ddf57e846d37..68dd76b701e1 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -126,6 +126,23 @@ section.problem { } } + .targeted-feedback-span { + > span { + margin: $baseline 0; + display: block; + border: 1px solid #000; + padding: 9px 15px $baseline; + background: #fff; + position: relative; + box-shadow: inset 0 0 0 1px #eee; + border-radius: 3px; + + &:empty { + display: none; + } + } + } + div { p { &.answer { @@ -628,6 +645,34 @@ section.problem { } } + .detailed-targeted-feedback { + > p:first-child { + color: red; + text-transform: uppercase; + font-weight: bold; + font-style: normal; + font-size: 0.9em; + } + + p:last-child { + margin-bottom: 0; + } + } + + .detailed-targeted-feedback-correct { + > p:first-child { + color: green; + text-transform: uppercase; + font-weight: bold; + font-style: normal; + font-size: 0.9em; + } + + p:last-child { + margin-bottom: 0; + } + } + div.capa_alert { margin-top: $baseline; padding: 8px 12px;