Merge pull request #69 from open-craft/ext-feedback-links-polish-oc-986

Fixed appearance of step links in extended feedback

problem_builder/public/css/problem-builder.css

@@ -126,7 +126,6 @@
 
 .mentoring .grade .checkmark-incorrect {
     margin-left: 10px;
-    margin-right: 20px;
 }
 
 .mentoring input[type=button],
@@ -159,11 +158,12 @@
 }
 
 .mentoring .results-section {
-    float: left;
+    margin-left: 40px;
 }
 
 .mentoring .results-section p {
-    margin: 4px;
+    margin-bottom: 4px;
+    padding-top: 4px;
 }
 
 .mentoring .clear {

problem_builder/public/themes/lms.css

@@ -103,3 +103,11 @@ div.course-wrapper section.course-content .themed-xblock.mentoring p:empty {
 .themed-xblock.mentoring .choice.incorrect .choice-tips-container.active {
     border-color: #c1373f;
 }
+
+.themed-xblock.mentoring .review-list {
+    margin-top: 0;
+}
+
+.themed-xblock.mentoring .grade .grade-result .results-section p {
+    margin-bottom: 4px; /* Override LMS rule 'div.course-wrapper section.course-content p { margin-bottom: huge; }' */
+}

problem_builder/templates/html/review_step.html

@@ -57,11 +57,19 @@
 
   <!-- Template for extended feedback: Show extended feedback details when all attempts are used up. -->
   <script type="text/template" id="xblock-step-links-template">
-  <% var q, last_question; %>
   <ul class="review-list <%= correctness %>-list">
-      <% for (var question in questions) {{ q = questions[question]; last_question = question == questions.length - 1; %>
-      <li><a href="#" class="step-link" data-step="<%= q.step %>"><%= _.template(gettext("Question {number}"), {number: q.number}, {interpolate: /\{(.+?)\}/g}) %></a></li>
-      <% }} %>
+      <% for (var question in questions) { %>
+        <%
+          var q = questions[question];
+          var last_question = question == questions.length - 1;
+          var second_last_question = question == questions.length - 2;
+        %>
+        <li>
+          <a href="#" class="step-link" data-step="<%= q.step %>"><%= 
+            _.template(gettext("Question {number}"), {number: q.number}, {interpolate: /\{(.+?)\}/g})
+          %></a><% if (!last_question) { %><%= (questions.length > 2 ? ", " : "") %><%= (second_last_question ? " " + gettext("and"): "") %><% } %>
+        </li>
+      <% } %>
   </ul>
   </script>
 

problem_builder/tests/integration/test_step_builder.py

@@ -264,6 +264,8 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
             "num_attempts": 1, "max_attempts": max_attempts
         }
         self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
+        if extended_feedback and max_attempts == 1:
+            self.assertIn("Question 1 and Question 3", step_builder.find_element_by_css_selector('.correct-list').text)
 
         if max_attempts == 1:
             self.assert_message_text(step_builder, "On review message text")
@@ -302,6 +304,11 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
             "num_attempts": 2, "max_attempts": max_attempts
         }
         self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
+        if extended_feedback and max_attempts == 2:
+            self.assertIn(
+                "Question 1, Question 2, and Question 4",
+                step_builder.find_element_by_css_selector('.correct-list').text
+            )
 
         if max_attempts == 2:
             self.assert_disabled(controls.try_again)

test_requirements.txt

 -e git+https://github.com/edx/XBlock.git@tag-master-2015-05-22#egg=XBlock
+selenium==2.47.3  # 2.48 is not working atm