From 5a0c5535c6fb46f91ee472ad3fb72fa4b27b811f Mon Sep 17 00:00:00 2001 From: Jacek Bzdak Date: Tue, 29 Mar 2016 23:46:12 +0200 Subject: [PATCH 1/3] Detach HTML elements from the DOM when hiding a step. --- problem_builder/mentoring.py | 1 + problem_builder/public/js/lms_util.js | 122 ++++++++++++++++++ .../public/js/mentoring_with_steps.js | 108 ++++++++++------ problem_builder/public/js/step.js | 32 ++--- .../tests/integration/test_step_builder.py | 4 +- 5 files changed, 207 insertions(+), 60 deletions(-) create mode 100644 problem_builder/public/js/lms_util.js diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index 5f4420bf..d20848c6 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -1027,6 +1027,7 @@ def student_view(self, context): })) fragment.add_css_url(self.runtime.local_resource_url(self, 'public/css/problem-builder.css')) fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/vendor/underscore-min.js')) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/lms_util.js')) fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/mentoring_with_steps.js')) fragment.add_resource(loader.load_unicode('templates/html/mentoring_attempts.html'), "text/html") diff --git a/problem_builder/public/js/lms_util.js b/problem_builder/public/js/lms_util.js new file mode 100644 index 00000000..d2f62dc9 --- /dev/null +++ b/problem_builder/public/js/lms_util.js @@ -0,0 +1,122 @@ +(function () { + + /** + * Manager for HTML XBlocks. These blocks are hid by detaching and shown + * by re-attaching them to the DOM.This is only generic way to generically + * handle things like video players (they should stop playing when removed from DOM). + * + * @param html an html xblock + */ + function HtmlManager(html) { + var $element = $(html.element); + var $anchor = $("").addClass("sb-video-anchor").insertBefore($element); + this.show = function () { + $element.insertAfter($anchor); + }; + this.hide = function () { + $element.detach() + }; + } + + /** + * + * Manager for HTML Video child. Videos are paused when hiding them, and re-sized when showing them. + * @param video an video xblock + * + */ + function VideoManager(video) { + this.show = function () { + if (typeof video.resizer === 'undefined'){ + // This one is tricky: but it looks like resizer is undefined only if the video is on the + // step that is initially visible (and then no resizing is necessary) + return; + } + video.resizer.align(); + }; + this.hide = function () { + if (typeof video.videoPlayer === 'undefined'){ + // If video player is undefined this means that the video isn't loaded yet, so no need to + // pause it. + return; + } + video.videoPlayer.pause(); + }; + } + + /** + * Manager for Plot Xblocks. Handles updating a plot before displaying it. + * @param plot + */ + function PlotManager(plot) { + this.show = function () { + plot.update(); + }; + this.hide = function () {}; + } + + + function ChildManager(xblock_element, runtime) { + + var Managers = { + 'video': VideoManager, + 'sb-plot': PlotManager + }; + + var children = runtime.children(xblock_element); + + /** + * A list of managers for children than need special care when showing or hiding. + * + * @type {show, hide}[] + */ + var managedChildren = []; + + /*** + * This is a workaround for issue where jquery.xblock.Runtime doesn't return HTML Xblocks when querying + * for children. + * + * This can be removed when: + * + * * We allow inclusion of Ooyala blocks inside ProblemBuilder and our clients migrate to Ooyala, in this case + * we may drop special handling of HTML blocks. See discussions in OC-1441. + * * We include HTML blocks in runtime.children for runtime of jquery.xblock, then just add + * `html: HtmlManager` to `Managers`, and remove this block. + */ + $("div.xblock.xblock-student_view.xmodule_HtmlModule", xblock_element).each(function (idx, element) { + managedChildren.push(new HtmlManager({element:element})); + }); + + for (var idx = 0; idx < children.length; idx++){ + var child = children[idx]; + // NOTE: While following assertion is true for e.g Video blocks: + // child.type == $(child.element).data('block-type') it is invalidated by for all sb-* blocks + var type = $(child.element).data('block-type'); + var constructor = Managers[type]; + if (typeof constructor === 'undefined'){ + // This block does not requires special care, moving on + continue ; + } + managedChildren.push(new constructor(child)); + } + + this.show = function () { + for (var idx = 0; idx").addClass("review-anchor").insertBefore(reviewStepDOM); var hasAReviewStep = reviewStepDOM.length == 1; + /** + * Returns the active step + * @returns MentoringStepBlock + */ + function getActiveStep() { + return steps[activeStepIndex]; + } + + /** + * Calls a function for each registered step. The object passed to this function is a MentoringStepBlock. + * + * @param func single arg function. + */ + function forEachStep(func){ + for (var idx=0; idx < steps.length; idx++) { + func(steps[idx]); + } + } + + /** + * Displays the active step + */ + function showActiveStep() { + var step = getActiveStep(); + step.show(); + } + + /** + * Hides all steps + */ + function hideAllSteps() { + forEachStep(function(step){ + step.hide(); + }); + } + function isLastStep() { - return (activeStep === steps.length-1); + return (activeStepIndex === steps.length-1); } function atReviewStep() { - return (activeStep === -1); + return (activeStepIndex === -1); } function someAttemptsLeft() { @@ -49,7 +87,7 @@ function MentoringWithStepsBlock(runtime, element) { } else { checkmark.addClass('checkmark-incorrect icon-exclamation fa-exclamation'); } - var step = steps[activeStep]; + var step = getActiveStep(); if (typeof step.showFeedback == 'function') { step.showFeedback(response); } @@ -78,14 +116,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 = getActiveStep(); + 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 +140,14 @@ function MentoringWithStepsBlock(runtime, element) { } function getResults() { - var step = steps[activeStep]; - step.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 = steps[activeStep]; + var step = getActiveStep(); var results = response.results; var options = { checkmark: checkmark @@ -118,14 +155,11 @@ 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); + forEachStep(function (step) { + $('input[type=radio], input[type=checkbox]', step.element).prop('checked', false); + }); } function cleanAll() { @@ -139,7 +173,7 @@ function MentoringWithStepsBlock(runtime, element) { } function updateNextLabel() { - var step = steps[activeStep]; + var step = getActiveStep(); nextDOM.attr('value', step.getStepLabel()); } @@ -164,7 +198,7 @@ function MentoringWithStepsBlock(runtime, element) { nextDOM.on('click', updateDisplay); reviewButtonDOM.on('click', showGrade); - var step = steps[activeStep]; + var step = getActiveStep(); if (step.hasQuestion()) { // Step includes one or more questions nextDOM.attr('disabled', 'disabled'); submitDOM.show(); @@ -217,11 +251,20 @@ function MentoringWithStepsBlock(runtime, element) { reviewButtonDOM.hide(); tryAgainDOM.show(); + /** + * We detach review step from DOM, this is required to handle HTML + * blocks that can be added to the Review step. + * + * NOTE: This is handled differently than step js. As the html contents + * of review step are replaced with fresh contents in submit function. + */ + reviewStepDOM.insertBefore(reviewStepAnchor); reviewStepDOM.show(); } function hideReviewStep() { reviewStepDOM.hide(); + reviewStepDOM.detach(); } function getStepToReview(event) { @@ -231,7 +274,7 @@ function MentoringWithStepsBlock(runtime, element) { } function jumpToReview(stepIndex) { - activeStep = stepIndex; + activeStepIndex = stepIndex; cleanAll(); showActiveStep(); updateNextLabel(); @@ -245,7 +288,7 @@ function MentoringWithStepsBlock(runtime, element) { nextDOM.show(); nextDOM.removeAttr('disabled'); } - var step = steps[activeStep]; + var step = getActiveStep(); tryAgainDOM.hide(); if (step.hasQuestion()) { @@ -269,11 +312,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 +324,7 @@ function MentoringWithStepsBlock(runtime, element) { function validateXBlock() { var isValid = true; - var step = steps[activeStep]; + var step = getActiveStep(); if (step) { isValid = step.validate(); } @@ -298,16 +336,14 @@ 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, is_step_builder: true }; - options.mentoring = mentoring; step.initChildren(options); - } + }); } function setContent(dom, content) { @@ -347,7 +383,7 @@ function MentoringWithStepsBlock(runtime, element) { } function reviewNextStep() { - jumpToReview(activeStep+1); + jumpToReview(activeStepIndex+1); } function handleTryAgain(result) { @@ -356,7 +392,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/public/js/step.js b/problem_builder/public/js/step.js index 38993dc2..b0c96ec6 100644 --- a/problem_builder/public/js/step.js +++ b/problem_builder/public/js/step.js @@ -4,6 +4,8 @@ function MentoringStepBlock(runtime, element) { var submitXHR, resultsXHR, message = $(element).find('.sb-step-message'); + + var childManager = new ProblemBuilderUtilLMS.ChildManager(element, runtime); function callIfExists(obj, fn) { if (typeof obj !== 'undefined' && typeof obj[fn] == 'function') { @@ -13,13 +15,6 @@ function MentoringStepBlock(runtime, element) { } } - function updateVideo(video) { - video.resizer.align(); - } - - function updatePlot(plot) { - plot.update(); - } return { @@ -56,7 +51,7 @@ function MentoringStepBlock(runtime, element) { }, showFeedback: function(response) { - // Called when user has just submitted an answer or is reviewing their answer durign extended feedback. + // Called when user has just submitted an answer or is reviewing their answer during extended feedback. if (message.length) { message.fadeIn(); $(document).click(function() { @@ -110,20 +105,15 @@ function MentoringStepBlock(runtime, element) { return $('.sb-step', element).data('has-question'); }, - updateChildren: function() { - children.forEach(function(child) { - var type = $(child.element).data('block-type'); - switch (type) { - case 'video': - updateVideo(child); - break; - case 'sb-plot': - updatePlot(child); - break; - } - }); - } + show: function () { + $(element).show(); + childManager.show(); + }, + hide: function () { + $(element).hide(); + childManager.hide(); + } }; } diff --git a/problem_builder/tests/integration/test_step_builder.py b/problem_builder/tests/integration/test_step_builder.py index 7846ed77..7657b552 100644 --- a/problem_builder/tests/integration/test_step_builder.py +++ b/problem_builder/tests/integration/test_step_builder.py @@ -624,9 +624,7 @@ 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. - """ + # Test that conditional messages in the review step are visible or not, as appropriate. max_attempts = 3 extended_feedback = False params = { From 2af1ddcaa3ab4b830aeb2014be0a1edd7d8ac228 Mon Sep 17 00:00:00 2001 From: Jacek Bzdak Date: Thu, 7 Apr 2016 13:10:38 +0200 Subject: [PATCH 2/3] Removed special handling when hiding video blocks. --- problem_builder/public/js/lms_util.js | 48 +++++++++---------- .../public/js/mentoring_with_steps.js | 29 +++++------ problem_builder/public/js/step.js | 10 +++- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/problem_builder/public/js/lms_util.js b/problem_builder/public/js/lms_util.js index d2f62dc9..b2e8fec0 100644 --- a/problem_builder/public/js/lms_util.js +++ b/problem_builder/public/js/lms_util.js @@ -1,8 +1,8 @@ (function () { /** - * Manager for HTML XBlocks. These blocks are hid by detaching and shown - * by re-attaching them to the DOM.This is only generic way to generically + * Manager for HTML XBlocks. These blocks are hidden by detaching and shown + * by re-attaching them to the DOM. This is only way to generically * handle things like video players (they should stop playing when removed from DOM). * * @param html an html xblock @@ -20,31 +20,29 @@ /** * - * Manager for HTML Video child. Videos are paused when hiding them, and re-sized when showing them. + * Manager for HTML Video child. Videos are re-sized when showing them. * @param video an video xblock * */ function VideoManager(video) { this.show = function () { - if (typeof video.resizer === 'undefined'){ + if (typeof video.resizer === 'undefined') { // This one is tricky: but it looks like resizer is undefined only if the video is on the // step that is initially visible (and then no resizing is necessary) return; } video.resizer.align(); }; - this.hide = function () { - if (typeof video.videoPlayer === 'undefined'){ - // If video player is undefined this means that the video isn't loaded yet, so no need to - // pause it. - return; - } - video.videoPlayer.pause(); - }; + /** + * Videos should be paused when user leaves a step containing a video. There is was a proposed implementation + * but since it didn't work on every system we decided to drop it (it was out of scope for current task + * nevertheless). See OC-1441 for details. + */ + this.hide = function () {}; } /** - * Manager for Plot Xblocks. Handles updating a plot before displaying it. + * Manager for Plot Xblocks. Handles updating a plot before displaying it. * @param plot */ function PlotManager(plot) { @@ -65,48 +63,48 @@ var children = runtime.children(xblock_element); /** - * A list of managers for children than need special care when showing or hiding. + * A list of managers for children that need special care when showing or hiding. * * @type {show, hide}[] */ var managedChildren = []; /*** - * This is a workaround for issue where jquery.xblock.Runtime doesn't return HTML Xblocks when querying + * This is a workaround for issue where jquery.xblock.Runtime doesn't return HTML blocks when querying * for children. * * This can be removed when: * - * * We allow inclusion of Ooyala blocks inside ProblemBuilder and our clients migrate to Ooyala, in this case + * * We allow inclusion of Ooyala blocks inside StepBuilder and our clients migrate to Ooyala, in this case * we may drop special handling of HTML blocks. See discussions in OC-1441. * * We include HTML blocks in runtime.children for runtime of jquery.xblock, then just add * `html: HtmlManager` to `Managers`, and remove this block. */ - $("div.xblock.xblock-student_view.xmodule_HtmlModule", xblock_element).each(function (idx, element) { - managedChildren.push(new HtmlManager({element:element})); + $("div.xblock.xblock-student_view.xmodule_HtmlModule", xblock_element).each(function(idx, element) { + managedChildren.push(new HtmlManager({ element: element })); }); - for (var idx = 0; idx < children.length; idx++){ + for (var idx = 0; idx < children.length; idx++) { var child = children[idx]; - // NOTE: While following assertion is true for e.g Video blocks: - // child.type == $(child.element).data('block-type') it is invalidated by for all sb-* blocks + // NOTE: While the following assertion is true for e.g Video blocks: + // child.type == $(child.element).data('block-type') it is invalid for all sb-* blocks var type = $(child.element).data('block-type'); var constructor = Managers[type]; - if (typeof constructor === 'undefined'){ + if (typeof constructor === 'undefined') { // This block does not requires special care, moving on - continue ; + continue; } managedChildren.push(new constructor(child)); } this.show = function () { - for (var idx = 0; idx0; } diff --git a/problem_builder/public/js/step.js b/problem_builder/public/js/step.js index b0c96ec6..f21526be 100644 --- a/problem_builder/public/js/step.js +++ b/problem_builder/public/js/step.js @@ -105,12 +105,18 @@ function MentoringStepBlock(runtime, element) { return $('.sb-step', element).data('has-question'); }, - show: function () { + /** + * Shows a step, updating all children. + */ + showStep: function () { $(element).show(); childManager.show(); }, - hide: function () { + /** + * Hides a step, updating all children. + */ + hideStep: function () { $(element).hide(); childManager.hide(); } From 4b7b231301223e996ce09ddc0a0c164f61de5d46 Mon Sep 17 00:00:00 2001 From: Jacek Bzdak Date: Thu, 7 Apr 2016 22:14:31 +0200 Subject: [PATCH 3/3] Renamed lms_utils -> step_util --- problem_builder/mentoring.py | 2 +- problem_builder/public/js/step.js | 2 +- problem_builder/public/js/{lms_util.js => step_util.js} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename problem_builder/public/js/{lms_util.js => step_util.js} (99%) diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index d20848c6..d437cc29 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -1027,7 +1027,7 @@ def student_view(self, context): })) fragment.add_css_url(self.runtime.local_resource_url(self, 'public/css/problem-builder.css')) fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/vendor/underscore-min.js')) - fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/lms_util.js')) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/step_util.js')) fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/mentoring_with_steps.js')) fragment.add_resource(loader.load_unicode('templates/html/mentoring_attempts.html'), "text/html") diff --git a/problem_builder/public/js/step.js b/problem_builder/public/js/step.js index f21526be..a53ff032 100644 --- a/problem_builder/public/js/step.js +++ b/problem_builder/public/js/step.js @@ -5,7 +5,7 @@ function MentoringStepBlock(runtime, element) { var submitXHR, resultsXHR, message = $(element).find('.sb-step-message'); - var childManager = new ProblemBuilderUtilLMS.ChildManager(element, runtime); + var childManager = new ProblemBuilderStepUtil.ChildManager(element, runtime); function callIfExists(obj, fn) { if (typeof obj !== 'undefined' && typeof obj[fn] == 'function') { diff --git a/problem_builder/public/js/lms_util.js b/problem_builder/public/js/step_util.js similarity index 99% rename from problem_builder/public/js/lms_util.js rename to problem_builder/public/js/step_util.js index b2e8fec0..50c5f58e 100644 --- a/problem_builder/public/js/lms_util.js +++ b/problem_builder/public/js/step_util.js @@ -111,7 +111,7 @@ } - window.ProblemBuilderUtilLMS = { + window.ProblemBuilderStepUtil = { ChildManager: ChildManager