Fix scrolling for steps without questions.

Also includes a drive-by fix that makes integration tests for plot
blocks more robust.

problem_builder/public/js/mentoring_with_steps.js

@@ -158,19 +158,47 @@ function MentoringWithStepsBlock(runtime, element) {
             showActiveStep();
             validateXBlock();
             updateNextLabel();
+
+            // Reinstate default event handlers
+            nextDOM.on('click', updateDisplay);
+            reviewButtonDOM.on('click', showGrade);
+
             var step = steps[activeStep];
-            if (step.hasQuestion()) {
+            if (step.hasQuestion()) {  // Step includes one or more questions
                 nextDOM.attr('disabled', 'disabled');
-            } else {
+                submitDOM.show();
+                if (isLastStep()) {  // Step is last step
+                    nextDOM.hide();
+                    if (hasAReviewStep) {  // Step Builder includes review step
+                        reviewButtonDOM.attr('disabled', 'disabled');
+                        reviewButtonDOM.show();
+                    }
+                }
+            } else {  // Step does not include any questions
                 nextDOM.removeAttr('disabled');
-            }
-            if (isLastStep() && hasAReviewStep) {
-                if (step.hasQuestion()) {
-                    reviewButtonDOM.attr('disabled', 'disabled');
-                } else {
+                submitDOM.hide();
+                if (isLastStep()) {  // Step is last step
+                    // Remove default event handler from button that displays review.
+                    // This is necessary to make sure updateDisplay is not called twice
+                    // when user clicks this button next;
+                    // "submit" already does the right thing w/r/t updating the display,
+                    // and calling updateDisplay twice causes issues with scrolling behavior:
+                    reviewButtonDOM.off();
+                    reviewButtonDOM.one('click', submit);
                     reviewButtonDOM.removeAttr('disabled');
+                    nextDOM.hide();
+                    if (hasAReviewStep) {  // Step Builder includes review step
+                        reviewButtonDOM.show();
+                    }
+                } else {  // Step is not last step
+                    // Remove default event handler from button that displays next step.
+                    // This is necessary to make sure updateDisplay is not called twice
+                    // when user clicks this button next;
+                    // "submit" already does the right thing w/r/t updating the display,
+                    // and calling updateDisplay twice causes issues with scrolling behavior:
+                    nextDOM.off();
+                    nextDOM.one('click', submit);
                 }
-                reviewButtonDOM.show();
             }
         }
 
@@ -266,20 +294,6 @@ function MentoringWithStepsBlock(runtime, element) {
         } else {
             submitDOM.removeAttr('disabled');
         }
-        if (isLastStep() && step.hasQuestion()) {
-            nextDOM.hide();
-        } else if (isLastStep()) {
-            reviewButtonDOM.one('click', submit);
-            reviewButtonDOM.removeAttr('disabled');
-            nextDOM.hide()
-        } else if (!step.hasQuestion()) {
-            nextDOM.one('click', submit);
-        }
-        if (step.hasQuestion()) {
-            submitDOM.show();
-        } else {
-            submitDOM.hide();
-        }
     }
 
     function initSteps(options) {

problem_builder/tests/integration/test_step_builder.py

@@ -659,14 +659,17 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         )
         choices.select(choice_name)
 
-    def submit_and_go_to_next_step(self, controls, last=False):
+    def submit_and_go_to_next_step(self, controls, last=False, no_questions=False):
         controls.submit.click()
         self.wait_until_clickable(controls.next_question)
         controls.next_question.click()
         if last:
             self.wait_until_hidden(controls.next_question)
         else:
-            self.wait_until_disabled(controls.next_question)
+            if no_questions:
+                self.wait_until_hidden(controls.submit)
+            else:
+                self.wait_until_disabled(controls.next_question)
 
     def plot_controls(self, step_builder):
         class Namespace(object):
@@ -779,9 +782,19 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         plot_controls.quadrants_button.click()
         self.check_quadrant_labels(step_builder, plot_controls, hidden=False)
 
+    def wait_for_multiple_elements(self, step_builder, selector, expected_number_of_elements):
+        for second in range(self.timeout):  # pylint: disable=unused-variable
+            elements = step_builder.find_elements_by_css_selector(selector)
+            if not len(elements) == expected_number_of_elements:
+                time.sleep(1)
+            else:
+                return
+        self.fail("{} elements not present after {} seconds. Selector: '{}'".format(
+            expected_number_of_elements, self.timeout, selector
+        ))
+
     def check_overlays(self, step_builder, total_num_points, overlays):
-        points = step_builder.find_elements_by_css_selector("circle")
-        self.assertEquals(len(points), total_num_points)
+        self.wait_for_multiple_elements(step_builder, "circle", total_num_points)
 
         for overlay in overlays:
             # Check if correct number of points is present
@@ -1322,6 +1335,10 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         )
         self.assertAlmostEqual(scroll_top, step_builder_offset, delta=1)
 
+    def scroll_down(self):
+        self.browser.execute_script("$(window).scrollTop(50)")
+        time.sleep(1)
+
     def test_scroll_into_view(self):
         step_builder, controls = self.load_assessment_scenario("step_builder_long_steps.xml", {})
         # First step
@@ -1333,15 +1350,19 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         self.provide_freeform_answer(1, 4, step_builder, "This is the answer")
         self.provide_freeform_answer(1, 5, step_builder, "This is the answer")
         # - Submit and go to next step
-        self.submit_and_go_to_next_step(controls, last=True)
+        self.submit_and_go_to_next_step(controls, no_questions=True)
+        # Second step
+        self.check_viewport()
+        self.scroll_down()
+        self.html_section(step_builder, controls)
         # Last step
         self.check_viewport()
         # - Answer questions
-        self.provide_freeform_answer(2, 1, step_builder, "This is the answer")
-        self.provide_freeform_answer(2, 2, step_builder, "This is the answer")
-        self.provide_freeform_answer(2, 3, step_builder, "This is the answer")
-        self.provide_freeform_answer(2, 4, step_builder, "This is the answer")
-        self.provide_freeform_answer(2, 5, step_builder, "This is the answer")
+        self.provide_freeform_answer(3, 1, step_builder, "This is the answer")
+        self.provide_freeform_answer(3, 2, step_builder, "This is the answer")
+        self.provide_freeform_answer(3, 3, step_builder, "This is the answer")
+        self.provide_freeform_answer(3, 4, step_builder, "This is the answer")
+        self.provide_freeform_answer(3, 5, step_builder, "This is the answer")
         # - Submit and go to review step
         self.submit_and_go_to_review_step(step_builder, controls, result=CORRECT)
         # Review step
@@ -1350,21 +1371,31 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         # - Review questions belonging to first step
         question_links[2].click()
         self.check_viewport()
+        self.scroll_down()
         # - Jump to review step
         controls.review_link.click()
         self.check_viewport()
+        self.scroll_down()
         # - Review questions belonging to last step
         question_links[7].click()
         self.check_viewport()
+        self.scroll_down()
         # - Jump to review step
         controls.review_link.click()
         self.check_viewport()
+        self.scroll_down()
         # - Review questions belonging to first step
         question_links[2].click()
         self.check_viewport()
+        self.scroll_down()
+        # - Navigate to second step
+        controls.next_question.click()
+        self.check_viewport()
+        self.scroll_down()
         # - Review questions belonging to last step
         controls.next_question.click()
         self.check_viewport()
+        self.scroll_down()
         # - Navigate to review step
         controls.review.click()
         self.check_viewport()

problem_builder/tests/integration/xml_templates/step_builder_long_steps.xml

@@ -9,7 +9,11 @@
     <pb-answer name="goal" question="What is your goal?" />
   </sb-step>
 
-  <sb-step display_name="Second step">
+  <sb-step display_name="Second Step">
+    <html_demo>Test test test</html_demo>
+  </sb-step>
+
+  <sb-step display_name="Last step">
     <pb-answer name="goal" question="What is your goal?" />
     <pb-answer name="goal" question="What is your goal?" />
     <pb-answer name="goal" question="What is your goal?" />