From 6810c785fb888598a0f888954ba4c91f19bae52d Mon Sep 17 00:00:00 2001
From: Jacek Bzdak <jacek@opencraft.com>
Date: Tue, 29 Mar 2016 23:46:12 +0200
Subject: [PATCH 1/7] Initial implementation

---
 .../public/js/mentoring_with_steps.js         | 116 ++++++++++++------
 .../tests/integration/test_step_builder.py    |   5 +-
 2 files changed, 80 insertions(+), 41 deletions(-)

diff --git a/problem_builder/public/js/mentoring_with_steps.js b/problem_builder/public/js/mentoring_with_steps.js
index 532c4f9e..752231e6 100644
--- a/problem_builder/public/js/mentoring_with_steps.js
+++ b/problem_builder/public/js/mentoring_with_steps.js
@@ -7,17 +7,18 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     var children = runtime.children(element);
+
     var steps = [];
 
     for (var i = 0; i < children.length; i++) {
         var child = children[i];
         var blockType = $(child.element).data('block-type');
         if (blockType === 'sb-step') {
-            steps.push(child);
+            registerStep(child)
         }
     }
 
-    var activeStep = $('.mentoring', element).data('active-step');
+    var activeStepIndex = $('.mentoring', element).data('active-step');
     var attemptsTemplate = _.template($('#xblock-attempts-template').html());
     var message = $('.sb-step-message', element);
     var checkmark, submitDOM, nextDOM, reviewButtonDOM, tryAgainDOM,
@@ -25,12 +26,66 @@ function MentoringWithStepsBlock(runtime, element) {
     var reviewStepDOM = $("div.xblock[data-block-type=sb-review-step], div.xblock-v1[data-block-type=sb-review-step]", element);
     var hasAReviewStep = reviewStepDOM.length == 1;
 
+    function registerStep(step) {
+        var $element = $(step.element);
+        var $anchor = $('<span class="xblock-sb-anchor"/>');
+        $anchor.insertBefore($element);
+        var step = {
+            idx: step.length,
+            $element: $element,
+            xblock: step,
+            $anchor: $anchor
+        };
+        steps.push(step);
+    }
+    
+    function getActiveStep() {
+        return getStepByIndex(activeStepIndex);
+    }
+
+    function getActiveStepBlock() {
+        return getActiveStep()['xblock'];
+    }
+
+    function getStepByIndex(idx){
+        return steps[idx];
+    }
+
+    function forEachStep(func){
+        for (var idx=0; idx<steps.length; idx++) {
+            func(steps[idx]);
+        }
+    }
+
+    function showActiveStep() {
+        var step = getActiveStep();
+        showStep(step);
+        step.xblock.updateChildren();
+    }
+
+    function showStep(step) {
+        // step.$element.insertAfter(step.$anchor);
+        step.$element.show();
+    }
+
+    function hideStep(step) {
+        step.$element.detach();
+        step.$element.insertAfter(step.$anchor);
+        step.$element.hide();
+    }
+
+    function hideAllSteps() {
+        forEachStep(function(step){
+            hideStep(step);
+        });
+    }
+
     function isLastStep() {
-        return (activeStep === steps.length-1);
+        return (activeStepIndex === steps.length-1);
     }
 
     function atReviewStep() {
-        return (activeStep === -1);
+        return (activeStepIndex === -1);
     }
 
     function someAttemptsLeft() {
@@ -49,7 +104,7 @@ function MentoringWithStepsBlock(runtime, element) {
         } else {
             checkmark.addClass('checkmark-incorrect icon-exclamation fa-exclamation');
         }
-        var step = steps[activeStep];
+        var step = getActiveStepBlock();
         if (typeof step.showFeedback == 'function') {
             step.showFeedback(response);
         }
@@ -78,14 +133,14 @@ function MentoringWithStepsBlock(runtime, element) {
     function submit() {
         submitDOM.attr('disabled', 'disabled'); // Disable the button until the results load.
         var submitUrl = runtime.handlerUrl(element, 'submit');
-
-        var hasQuestion = steps[activeStep].hasQuestion();
-        var data = steps[activeStep].getSubmitData();
-        data["active_step"] = activeStep;
+        var activeStep = getActiveStepBlock();
+        var hasQuestion = activeStep.hasQuestion();
+        var data = activeStep.getSubmitData();
+        data["active_step"] = activeStepIndex;
         $.post(submitUrl, JSON.stringify(data)).success(function(response) {
             showFeedback(response);
-            activeStep = response.active_step;
-            if (activeStep === -1) {
+            activeStepIndex = response.active_step;
+            if (activeStepIndex === -1) {
                 // We are now showing the review step / end
                 // Update the number of attempts.
                 attemptsDOM.data('num_attempts', response.num_attempts);
@@ -102,15 +157,14 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function getResults() {
-        var step = steps[activeStep];
-        step.getResults(handleReviewResults);
+        getActiveStepBlock().getResults(handleReviewResults);
     }
 
     function handleReviewResults(response) {
         // Show step-level feedback
         showFeedback(response);
         // Forward to active step to show answer level feedback
-        var step = steps[activeStep];
+        var step = getActiveStepBlock();
         var results = response.results;
         var options = {
             checkmark: checkmark
@@ -118,11 +172,6 @@ function MentoringWithStepsBlock(runtime, element) {
         step.handleReview(results, options);
     }
 
-    function hideAllSteps() {
-        for (var i=0; i < steps.length; i++) {
-            $(steps[i].element).hide();
-        }
-    }
 
     function clearSelections() {
         $('input[type=radio], input[type=checkbox]', element).prop('checked', false);
@@ -139,7 +188,7 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function updateNextLabel() {
-        var step = steps[activeStep];
+        var step = getActiveStepBlock();
         nextDOM.attr('value', step.getStepLabel());
     }
 
@@ -164,7 +213,7 @@ function MentoringWithStepsBlock(runtime, element) {
             nextDOM.on('click', updateDisplay);
             reviewButtonDOM.on('click', showGrade);
 
-            var step = steps[activeStep];
+            var step = getActiveStepBlock();
             if (step.hasQuestion()) {  // Step includes one or more questions
                 nextDOM.attr('disabled', 'disabled');
                 submitDOM.show();
@@ -231,7 +280,7 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function jumpToReview(stepIndex) {
-        activeStep = stepIndex;
+        activeStepIndex = stepIndex;
         cleanAll();
         showActiveStep();
         updateNextLabel();
@@ -245,7 +294,7 @@ function MentoringWithStepsBlock(runtime, element) {
             nextDOM.show();
             nextDOM.removeAttr('disabled');
         }
-        var step = steps[activeStep];
+        var step = getActiveStepBlock();
 
         tryAgainDOM.hide();
         if (step.hasQuestion()) {
@@ -269,11 +318,6 @@ function MentoringWithStepsBlock(runtime, element) {
         } // Don't show attempts if unlimited attempts available (max_attempts === 0)
     }
 
-    function showActiveStep() {
-        var step = steps[activeStep];
-        $(step.element).show();
-        step.updateChildren();
-    }
 
     function onChange() {
         // We do not allow users to modify answers belonging to a step after submitting them:
@@ -286,7 +330,7 @@ function MentoringWithStepsBlock(runtime, element) {
 
     function validateXBlock() {
         var isValid = true;
-        var step = steps[activeStep];
+        var step = getActiveStepBlock();
         if (step) {
             isValid = step.validate();
         }
@@ -298,15 +342,13 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function initSteps(options) {
-        for (var i=0; i < steps.length; i++) {
-            var step = steps[i];
-            var mentoring = {
+        forEachStep(function (step) {
+            options.mentoring = {
                 setContent: setContent,
                 publish_event: publishEvent
             };
-            options.mentoring = mentoring;
-            step.initChildren(options);
-        }
+            step.xblock.initChildren(options);
+        });
     }
 
     function setContent(dom, content) {
@@ -346,7 +388,7 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function reviewNextStep() {
-        jumpToReview(activeStep+1);
+        jumpToReview(activeStepIndex+1);
     }
 
     function handleTryAgain(result) {
@@ -355,7 +397,7 @@ function MentoringWithStepsBlock(runtime, element) {
         // and interrupting their experience with the current unit
         notify('navigation', {state: 'lock'});
 
-        activeStep = result.active_step;
+        activeStepIndex = result.active_step;
         clearSelections();
         updateDisplay();
         tryAgainDOM.hide();
diff --git a/problem_builder/tests/integration/test_step_builder.py b/problem_builder/tests/integration/test_step_builder.py
index 7846ed77..ad7800c6 100644
--- a/problem_builder/tests/integration/test_step_builder.py
+++ b/problem_builder/tests/integration/test_step_builder.py
@@ -624,9 +624,6 @@ def test_review_tips(self):
 
     @data(True, False)
     def test_conditional_messages(self, include_messages):
-        """
-        Test that conditional messages in the review step are visible or not, as appropriate.
-        """
         max_attempts = 3
         extended_feedback = False
         params = {
@@ -1371,7 +1368,7 @@ def test_no_review_step(self):
 
     def provide_freeform_answer(self, step_number, question_number, step_builder, text_input):
         steps = step_builder.find_elements_by_css_selector('div[data-block-type="sb-step"]')
-        current_step = steps[step_number-1]
+        current_step = steps[0]
         freeform_questions = current_step.find_elements_by_css_selector('div[data-block-type="pb-answer"]')
         current_question = freeform_questions[question_number-1]
 

From b8927bb8ec2d7c648aa211f924a49d3666a8b24d Mon Sep 17 00:00:00 2001
From: Jacek Bzdak <jacek@opencraft.com>
Date: Wed, 30 Mar 2016 15:39:34 +0200
Subject: [PATCH 2/7] Rever test changes

---
 problem_builder/tests/integration/test_step_builder.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/problem_builder/tests/integration/test_step_builder.py b/problem_builder/tests/integration/test_step_builder.py
index ad7800c6..7846ed77 100644
--- a/problem_builder/tests/integration/test_step_builder.py
+++ b/problem_builder/tests/integration/test_step_builder.py
@@ -624,6 +624,9 @@ def test_review_tips(self):
 
     @data(True, False)
     def test_conditional_messages(self, include_messages):
+        """
+        Test that conditional messages in the review step are visible or not, as appropriate.
+        """
         max_attempts = 3
         extended_feedback = False
         params = {
@@ -1368,7 +1371,7 @@ def test_no_review_step(self):
 
     def provide_freeform_answer(self, step_number, question_number, step_builder, text_input):
         steps = step_builder.find_elements_by_css_selector('div[data-block-type="sb-step"]')
-        current_step = steps[0]
+        current_step = steps[step_number-1]
         freeform_questions = current_step.find_elements_by_css_selector('div[data-block-type="pb-answer"]')
         current_question = freeform_questions[question_number-1]
 

From c9651b6074018bab47523e360812c0183b8e0a2d Mon Sep 17 00:00:00 2001
From: Jacek Bzdak <jacek@opencraft.com>
Date: Wed, 30 Mar 2016 16:12:21 +0200
Subject: [PATCH 3/7] Some renames

---
 .../public/js/mentoring_with_steps.js         | 21 +++++++++----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/problem_builder/public/js/mentoring_with_steps.js b/problem_builder/public/js/mentoring_with_steps.js
index 752231e6..f58be3a8 100644
--- a/problem_builder/public/js/mentoring_with_steps.js
+++ b/problem_builder/public/js/mentoring_with_steps.js
@@ -43,8 +43,8 @@ function MentoringWithStepsBlock(runtime, element) {
         return getStepByIndex(activeStepIndex);
     }
 
-    function getActiveStepBlock() {
-        return getActiveStep()['xblock'];
+    function getActiveBlock() {
+        return getActiveStep().xblock;
     }
 
     function getStepByIndex(idx){
@@ -64,7 +64,6 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function showStep(step) {
-        // step.$element.insertAfter(step.$anchor);
         step.$element.show();
     }
 
@@ -104,7 +103,7 @@ function MentoringWithStepsBlock(runtime, element) {
         } else {
             checkmark.addClass('checkmark-incorrect icon-exclamation fa-exclamation');
         }
-        var step = getActiveStepBlock();
+        var step = getActiveBlock();
         if (typeof step.showFeedback == 'function') {
             step.showFeedback(response);
         }
@@ -133,7 +132,7 @@ function MentoringWithStepsBlock(runtime, element) {
     function submit() {
         submitDOM.attr('disabled', 'disabled'); // Disable the button until the results load.
         var submitUrl = runtime.handlerUrl(element, 'submit');
-        var activeStep = getActiveStepBlock();
+        var activeStep = getActiveBlock();
         var hasQuestion = activeStep.hasQuestion();
         var data = activeStep.getSubmitData();
         data["active_step"] = activeStepIndex;
@@ -157,14 +156,14 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function getResults() {
-        getActiveStepBlock().getResults(handleReviewResults);
+        getActiveBlock().getResults(handleReviewResults);
     }
 
     function handleReviewResults(response) {
         // Show step-level feedback
         showFeedback(response);
         // Forward to active step to show answer level feedback
-        var step = getActiveStepBlock();
+        var step = getActiveBlock();
         var results = response.results;
         var options = {
             checkmark: checkmark
@@ -188,7 +187,7 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function updateNextLabel() {
-        var step = getActiveStepBlock();
+        var step = getActiveBlock();
         nextDOM.attr('value', step.getStepLabel());
     }
 
@@ -213,7 +212,7 @@ function MentoringWithStepsBlock(runtime, element) {
             nextDOM.on('click', updateDisplay);
             reviewButtonDOM.on('click', showGrade);
 
-            var step = getActiveStepBlock();
+            var step = getActiveBlock();
             if (step.hasQuestion()) {  // Step includes one or more questions
                 nextDOM.attr('disabled', 'disabled');
                 submitDOM.show();
@@ -294,7 +293,7 @@ function MentoringWithStepsBlock(runtime, element) {
             nextDOM.show();
             nextDOM.removeAttr('disabled');
         }
-        var step = getActiveStepBlock();
+        var step = getActiveBlock();
 
         tryAgainDOM.hide();
         if (step.hasQuestion()) {
@@ -330,7 +329,7 @@ function MentoringWithStepsBlock(runtime, element) {
 
     function validateXBlock() {
         var isValid = true;
-        var step = getActiveStepBlock();
+        var step = getActiveBlock();
         if (step) {
             isValid = step.validate();
         }

From d356500a8d582cc1bcbb7f57440e511a998d81e5 Mon Sep 17 00:00:00 2001
From: Jacek Bzdak <jacek@opencraft.com>
Date: Wed, 30 Mar 2016 16:23:54 +0200
Subject: [PATCH 4/7] Code cleanups and documentation

---
 .../public/js/mentoring_with_steps.js         | 98 +++++++++++++------
 1 file changed, 66 insertions(+), 32 deletions(-)

diff --git a/problem_builder/public/js/mentoring_with_steps.js b/problem_builder/public/js/mentoring_with_steps.js
index f58be3a8..8dc977dd 100644
--- a/problem_builder/public/js/mentoring_with_steps.js
+++ b/problem_builder/public/js/mentoring_with_steps.js
@@ -14,7 +14,7 @@ function MentoringWithStepsBlock(runtime, element) {
         var child = children[i];
         var blockType = $(child.element).data('block-type');
         if (blockType === 'sb-step') {
-            registerStep(child)
+            registerStep(child);
         }
     }
 
@@ -25,54 +25,88 @@ function MentoringWithStepsBlock(runtime, element) {
         gradeDOM, attemptsDOM, reviewLinkDOM, submitXHR;
     var reviewStepDOM = $("div.xblock[data-block-type=sb-review-step], div.xblock-v1[data-block-type=sb-review-step]", element);
     var hasAReviewStep = reviewStepDOM.length == 1;
-
+    
+    /**
+     * Registers step to in this builder, it stores the xblock element, and some additional metadata used to safely
+     * remove and re-attach xblock to the DOM. See showStep and hideStep to see why we need to do it.
+     * @param step xblock instance to register
+     */
     function registerStep(step) {
         var $element = $(step.element);
         var $anchor = $('<span class="xblock-sb-anchor"/>');
         $anchor.insertBefore($element);
-        var step = {
+        var step_wrapper = {
             idx: step.length,
             $element: $element,
             xblock: step,
             $anchor: $anchor
         };
-        steps.push(step);
-    }
-    
-    function getActiveStep() {
-        return getStepByIndex(activeStepIndex);
+        steps.push(step_wrapper);
     }
 
-    function getActiveBlock() {
-        return getActiveStep().xblock;
+    /**
+     * Returns wrapper for active step
+     * @returns {*}, an object containing xblock, and additional metadata (see registerStep where this object is created
+     * for properties)
+     */
+    function getWrapperForActiveStep() {
+        return steps[activeStepIndex](activeStepIndex);
     }
 
-    function getStepByIndex(idx){
-        return steps[idx];
+    /**
+     * Returns active step)
+     * @returns {*}
+     */
+    function getActiveStep() {
+        return getWrapperForActiveStep().xblock;
     }
 
+    /**
+     * Calls a function for each registered step. Object passed to this function is an step wrapper object
+     * (see registerStep where this object is created a list of properties)
+     *
+     * @param func single arg function.
+     */
     function forEachStep(func){
         for (var idx=0; idx<steps.length; idx++) {
             func(steps[idx]);
         }
     }
 
+    /**
+     * Displays a step
+     * @param step_wrapper 
+     */
+    function showStep(step_wrapper) {
+        step_wrapper.$element.show();
+        step_wrapper.xblock.updateChildren();
+    }
+
+    /**
+     * Hides a step
+     * @param step_wrapper
+     */
+    function hideStep(step_wrapper) {
+        // This is a hacky workaround, but it works. It detaches this element from dom, and re-attaches it immediately,
+        // which has the side effect of stopping any video elements (tested with Ooyala and Youtube). 
+        // This solution was chosen as permanently detaching xblocks from DOM is not anticipated by lot of software 
+        // and some code in this repository. 
+        step_wrapper.$element.detach();
+        step_wrapper.$element.insertAfter(step_wrapper.$anchor);
+        step_wrapper.$element.hide();
+    }
+
+    /**
+     * Displays active step
+     */
     function showActiveStep() {
-        var step = getActiveStep();
+        var step = getWrapperForActiveStep();
         showStep(step);
-        step.xblock.updateChildren();
-    }
-
-    function showStep(step) {
-        step.$element.show();
-    }
-
-    function hideStep(step) {
-        step.$element.detach();
-        step.$element.insertAfter(step.$anchor);
-        step.$element.hide();
     }
 
+    /**
+     * Hides all steps
+     */
     function hideAllSteps() {
         forEachStep(function(step){
             hideStep(step);
@@ -103,7 +137,7 @@ function MentoringWithStepsBlock(runtime, element) {
         } else {
             checkmark.addClass('checkmark-incorrect icon-exclamation fa-exclamation');
         }
-        var step = getActiveBlock();
+        var step = getActiveStep();
         if (typeof step.showFeedback == 'function') {
             step.showFeedback(response);
         }
@@ -132,7 +166,7 @@ function MentoringWithStepsBlock(runtime, element) {
     function submit() {
         submitDOM.attr('disabled', 'disabled'); // Disable the button until the results load.
         var submitUrl = runtime.handlerUrl(element, 'submit');
-        var activeStep = getActiveBlock();
+        var activeStep = getActiveStep();
         var hasQuestion = activeStep.hasQuestion();
         var data = activeStep.getSubmitData();
         data["active_step"] = activeStepIndex;
@@ -156,14 +190,14 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function getResults() {
-        getActiveBlock().getResults(handleReviewResults);
+        getActiveStep().getResults(handleReviewResults);
     }
 
     function handleReviewResults(response) {
         // Show step-level feedback
         showFeedback(response);
         // Forward to active step to show answer level feedback
-        var step = getActiveBlock();
+        var step = getActiveStep();
         var results = response.results;
         var options = {
             checkmark: checkmark
@@ -187,7 +221,7 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function updateNextLabel() {
-        var step = getActiveBlock();
+        var step = getActiveStep();
         nextDOM.attr('value', step.getStepLabel());
     }
 
@@ -212,7 +246,7 @@ function MentoringWithStepsBlock(runtime, element) {
             nextDOM.on('click', updateDisplay);
             reviewButtonDOM.on('click', showGrade);
 
-            var step = getActiveBlock();
+            var step = getActiveStep();
             if (step.hasQuestion()) {  // Step includes one or more questions
                 nextDOM.attr('disabled', 'disabled');
                 submitDOM.show();
@@ -293,7 +327,7 @@ function MentoringWithStepsBlock(runtime, element) {
             nextDOM.show();
             nextDOM.removeAttr('disabled');
         }
-        var step = getActiveBlock();
+        var step = getActiveStep();
 
         tryAgainDOM.hide();
         if (step.hasQuestion()) {
@@ -329,7 +363,7 @@ function MentoringWithStepsBlock(runtime, element) {
 
     function validateXBlock() {
         var isValid = true;
-        var step = getActiveBlock();
+        var step = getActiveStep();
         if (step) {
             isValid = step.validate();
         }

From 8b76fb2fd65abc5db2f204263062dc2831e3a1ea Mon Sep 17 00:00:00 2001
From: Jacek Bzdak <jacek@opencraft.com>
Date: Wed, 30 Mar 2016 16:37:06 +0200
Subject: [PATCH 5/7] Fix stupid typo

---
 problem_builder/public/js/mentoring_with_steps.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/problem_builder/public/js/mentoring_with_steps.js b/problem_builder/public/js/mentoring_with_steps.js
index 8dc977dd..c6f499a5 100644
--- a/problem_builder/public/js/mentoring_with_steps.js
+++ b/problem_builder/public/js/mentoring_with_steps.js
@@ -50,7 +50,7 @@ function MentoringWithStepsBlock(runtime, element) {
      * for properties)
      */
     function getWrapperForActiveStep() {
-        return steps[activeStepIndex](activeStepIndex);
+        return steps[activeStepIndex];
     }
 
     /**

From 2849e6d2e955cdcf05b288e4054ea254d40cd0dc Mon Sep 17 00:00:00 2001
From: Jacek Bzdak <jacek@opencraft.com>
Date: Wed, 30 Mar 2016 16:47:17 +0200
Subject: [PATCH 6/7] Fixed a pronoun

---
 problem_builder/public/js/mentoring_with_steps.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/problem_builder/public/js/mentoring_with_steps.js b/problem_builder/public/js/mentoring_with_steps.js
index c6f499a5..ab64d4a3 100644
--- a/problem_builder/public/js/mentoring_with_steps.js
+++ b/problem_builder/public/js/mentoring_with_steps.js
@@ -97,7 +97,7 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     /**
-     * Displays active step
+     * Displays the active step
      */
     function showActiveStep() {
         var step = getWrapperForActiveStep();

From 020ab935720915d5dff84eb65f98ce0aa5612ff9 Mon Sep 17 00:00:00 2001
From: Jacek Bzdak <jacek@opencraft.com>
Date: Wed, 30 Mar 2016 22:30:35 +0200
Subject: [PATCH 7/7] PR Review fixes

---
 .../public/js/mentoring_with_steps.js         | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/problem_builder/public/js/mentoring_with_steps.js b/problem_builder/public/js/mentoring_with_steps.js
index ab64d4a3..ffd313a9 100644
--- a/problem_builder/public/js/mentoring_with_steps.js
+++ b/problem_builder/public/js/mentoring_with_steps.js
@@ -27,7 +27,8 @@ function MentoringWithStepsBlock(runtime, element) {
     var hasAReviewStep = reviewStepDOM.length == 1;
     
     /**
-     * Registers step to in this builder, it stores the xblock element, and some additional metadata used to safely
+     * Registers step with this StepBuilder instance: Creates and stores a wrapper that contains the XBlock instance
+     * and associated DOM element, as well as some additional metadata used to safely
      * remove and re-attach xblock to the DOM. See showStep and hideStep to see why we need to do it.
      * @param step xblock instance to register
      */
@@ -36,7 +37,6 @@ function MentoringWithStepsBlock(runtime, element) {
         var $anchor = $('<span class="xblock-sb-anchor"/>');
         $anchor.insertBefore($element);
         var step_wrapper = {
-            idx: step.length,
             $element: $element,
             xblock: step,
             $anchor: $anchor
@@ -45,16 +45,16 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     /**
-     * Returns wrapper for active step
-     * @returns {*}, an object containing xblock, and additional metadata (see registerStep where this object is created
-     * for properties)
+     * Returns wrapper for the active step
+     * @returns {*}, an object containing the XBlock instance and associated DOM element for the active step, as well
+     * as additional metadata (see registerStep where this object is created for properties)
      */
     function getWrapperForActiveStep() {
         return steps[activeStepIndex];
     }
 
     /**
-     * Returns active step)
+     * Returns the active step
      * @returns {*}
      */
     function getActiveStep() {
@@ -62,13 +62,13 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     /**
-     * Calls a function for each registered step. Object passed to this function is an step wrapper object
-     * (see registerStep where this object is created a list of properties)
+     * Calls a function for each registered step. The object passed to this function is a step wrapper object
+     * (see registerStep where this object is created for a list of properties)
      *
      * @param func single arg function.
      */
     function forEachStep(func){
-        for (var idx=0; idx<steps.length; idx++) {
+        for (var idx=0; idx < steps.length; idx++) {
             func(steps[idx]);
         }
     }
@@ -87,10 +87,12 @@ function MentoringWithStepsBlock(runtime, element) {
      * @param step_wrapper
      */
     function hideStep(step_wrapper) {
-        // This is a hacky workaround, but it works. It detaches this element from dom, and re-attaches it immediately,
+        // This is a hacky workaround, but it works. It detaches this element from DOM, and re-attaches it immediately,
         // which has the side effect of stopping any video elements (tested with Ooyala and Youtube). 
-        // This solution was chosen as permanently detaching xblocks from DOM is not anticipated by lot of software 
-        // and some code in this repository. 
+        // This solution was chosen as permanently detaching xblocks from DOM is not anticipated by:
+        //  * Selenium tests
+        //  * Some javascript code here
+        //  * Anything else that inspects DOM elements and makes choices         
         step_wrapper.$element.detach();
         step_wrapper.$element.insertAfter(step_wrapper.$anchor);
         step_wrapper.$element.hide();