Addressing review notes

openedx · Aug 1, 2016 · f822741 · f822741
1 parent 560aae0
commit f822741
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 32 deletions.
diff --git a/drag_and_drop_v2/drag_and_drop_v2.py b/drag_and_drop_v2/drag_and_drop_v2.py
@@ -197,7 +197,7 @@ def items_without_answers():
 
         return {
             "mode": self.mode,
-            'max_attempts': self.max_attempts,
+            "max_attempts": self.max_attempts,
             "zones": self._get_zones(),
             # SDK doesn't supply url_name.
             "url_name": getattr(self, 'url_name', ''),

diff --git a/drag_and_drop_v2/public/css/drag_and_drop.css b/drag_and_drop_v2/public/css/drag_and_drop.css
@@ -376,6 +376,9 @@
 
 .xblock--drag-and-drop .actions-toolbar {
     margin: 5px 0;
+    position: relative;
+    min-width: 500px;
+    min-height: 2.5em;
 }
 
 .xblock--drag-and-drop .attempts-used {
@@ -385,8 +388,8 @@
 }
 
 .xblock--drag-and-drop .sidebar-buttons {
-    float: right;
-    display: flex;
+    position: absolute;
+    right: 0;
 }
 
 .xblock--drag-and-drop .sidebar-buttons .btn-brand {

diff --git a/drag_and_drop_v2/public/js/drag_and_drop.js b/drag_and_drop_v2/public/js/drag_and_drop.js
@@ -2,7 +2,7 @@ function DragAndDropTemplates(configuration) {
     "use strict";
     var h = virtualDom.h;
     // Set up a mock for gettext if it isn't available in the client runtime:
-    if (!window.gettext) { window.gettext = function gettext_stub(string) { return string; }; }
+    if (!window.gettext) {window.gettext = function gettext_stub(string) {return string;};}
 
     var itemSpinnerTemplate = function(xhr_active) {
         if (!xhr_active) {
@@ -104,11 +104,11 @@ function DragAndDropTemplates(configuration) {
         if (item.imageURL) {
             item_content_html = '<img src="' + item.imageURL + '" alt="' + item.imageDescription + '" />';
         }
-        var item_content = h('div', { innerHTML: item_content_html, className: "item-content" });
+        var item_content = h('div', {innerHTML: item_content_html, className: "item-content"});
         if (item.is_placed) {
             // Insert information about zone in which this item has been placed
             var item_description_id = configuration.url_name + '-item-' + item.value + '-description';
-            item_content.properties.attributes = { 'aria-describedby': item_description_id };
+            item_content.properties.attributes = {'aria-describedby': item_description_id};
             var zone_title = (zone.title || "Unknown Zone");  // This "Unknown" text should never be seen, so does not need i18n
             var description_content;
             if (configuration.mode === DragAndDropBlock.ASSESSMENT_MODE) {
@@ -121,7 +121,7 @@ function DragAndDropTemplates(configuration) {
             }
             var item_description = h(
                 'div',
-                { id: item_description_id, className: 'sr' },
+                {id: item_description_id, className: 'sr'},
                 description_content
             );
             children.splice(1, 0, item_description);
@@ -153,7 +153,7 @@ function DragAndDropTemplates(configuration) {
         var items_in_zone = [];
         if (zone.align !== 'none') {
             item_wrapper += '.item-align.item-align-' + zone.align;
-            var is_item_in_zone = function(i) { return i.is_placed && (i.zone === zone.uid); };
+            var is_item_in_zone = function(i) {return i.is_placed && (i.zone === zone.uid);};
             items_in_zone = $.grep(ctx.items, is_item_in_zone);
         }
 
@@ -177,8 +177,8 @@ function DragAndDropTemplates(configuration) {
                     }
                 },
                 [
-                    h('p', { className: className }, zone.title),
-                    h('p', { className: 'zone-description sr' }, zone.description),
+                    h('p', {className: className}, zone.title),
+                    h('p', {className: 'zone-description sr'}, zone.description),
                     h(item_wrapper, renderCollection(itemTemplate, items_in_zone, ctx))
                 ]
             )
@@ -187,11 +187,11 @@ function DragAndDropTemplates(configuration) {
 
     var feedbackTemplate = function(ctx) {
         var feedback_display = ctx.feedback_html ? 'block' : 'none';
-        var properties = { attributes: { 'aria-live': 'polite' } };
+        var properties = {attributes: {'aria-live': 'polite'}};
         return (
             h('section.feedback', properties, [
-                h('h3.title1', { style: { display: feedback_display } }, gettext('Feedback')),
-                h('p.message', { style: { display: feedback_display }, innerHTML: ctx.feedback_html })
+                h('h3.title1', {style: {display: feedback_display}}, gettext('Feedback')),
+                h('p.message', {style: {display: feedback_display}, innerHTML: ctx.feedback_html})
             ])
         );
     };
@@ -200,7 +200,7 @@ function DragAndDropTemplates(configuration) {
         return (
             h('div.keyboard-help-dialog', [
                 h('div.modal-window-overlay'),
-                h('div.modal-window', { attributes: { role: 'dialog', 'aria-labelledby': 'modal-window-title' }, style: {} }, [
+                h('div.modal-window', {attributes: {role: 'dialog', 'aria-labelledby': 'modal-window-title'}}, [
                     h('div.modal-header', [
                         h('h2.modal-window-title', gettext('Keyboard Help'))
                     ]),
@@ -222,15 +222,15 @@ function DragAndDropTemplates(configuration) {
     };
 
     var submitAnswerTemplate = function(ctx) {
-        var attemptsUsedDisplay = ctx.max_attempts > 0 ? 'inline': 'none';
-        var button_enabled = ctx.items.some(function(item) { return item.is_placed}) &&
-            (ctx.max_attempts == 0 || ctx.max_attempts > ctx.num_attempts);
+        var attemptsUsedDisplay = (ctx.max_attempts && ctx.max_attempts > 0) ? 'inline': 'none';
+        var button_enabled = ctx.items.some(function(item) {return item.is_placed;}) &&
+            (ctx.max_attempts === null || ctx.max_attempts > ctx.num_attempts);
 
         return (
           h("section.submit-answer", {}, [
-              h("button.btn-brand.submit-answer-button", { disabled: !button_enabled}, gettext("Submit")),
+              h("button.btn-brand.submit-answer-button", {disabled: !button_enabled}, gettext("Submit")),
               h(
-                  "span.attempts-used", {'style': {'display': attemptsUsedDisplay}},
+                  "span.attempts-used", {style: {display: attemptsUsedDisplay}},
                   gettext("You have used {used} of {total} attempts.")
                       .replace("{used}", ctx.num_attempts).replace("{total}", ctx.max_attempts)
               )
@@ -242,15 +242,15 @@ function DragAndDropTemplates(configuration) {
         return(
             h("section.sidebar-buttons", {}, [
                 h(
-                    'button.unbutton.btn-brand.btn-small.keyboard-help-button.link-button', { attributes: { tabindex: 0 } },
+                    'button.unbutton.btn-brand.btn-small.keyboard-help-button.link-button', {attributes: {tabindex: 0}},
                     [
                         h("i.btn-icon.fa.fa-question"),
                         gettext('Keyboard Help')
                     ]
                 ),
                 h(
-                    'button.unbutton.btn-brand..btn-small.reset-button.link-button',
-                    { disabled: ctx.disable_reset_button, attributes: { tabindex: 0 }, 'aria-live': 'off'},
+                    'button.unbutton.btn-brand.btn-small.reset-button.link-button',
+                    {disabled: ctx.disable_reset_button, attributes: {tabindex: 0}},
                     [
                         h("i.btn-icon.fa.fa-refresh"),
                         gettext('Reset')
@@ -269,10 +269,10 @@ function DragAndDropTemplates(configuration) {
         }
         // Render only items_in_bank and items_placed_unaligned here;
         // items placed in aligned zones will be rendered by zoneTemplate.
-        var is_item_placed = function(i) { return i.is_placed; };
+        var is_item_placed = function(i) {return i.is_placed;};
         var items_placed = $.grep(ctx.items, is_item_placed);
         var items_in_bank = $.grep(ctx.items, is_item_placed, true);
-        var is_item_placed_unaligned = function(i) { return i.zone_align === 'none'; };
+        var is_item_placed_unaligned = function(i) {return i.zone_align === 'none';};
         var items_placed_unaligned = $.grep(items_placed, is_item_placed_unaligned);
         return (
             h('section.themed-xblock.xblock--drag-and-drop', [
@@ -317,7 +317,6 @@ function DragAndDropTemplates(configuration) {
                     sidebarTemplate(ctx), // sidebar has float:right, so putting it before the content to float
                     (ctx.show_submit_answer ? submitAnswerTemplate(ctx) : null)
                 ]),
-                h("div", {style: {clear: 'both'}}), // div to clear floating
                 keyboardHelpPopupTemplate(ctx),
                 feedbackTemplate(ctx),
             ])
@@ -336,7 +335,7 @@ function DragAndDropBlock(runtime, element, configuration) {
     var renderView = DragAndDropTemplates(configuration);
 
     // Set up a mock for gettext if it isn't available in the client runtime:
-    if (!window.gettext) { window.gettext = function gettext_stub(string) { return string; }; }
+    if (!window.gettext) {window.gettext = function gettext_stub(string) {return string;};}
 
     var $element = $(element);
     element = $element[0]; // TODO: This line can be removed when we no longer support Dogwood.
@@ -488,7 +487,7 @@ function DragAndDropBlock(runtime, element, configuration) {
                 promise.reject();
             }
         }, false);
-        img.addEventListener("error", function() { promise.reject(); });
+        img.addEventListener("error", function() {promise.reject();});
         img.src = configuration.target_img_expanded_url;
         img.alt = configuration.target_img_description;
         return promise;
@@ -617,7 +616,7 @@ function DragAndDropBlock(runtime, element, configuration) {
             if (isFirstZone) {
                 evt.preventDefault();
                 $root.find('.target .zone').last().focus();
-            }
+           }
         } else {  // Going forward
             var isLastZone = $currentZone.next('.zone').length === 0;
             if (isLastZone) {
@@ -950,8 +949,8 @@ function DragAndDropBlock(runtime, element, configuration) {
                     if (configuration.items[i].id === +item_id) {
                         var size = configuration.items[i].size;
                         // size is an object like '{width: "50px", height: "auto"}'
-                        if (parseInt(size.width ) > 0) {  width = parseInt(size.width); }
-                        if (parseInt(size.height) > 0) { height = parseInt(size.height); }
+                        if (parseInt(size.width ) > 0) {width = parseInt(size.width);}
+                        if (parseInt(size.height) > 0) {height = parseInt(size.height);}
                         break;
                     }
                 }

diff --git a/tests/unit/data/old/config_out.json b/tests/unit/data/old/config_out.json
@@ -1,7 +1,7 @@
 {
     "title": "Drag and Drop",
     "mode": "standard",
-    "max_attempts": 0,
+    "max_attempts": null,
     "show_title": true,
     "problem_text": "",
     "show_problem_header": true,

diff --git a/tests/unit/test_basics.py b/tests/unit/test_basics.py
@@ -31,7 +31,7 @@ def test_get_configuration(self):
         items = config.pop("items")
         self.assertEqual(config, {
             "mode": DragAndDropBlock.STANDARD_MODE,
-            "max_attempts": 0,
+            "max_attempts": None,
             "display_zone_borders": False,
             "display_zone_labels": False,
             "title": "Drag and Drop",