From c3b6b10478393ba26608bed60d36e998b6fbb540 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Wed, 8 Jun 2016 14:18:10 +0500 Subject: [PATCH 1/3] Problem (capa) feedback UX revamp. TNL-4877 --- cms/static/sass/_build-v1.scss | 4 + cms/static/sass/edx-pattern-library-shims | 1 + .../sass/elements/_system-feedback.scss | 1 - cms/static/sass/elements/_xblocks.scss | 10 +- cms/static/sass/xmodule/_headings.scss | 5 +- common/lib/capa/capa/capa_problem.py | 15 +- common/lib/capa/capa/responsetypes.py | 30 +- .../capa/capa/templates/annotationinput.html | 6 +- .../capa/templates/chemicalequationinput.html | 4 +- .../lib/capa/capa/templates/choicegroup.html | 6 +- .../lib/capa/capa/templates/choicetext.html | 13 +- common/lib/capa/capa/templates/codeinput.html | 9 +- .../capa/capa/templates/crystallography.html | 5 +- .../capa/templates/designprotein2dinput.html | 2 +- .../capa/templates/drag_and_drop_input.html | 6 +- .../capa/capa/templates/editageneinput.html | 2 +- .../capa/capa/templates/editamolecule.html | 3 +- .../capa/capa/templates/filesubmission.html | 2 +- .../capa/templates/formulaequationinput.html | 6 +- common/lib/capa/capa/templates/jsinput.html | 6 +- .../lib/capa/capa/templates/jstextline.html | 28 -- .../lib/capa/capa/templates/optioninput.html | 8 +- common/lib/capa/capa/templates/textline.html | 10 +- .../lib/capa/capa/templates/vsepr_input.html | 6 +- .../capa/capa/tests/response_xml_factory.py | 22 +- .../capa/tests/test_hint_functionality.py | 154 +++---- .../lib/capa/capa/tests/test_html_render.py | 6 +- .../capa/capa/tests/test_input_templates.py | 2 +- .../capa/capa/tests/test_targeted_feedback.py | 33 +- common/lib/xmodule/xmodule/capa_base.py | 237 ++++++---- common/lib/xmodule/xmodule/capa_module.py | 11 +- .../lib/xmodule/xmodule/css/capa/display.scss | 434 +++++++++++------- .../js/fixtures/matlabinput_problem.html | 67 +-- .../xmodule/js/fixtures/problem_content.html | 26 +- .../xmodule/xmodule/js/karma_xmodule.conf.js | 13 +- .../xmodule/js/spec/capa/display_spec.coffee | 263 +++++------ .../xmodule/js/src/capa/display.coffee | 407 +++++++++------- .../xmodule/modulestore/inheritance.py | 5 - .../templates/problem/jsinput_response.yaml | 2 +- .../xmodule/xmodule/tests/test_capa_module.py | 193 +++----- .../tests/test_delay_between_attempts.py | 14 +- .../xmodule/tests/test_xblock_wrappers.py | 1 + common/static/js/fixtures/sr-fixture.html | 4 + .../js/spec/accessibility_tools_spec.js | 34 ++ common/static/js/src/accessibility_tools.js | 40 +- .../edx-pattern-library-shims/_buttons.scss | 119 +++++ .../base/_variables.scss | 222 +++++++++ .../pages/lms/annotation_component.py | 2 +- common/test/acceptance/pages/lms/problem.py | 178 ++++++- .../pages/studio/settings_advanced.py | 1 - .../pages/xblock/crowdsourcehinter_problem.py | 2 +- .../acceptance/tests/data/multiple_choice.xml | 4 +- .../tests/lms/test_certificate_web_view.py | 10 +- .../acceptance/tests/lms/test_conditional.py | 2 +- common/test/acceptance/tests/lms/test_lms.py | 4 +- .../tests/lms/test_lms_courseware.py | 10 +- .../tests/lms/test_lms_entrance_exams.py | 2 +- .../acceptance/tests/lms/test_lms_gating.py | 2 +- .../acceptance/tests/lms/test_lms_problems.py | 419 +++++++++++++---- .../tests/lms/test_lms_user_preview.py | 8 +- .../tests/lms/test_problem_types.py | 333 ++++++++++---- .../tests/lms/test_progress_page.py | 2 +- .../courseware/features/problems.feature | 113 ++--- .../courseware/features/problems.py | 30 +- .../courseware/features/problems_setup.py | 26 +- lms/static/sass/_build-lms-v1.scss | 3 + lms/static/sass/base/_headings.scss | 7 +- lms/static/sass/edx-pattern-library-shims | 1 + .../sass/elements/_system-feedback.scss | 1 - lms/static/sass/partials/base/_variables.scss | 7 +- .../proctored-exam-status.underscore | 2 +- lms/templates/homework.html | 17 - lms/templates/problem.html | 89 +++- lms/templates/problem_ajax.html | 2 +- lms/templates/problem_notifications.html | 19 + requirements/edx/github.txt | 2 +- 76 files changed, 2476 insertions(+), 1319 deletions(-) create mode 120000 cms/static/sass/edx-pattern-library-shims delete mode 100644 common/lib/capa/capa/templates/jstextline.html create mode 100644 common/static/js/fixtures/sr-fixture.html create mode 100644 common/static/sass/edx-pattern-library-shims/_buttons.scss create mode 100644 common/static/sass/edx-pattern-library-shims/base/_variables.scss create mode 120000 lms/static/sass/edx-pattern-library-shims delete mode 100644 lms/templates/homework.html create mode 100644 lms/templates/problem_notifications.html diff --git a/cms/static/sass/_build-v1.scss b/cms/static/sass/_build-v1.scss index b2d56c57ec..94cec810c8 100644 --- a/cms/static/sass/_build-v1.scss +++ b/cms/static/sass/_build-v1.scss @@ -87,3 +87,7 @@ // +CodeMirror Overrides // ==================== @import 'elements/codemirror-overrides'; + +// CAPA Problem Feedback +@import 'edx-pattern-library-shims/buttons'; + diff --git a/cms/static/sass/edx-pattern-library-shims b/cms/static/sass/edx-pattern-library-shims new file mode 120000 index 0000000000..eae51650c7 --- /dev/null +++ b/cms/static/sass/edx-pattern-library-shims @@ -0,0 +1 @@ +../../../common/static/sass/edx-pattern-library-shims \ No newline at end of file diff --git a/cms/static/sass/elements/_system-feedback.scss b/cms/static/sass/elements/_system-feedback.scss index 44510875eb..2bdcc879c0 100644 --- a/cms/static/sass/elements/_system-feedback.scss +++ b/cms/static/sass/elements/_system-feedback.scss @@ -399,7 +399,6 @@ margin: 0 auto; width: flex-grid(12); max-width: $fg-max-width; - min-width: $fg-min-width; strong { @extend %t-strong; diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index e702861075..ed77384405 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -248,15 +248,7 @@ color: $color-visibility-set; } } - - .action { - - .save { - // taking styles from LMS for these Save buttons to maintain consistency - // there is no studio-specific style for these LMS-styled buttons - @extend %btn-lms-style; - } - } + } // +Messaging - Xblocks diff --git a/cms/static/sass/xmodule/_headings.scss b/cms/static/sass/xmodule/_headings.scss index e44d31ec2d..c58fba2f3e 100644 --- a/cms/static/sass/xmodule/_headings.scss +++ b/cms/static/sass/xmodule/_headings.scss @@ -32,9 +32,8 @@ $headings-base-color: $gray-d2; %hd-2 { - margin-bottom: 1em; - font-size: 1.5em; - font-weight: $headings-font-weight-normal; + font-size: 1.1125em; + font-weight: $headings-font-weight-bold; line-height: 1.4em; } diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 548ccd43da..be6c8549ec 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -376,7 +376,7 @@ class LoncapaProblem(object): def grade_answers(self, answers): """ - Grade student responses. Called by capa_module.check_problem. + Grade student responses. Called by capa_module.submit_problem. `answers` is a dict of all the entries from request.POST, but with the first part of each key removed (the string before the first "_"). @@ -496,6 +496,7 @@ class LoncapaProblem(object): choice-level explanations shown to a student after submission. Does nothing if there is no targeted-feedback attribute. """ + _ = self.capa_system.i18n.ugettext # Note that the modifications has been done, avoiding problems if called twice. if hasattr(self, 'has_targeted'): return @@ -515,9 +516,12 @@ class LoncapaProblem(object): # Keep track of the explanation-id that corresponds to the student's answer # Also, keep track of the solution-id solution_id = None + choice_correctness_for_student_answer = _('Incorrect') 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': + choice_correctness_for_student_answer = _('Correct') if choice.get('correct') == 'true': solution_id = choice.get('explanation-id') @@ -527,7 +531,15 @@ class LoncapaProblem(object): if len(targetedfeedbackset) != 0: targetedfeedbackset = targetedfeedbackset[0] targetedfeedbacks = targetedfeedbackset.xpath('./targetedfeedback') + # find the legend by id in choicegroup.html for aria-describedby + problem_legend_id = str(choicegroup.get('id')) + '-legend' for targetedfeedback in targetedfeedbacks: + screenreadertext = etree.Element("span") + targetedfeedback.insert(0, screenreadertext) + screenreadertext.set('class', 'sr') + screenreadertext.text = choice_correctness_for_student_answer + targetedfeedback.set('role', 'group') + targetedfeedback.set('aria-describedby', problem_legend_id) # 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: @@ -561,6 +573,7 @@ class LoncapaProblem(object): # Add our solution instead to the targetedfeedbackset and change its tag name solution_element.tag = 'targetedfeedback' + targetedfeedbackset.append(solution_element) def get_html(self): diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 161f04155e..7482ca72a2 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -51,6 +51,7 @@ from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautifu import capa.xqueue_interface as xqueue_interface import capa.safe_exec as safe_exec +from openedx.core.djangolib.markup import HTML, Text log = logging.getLogger(__name__) @@ -352,9 +353,9 @@ class LoncapaResponse(object): # Tricky: label None means output defaults, while '' means output empty label if label is None: if correct: - label = _(u'Correct') + label = _(u'Correct:') else: - label = _(u'Incorrect') + label = _(u'Incorrect:') # self.runtime.track_function('get_demand_hint', event_info) # This this "feedback hint" event @@ -372,15 +373,23 @@ class LoncapaResponse(object): self.capa_module.runtime.track_function('edx.problem.hint.feedback_displayed', event_info) # Form the div-wrapped hint texts - hints_wrap = u''.join( - [u'
{1}
'.format(QUESTION_HINT_TEXT_STYLE, dct.get('text')) - for dct in hint_log] + hints_wrap = HTML('').join( + [HTML('
{hint_content}
').format( + question_hint_text_style=QUESTION_HINT_TEXT_STYLE, + hint_content=HTML(dct.get('text')) + ) for dct in hint_log] ) if multiline_mode: - hints_wrap = u'
{1}
'.format(QUESTION_HINT_MULTILINE, hints_wrap) + hints_wrap = HTML('
{hints_wrap}
').format( + question_hint_multiline=QUESTION_HINT_MULTILINE, + hints_wrap=hints_wrap + ) label_wrap = '' if label: - label_wrap = u'
{1}:
'.format(QUESTION_HINT_LABEL_STYLE, label) + label_wrap = HTML('{label} ').format( + question_hint_label_style=QUESTION_HINT_LABEL_STYLE, + label=Text(label) + ) # Establish the outer style if correct: @@ -389,7 +398,12 @@ class LoncapaResponse(object): style = QUESTION_HINT_INCORRECT_STYLE # Ready to go - return u'
{1}{2}
'.format(style, label_wrap, hints_wrap) + return HTML('
{text}
{lwrp}{hintswrap}
').format( + st=style, + text=Text(_("Answer")), + lwrp=label_wrap, + hintswrap=hints_wrap + ) def get_extended_hints(self, student_answers, new_cmap): """ diff --git a/common/lib/capa/capa/templates/annotationinput.html b/common/lib/capa/capa/templates/annotationinput.html index 3e20c975a3..02c434bd78 100644 --- a/common/lib/capa/capa/templates/annotationinput.html +++ b/common/lib/capa/capa/templates/annotationinput.html @@ -17,7 +17,7 @@
${comment_prompt}
-
${tag_prompt}
+
${tag_prompt}