diff --git a/common/lib/capa/capa/templates/choicegroup.html b/common/lib/capa/capa/templates/choicegroup.html index f4b39c7969..6d9b1276e0 100644 --- a/common/lib/capa/capa/templates/choicegroup.html +++ b/common/lib/capa/capa/templates/choicegroup.html @@ -18,47 +18,39 @@ import six

${description_text}

% endfor % for choice_id, choice_label in choices: + <% + label_class = 'response-label field-label label-inline' + input_class = 'field-input input-' + input_type + input_checked = '' + + if is_radio_input(choice_id) or (input_type != 'radio' and choice_id in value): + input_class += ' submitted' + if status.classname and not show_correctness == 'never': + label_class += ' choicegroup_' + status.classname + %>
- <% - label_class = 'response-label field-label label-inline' - %> - -
% endfor
- % if input_type == 'checkbox' or status.classname == 'unanswered': - % if show_correctness != 'never': - <%include file="status_span.html" args="status=status, status_id=id"/> - % else: - <%include file="status_span.html" args="status=status, status_id=id, hide_correctness=True"/> - % endif + % if show_correctness != 'never': + <%include file="status_span.html" args="status=status, status_id=id"/> + % else: + <%include file="status_span.html" args="status=status, status_id=id, hide_correctness=True"/> % endif
% if show_correctness == "never" and (value or status not in ['unsubmitted']): diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index 82342698b4..9e599db576 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -118,8 +118,8 @@ class TemplateTestCase(unittest.TestCase): If no elements are found, the assertion fails. """ element_list = xml_root.xpath(xpath) - self.assertGreater(len(element_list), 0, "Could not find element at '%s'" % str(xpath)) - + self.assertGreater(len(element_list), 0, "Could not find element at '%s'\n%s" % + (str(xpath), etree.tostring(xml_root))) if exact: self.assertEqual(text, element_list[0].text.strip()) else: @@ -345,7 +345,7 @@ class ChoiceGroupTemplateTest(TemplateTestCase): def test_option_marked_correct(self): """ Test conditions under which a particular option - (not the entire problem) is marked correct. + and the entire problem is marked correct. """ conditions = [ {'input_type': 'radio', 'value': '2'}, @@ -359,14 +359,14 @@ class ChoiceGroupTemplateTest(TemplateTestCase): xpath = "//label[contains(@class, 'choicegroup_correct')]" self.assert_has_xpath(xml, xpath, self.context) - # Should NOT mark the whole problem - xpath = "//div[@class='indicator-container']/span" - self.assert_no_xpath(xml, xpath, self.context) + # Should also mark the whole problem + xpath = "//div[@class='indicator-container']/span[@class='status correct']" + self.assert_has_xpath(xml, xpath, self.context) def test_option_marked_incorrect(self): """ Test conditions under which a particular option - (not the entire problem) is marked incorrect. + and the entire problem is marked incorrect. """ conditions = [ {'input_type': 'radio', 'value': '2'}, @@ -380,9 +380,9 @@ class ChoiceGroupTemplateTest(TemplateTestCase): xpath = "//label[contains(@class, 'choicegroup_incorrect')]" self.assert_has_xpath(xml, xpath, self.context) - # Should NOT mark the whole problem - xpath = "//div[@class='indicator-container']/span" - self.assert_no_xpath(xml, xpath, self.context) + # Should also mark the whole problem + xpath = "//div[@class='indicator-container']/span[@class='status incorrect']" + self.assert_has_xpath(xml, xpath, self.context) def test_never_show_correctness(self): """ diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 05355bd052..32bba7cda7 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -222,52 +222,6 @@ div.problem { &::after { @include margin-left($baseline*0.75); } - - &:hover { - border: 2px solid $blue; - } - - &.choicegroup_correct { - @include status-icon($correct, $checkmark-icon); - - border: 2px solid $correct; - - // keep green for correct answers on hover. - &:hover { - border-color: $correct; - } - } - - &.choicegroup_partially-correct { - @include status-icon($partially-correct, $asterisk-icon); - - border: 2px solid $partially-correct; - - // keep green for correct answers on hover. - &:hover { - border-color: $partially-correct; - } - } - - &.choicegroup_incorrect { - @include status-icon($incorrect, $cross-icon); - - border: 2px solid $incorrect; - - // keep red for incorrect answers on hover. - &:hover { - border-color: $incorrect; - } - } - - &.choicegroup_submitted { - border: 2px solid $submitted; - - // keep blue for submitted answers on hover. - &:hover { - border-color: $submitted; - } - } } .indicator-container { @@ -284,6 +238,41 @@ div.problem { input[type="checkbox"] { @include margin(($baseline/4) ($baseline/2) ($baseline/4) ($baseline/4)); } + + input { + &:focus, + &:hover { + & + label { + border: 2px solid $blue; + } + } + + &, + &:focus, + &:hover { + & + label.choicegroup_correct { + @include status-icon($correct, $checkmark-icon); + + border: 2px solid $correct; + } + + & + label.choicegroup_partially-correct { + @include status-icon($partially-correct, $asterisk-icon); + + border: 2px solid $partially-correct; + } + + & + label.choicegroup_incorrect { + @include status-icon($incorrect, $cross-icon); + + border: 2px solid $incorrect; + } + + & + label.choicegroup_submitted { + border: 2px solid $submitted; + } + } + } } // +Problem - Choice Group @@ -292,6 +281,10 @@ div.problem { .choicegroup { @extend %choicegroup-base; + .field { + position: relative; + } + label { @include padding($baseline/2); @include padding-left($baseline*1.9); @@ -308,6 +301,9 @@ div.problem { position: absolute; top: em(9); + width: $baseline*1.1; + height: $baseline*1.1; + z-index: 1; } legend { @@ -1628,6 +1624,17 @@ div.problem .imageinput.capa_inputtype { top: 3px; width: 25px; height: 20px; + + &.unsubmitted, + &.unanswered { + .status-icon { + content: ''; + } + + .status-message { + display: none; + } + } } .correct { @@ -1658,6 +1665,17 @@ div.problem .annotation-input { top: 3px; width: 25px; height: 20px; + + &.unsubmitted, + &.unanswered { + .status-icon { + content: ''; + } + + .status-message { + display: none; + } + } } .correct { diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.js b/common/lib/xmodule/xmodule/js/src/capa/display.js index 728b047bc7..5ec25bfc19 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.js +++ b/common/lib/xmodule/xmodule/js/src/capa/display.js @@ -230,16 +230,16 @@ // Render 'x point(s) possible (un/graded, results hidden)' if no current score provided. if (graded) { progressTemplate = ngettext( - // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; - '%(num_points)s point possible (graded, results hidden)', - '%(num_points)s points possible (graded, results hidden)', + // Translators: {num_points} is the number of points possible (examples: 1, 3, 10).; + '{num_points} point possible (graded, results hidden)', + '{num_points} points possible (graded, results hidden)', totalScore ); } else { progressTemplate = ngettext( - // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; - '%(num_points)s point possible (ungraded, results hidden)', - '%(num_points)s points possible (ungraded, results hidden)', + // Translators: {num_points} is the number of points possible (examples: 1, 3, 10).; + '{num_points} point possible (ungraded, results hidden)', + '{num_points} points possible (ungraded, results hidden)', totalScore ); } @@ -248,14 +248,14 @@ // But if staff has overridden score to a non-zero number, show it if (graded) { progressTemplate = ngettext( - // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; - '%(num_points)s point possible (graded)', '%(num_points)s points possible (graded)', + // Translators: {num_points} is the number of points possible (examples: 1, 3, 10).; + '{num_points} point possible (graded)', '{num_points} points possible (graded)', totalScore ); } else { progressTemplate = ngettext( - // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; - '%(num_points)s point possible (ungraded)', '%(num_points)s points possible (ungraded)', + // Translators: {num_points} is the number of points possible (examples: 1, 3, 10).; + '{num_points} point possible (ungraded)', '{num_points} points possible (ungraded)', totalScore ); } @@ -264,25 +264,25 @@ if (graded) { progressTemplate = ngettext( // This comment needs to be on one line to be properly scraped for the translators. - // Translators: %(earned)s is the number of points earned. %(possible)s is the total number of points (examples: 0/1, 1/1, 2/3, 5/10). The total number of points will always be at least 1. We pluralize based on the total number of points (example: 0/1 point; 1/2 points); - '%(earned)s/%(possible)s point (graded)', '%(earned)s/%(possible)s points (graded)', + // Translators: {earned} is the number of points earned. {possible} is the total number of points (examples: 0/1, 1/1, 2/3, 5/10). The total number of points will always be at least 1. We pluralize based on the total number of points (example: 0/1 point; 1/2 points); + '{earned}/{possible} point (graded)', '{earned}/{possible} points (graded)', totalScore ); } else { progressTemplate = ngettext( // This comment needs to be on one line to be properly scraped for the translators. - // Translators: %(earned)s is the number of points earned. %(possible)s is the total number of points (examples: 0/1, 1/1, 2/3, 5/10). The total number of points will always be at least 1. We pluralize based on the total number of points (example: 0/1 point; 1/2 points); - '%(earned)s/%(possible)s point (ungraded)', '%(earned)s/%(possible)s points (ungraded)', + // Translators: {earned} is the number of points earned. {possible} is the total number of points (examples: 0/1, 1/1, 2/3, 5/10). The total number of points will always be at least 1. We pluralize based on the total number of points (example: 0/1 point; 1/2 points); + '{earned}/{possible} point (ungraded)', '{earned}/{possible} points (ungraded)', totalScore ); } } - progress = interpolate( + progress = edx.StringUtils.interpolate( progressTemplate, { earned: curScore, num_points: totalScore, possible: totalScore - }, true + } ); return this.$('.problem-progress').text(progress); }; @@ -379,7 +379,7 @@ Problem.prototype.render = function(content, focusCallback) { var that = this; if (content) { - this.el.html(content); + edx.HtmlUtils.setHtml(this.el, edx.HtmlUtils.HTML(content)); return JavascriptLoader.executeModuleScripts(this.el, function() { that.setupInputTypes(); that.bind(); @@ -389,7 +389,7 @@ }); } else { return $.postWithPrefix('' + this.url + '/problem_get', function(response) { - that.el.html(response.html); + edx.HtmlUtils.setHtml(that.el, edx.HtmlUtils.HTML(response.html)); return JavascriptLoader.executeModuleScripts(that.el, function() { that.setupInputTypes(); that.bind(); @@ -560,11 +560,12 @@ } )); } - fd.append(element.id, file); + fd.append(element.id, file); // xss-lint: disable=javascript-jquery-append } if (element.files.length === 0) { fileNotSelected = true; - fd.append(element.id, ''); // In case we want to allow submissions with no file + // In case we want to allow submissions with no file + fd.append(element.id, ''); // xss-lint: disable=javascript-jquery-append } if (requiredFiles.length !== 0) { requiredFilesNotSubmitted = true; @@ -575,18 +576,21 @@ )); } } else { - fd.append(element.id, element.value); + fd.append(element.id, element.value); // xss-lint: disable=javascript-jquery-append } }); if (fileNotSelected) { errors.push(gettext('You did not select any files to submit.')); } - errorHtml = ''; + errorHtml = edx.HtmlUtils.interpolateHtml(edx.HtmlUtils.HTML(''), {errors: errorHtml}); this.gentle_alert(errorHtml); abortSubmission = fileTooLarge || fileNotSelected || unallowedFileSubmitted || requiredFilesNotSubmitted; if (abortSubmission) { @@ -965,6 +969,7 @@ return $(element).find('input').on('input', function() { var $p; $p = $(element).find('span.status'); + $p.removeClass('correct incorrect submitted'); return $p.parent().removeAttr('class').addClass('unsubmitted'); }); }, @@ -1002,6 +1007,7 @@ return $(element).find('input').on('input', function() { var $p; $p = $(element).find('span.status'); + $p.removeClass('correct incorrect submitted'); return $p.parent().removeClass('correct incorrect').addClass('unsubmitted'); }); } @@ -1069,8 +1075,8 @@ results = []; for (i = 0, len = answer.length; i < len; i++) { choice = answer[i]; - $inputLabel = $element.find('#input_' + inputId + '_' + choice).parent('label'); - $inputStatus = $inputLabel.find('#status_' + inputId); + $inputLabel = $element.find('#input_' + inputId + '_' + choice + ' + label'); + $inputStatus = $element.find('#status_' + inputId); // If the correct answer was already Submitted before "Show Answer" was selected, // the status HTML will already be present. Otherwise, inject the status HTML. @@ -1078,12 +1084,13 @@ // will be marked as "unanswered". In that case, for correct answers update the // classes accordingly. if ($inputStatus.hasClass('unanswered')) { - $inputStatus.removeAttr('class').addClass('status correct'); + edx.HtmlUtils.append($inputLabel, edx.HtmlUtils.HTML(correctStatusHtml)); $inputLabel.addClass('choicegroup_correct'); } else if (!$inputLabel.hasClass('choicegroup_correct')) { // If the status HTML is not already present (due to clicking Submit), append // the status HTML for correct answers. edx.HtmlUtils.append($inputLabel, edx.HtmlUtils.HTML(correctStatusHtml)); + $inputLabel.removeClass('choicegroup_incorrect'); results.push($inputLabel.addClass('choicegroup_correct')); } } @@ -1182,7 +1189,7 @@ types[key](context, value); } }); - container.html(canvas); + edx.HtmlUtils.setHtml(container, edx.HtmlUtils.HTML(canvas)); } else { console.log('Answer is absent for image input with id=' + id); // eslint-disable-line no-console } diff --git a/common/test/acceptance/pages/lms/problem.py b/common/test/acceptance/pages/lms/problem.py index 6ee2cf9c38..c51f2d9e84 100644 --- a/common/test/acceptance/pages/lms/problem.py +++ b/common/test/acceptance/pages/lms/problem.py @@ -489,30 +489,39 @@ class ProblemPage(PageObject): solution_selector = '.solution-span div.detailed-solution' return self.q(css=solution_selector).is_present() - def is_choice_highlighted(self, choice, choices_list): + def is_choice_highlighted(self, choice, choices_list, show_answer=True): """ Check if the given answer/choice is highlighted for choice group. + + show_answer: if set, then requires each choice to be marked with a status. + If not set, then the status can be elswhere in the problem. """ - choice_status_xpath = (u'//fieldset/div[contains(@class, "field")][{{0}}]' - u'/label[contains(@class, "choicegroup_{choice}")]' - u'/span[contains(@class, "status {choice}")]'.format(choice=choice)) - any_status_xpath = u'//fieldset/div[contains(@class, "field")][{0}]/label/span' - for choice in choices_list: - if not self.q(xpath=choice_status_xpath.format(choice)).is_present(): + if show_answer: + choice_status_xpath = (u'//fieldset/div[contains(@class, "field")][{{0}}]' + u'/label[contains(@class, "choicegroup_{choice}")]' + u'/span[contains(@class, "status {choice}")]'.format(choice=choice)) + any_status_xpath = u'//fieldset/div[contains(@class, "field")][{0}]/label/span' + else: + choice_status_xpath = (u'//fieldset/div[contains(@class, "field")][{{0}}]' + u'/label[contains(@class, "choicegroup_{choice}")]'.format(choice=choice)) + any_status_xpath = u'//div[contains(@class, "indicator-container")]/span[contains(@class, "status")]' + + for possible_choice in choices_list: + if not self.q(xpath=choice_status_xpath.format(possible_choice)).is_present(): return False # Check that there is only a single status span, as there were some bugs with multiple # spans (with various classes) being appended. - if not len(self.q(xpath=any_status_xpath.format(choice)).results) == 1: + if not len(self.q(xpath=any_status_xpath.format(possible_choice)).results) == 1: return False return True - def is_correct_choice_highlighted(self, correct_choices): + def is_correct_choice_highlighted(self, correct_choices, show_answer=True): """ Check if correct answer/choice highlighted for choice group. """ - return self.is_choice_highlighted('correct', correct_choices) + return self.is_choice_highlighted('correct', correct_choices, show_answer) def is_submitted_choice_highlighted(self, correct_choices): """ diff --git a/common/test/acceptance/tests/lms/test_problem_types.py b/common/test/acceptance/tests/lms/test_problem_types.py index 5048909d5e..54d3cb379e 100644 --- a/common/test/acceptance/tests/lms/test_problem_types.py +++ b/common/test/acceptance/tests/lms/test_problem_types.py @@ -786,7 +786,7 @@ class MultipleChoiceProblemTypeTest(MultipleChoiceProblemTypeBase, ProblemTypeTe # After submit, the answer should be marked as correct. self.problem_page.click_submit() - self.assertTrue(self.problem_page.is_correct_choice_highlighted(correct_choices=[3])) + self.assertTrue(self.problem_page.is_correct_choice_highlighted(correct_choices=[3], show_answer=False)) # Switch to an incorrect answer. This will hide the correctness indicator. self.answer_problem('incorrect')