Address review comments.

- Change a elements to buttons.
- Don't use placedholers.
- Make font sizes more consistent.
- Fix color contrasts.
- Use location.html_id() built-in function.

drag_and_drop_v2/drag_and_drop_v2.py

@@ -7,7 +7,6 @@
 import copy
 import json
 import urllib
-import uuid
 import webob
 
 from xblock.core import XBlock
@@ -95,7 +94,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
 
     question_text = String(
         display_name=_("Problem text"),
-        help=_("The description of the problem or instructions shown to the learner"),
+        help=_("The description of the problem or instructions shown to the learner."),
         scope=Scope.settings,
         default="",
     )
@@ -109,21 +108,21 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
 
     weight = Float(
         display_name=_("Maximum score"),
-        help=_("The maximum score the learner can receive for the problem"),
+        help=_("The maximum score the learner can receive for the problem."),
         scope=Scope.settings,
         default=1,
     )
 
     item_background_color = String(
         display_name=_("Item background color"),
-        help=_("The background color of draggable items in the problem."),
+        help=_("The background color of draggable items in the problem (e.g. 'blue' or '#0000ff')."),
         scope=Scope.settings,
         default="",
     )
 
     item_text_color = String(
         display_name=_("Item text color"),
-        help=_("Text color to use for draggable items."),
+        help=_("Text color to use for draggable items (e.g. 'white' or '#ffffff')."),
         scope=Scope.settings,
         default="",
     )
@@ -249,11 +248,12 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         """
 
         js_templates = loader.load_unicode('/templates/html/js_templates.html')
-        # A short random string to append to HTML element ID attributes to avoid multiple instances
-        # of the DnDv2 block on the same page sharing the same ID values.
+        # Get a 'html_id' string that is unique for this block.
+        # We append it to HTML element ID attributes to ensure multiple instances of the DnDv2 block
+        # on the same page don't share the same ID value.
         # We avoid using ID attributes in preference to classes, but sometimes we still need IDs to
         # connect 'for' and 'aria-describedby' attributes to the associated elements.
-        id_suffix = uuid.uuid4().hex[:16]
+        id_suffix = self.location.html_id()
         js_templates = js_templates.replace('{{id_suffix}}', id_suffix)
         context = {
             'js_templates': js_templates,

drag_and_drop_v2/public/css/drag_and_drop_edit.css

@@ -68,15 +68,19 @@
   clear: both;
 }
 
-.xblock--drag-and-drop--editor .tab h3,
-.xblock--drag-and-drop--editor .tab .h3 {
-    margin: 20px 0 8px 0;
+.xblock--drag-and-drop--editor .tab h3 {
+    font-size: 18px;
 }
 
-.xblock--drag-and-drop--editor .tab .h3 {
+.xblock--drag-and-drop--editor .tab h4,
+.xblock--drag-and-drop--editor .tab .h4 {
     display: block;
-    font: inherit;
-    font-size: 100%;
+    font-size: 16px;
+    margin: 20px 0 8px 0;
+}
+
+.xblock--drag-and-drop--editor .tab .items-form .item .row:first-of-type .h4 {
+    margin-top: 0;
 }
 
 .xblock--drag-and-drop--editor .tab-header,
@@ -98,20 +102,37 @@
     margin: 10px 0 0 0;
 }
 
-.xblock--drag-and-drop--editor .target-image-form input,
-.xblock--drag-and-drop--editor .target-image-form textarea {
+.xblock--drag-and-drop--editor .target-image-form input {
     width: 50%;
 }
+.xblock--drag-and-drop--editor .target-image-form textarea {
+    width: 97%;
+}
 
 .xblock--drag-and-drop--editor .target-image-form textarea {
     display: block;
 }
 
+.xblock--drag-and-drop--editor input,
+.xblock--drag-and-drop--editor textarea {
+    box-sizing: border-box;
+    font-size: 14px;
+    background: #fff;
+    box-shadow: none;
+    padding: 6px 8px;
+    border: 1px solid #b2b2b2;
+    color: #4c4c4c;
+}
+
 .xblock--drag-and-drop--editor label > span {
     display: inline-block;
     margin-bottom: 0.25em;
 }
 
+.xblock--drag-and-drop--editor label.checkbox-label {
+    font-size: 14px;
+}
+
 /* Main Tab */
 .xblock--drag-and-drop--editor .feedback-tab input,
 .xblock--drag-and-drop--editor .feedback-tab select {
@@ -151,29 +172,36 @@
   max-width: 100%;
 }
 
+.xblock--drag-and-drop--editor .zones-form .zone-row {
+    background-color: #b1d9f1;
+    padding: 1rem;
+    margin-bottom: 2rem;
+}
+
 .xblock--drag-and-drop--editor .zones-form .zone-row label {
     display: block;
 }
 
 .xblock--drag-and-drop--editor .zones-form .zone-row label > span {
     display: inline-block;
-    width: 6em;
+    font-size: 14px;
+    min-width: 8rem;
 }
 
 .xblock--drag-and-drop--editor .zones-form .zone-row label > input {
-    width: 60%;
+    width: 66%;
     margin: 0 0 5px;
     line-height: 2.664rem;  /* .title gets line-height from a Studio rule that does not apply to .description;
                                here we make sure that both input fields get the same value for line-height */
 }
 
-.xblock--drag-and-drop--editor .zones-form .zone-row .alignment {
-    margin-bottom: 15px;
+.xblock--drag-and-drop--editor .zones-form .zone-row .layout {
+    margin: 2rem 0;
 }
 
 .xblock--drag-and-drop--editor .zones-form .zone-row .layout label {
     display: inline-block;
-    width: 40%;
+    width: 45%;
 }
 
 .xblock--drag-and-drop--editor .zones-form .zone-row .layout .zone-size,
@@ -183,14 +211,19 @@
     line-height: inherit;
 }
 
+.xblock--drag-and-drop--editor .zones-form .zone-row .alignment {
+    margin-bottom: 15px;
+}
+
+
 .xblock--drag-and-drop--editor .feedback-form textarea {
     width: 99%;
     height: 128px;
 }
 
 .xblock--drag-and-drop--editor .form-help {
-    margin-top: 5px;
-    font-size: small;
+    margin: 0.5rem 0 1rem;
+    font-size: 12px;
 }
 
 .xblock--drag-and-drop--editor .item-styles-form,
@@ -199,14 +232,9 @@
 }
 
 .xblock--drag-and-drop--editor .items-form .item {
-    background-color: #8fcaec;
-    padding: 10px 0 1px;
-    margin: 15px 0;
-}
-
-.xblock--drag-and-drop--editor .items-form label,
-.xblock--drag-and-drop--editor .items-form .h3 {
-    margin: 0 1%;
+    background-color: #b1d9f1;
+    padding: 2rem;
+    margin-bottom: 2rem;
 }
 
 .xblock--drag-and-drop--editor .items-form select {
@@ -220,7 +248,6 @@
 .xblock--drag-and-drop--editor .items-form .item-text,
 .xblock--drag-and-drop--editor .items-form .item-image-url {
     width: 50%;
-    margin: 0 1%;
 }
 
 .xblock--drag-and-drop--editor .items-form .item-width {
@@ -235,16 +262,21 @@
     text-align: left;
 }
 
-.xblock--drag-and-drop--editor .items-form .row {
-    margin-bottom: 20px;
-}
 .xblock--drag-and-drop--editor .items-form .row.advanced {
     display: none;
 }
 .xblock--drag-and-drop--editor .items-form .row.advanced-link {
-    cursor: pointer;
-    padding-left: 1em;
-    font-size: 80%;
+    font-size: 12px;
+    margin-top: 2em;
+}
+.xblock--drag-and-drop--editor .items-form .row.advanced-link button {
+    background: none;
+    border: none;
+    color: #4c4c4c;
+}
+.xblock--drag-and-drop--editor .items-form .row.advanced-link button:before {
+    content: '\25B6';
+    margin-right: 0.5em;
 }
 .xblock--drag-and-drop-editor .items-form .zone-checkbox-row {
     margin-bottom: 0px;
@@ -269,22 +301,11 @@
     opacity: 0.5;
 }
 
-.xblock--drag-and-drop--editor .add-element {
-    text-decoration: none;
-    color: #1d5280;
-}
-
-.xblock--drag-and-drop--editor .remove-zone {
-    cursor: pointer;
-    float: right;
-    margin-top: 2px;
-    margin-right: 16px;
-}
-
+.xblock--drag-and-drop--editor .remove-zone,
 .xblock--drag-and-drop--editor .remove-item {
-    cursor: pointer;
     float: right;
-    margin-right: 16px;
+    padding: 3px;
+    border-radius: 12px;
 }
 
 .xblock--drag-and-drop--editor .icon {
@@ -292,9 +313,12 @@
     height: 14px;
     border-radius: 7px;
     background-color: #1d5280;
-    position: relative;
-    float: left;
-    margin: 0 5px 0 0;
+    display: inline-block;
+}
+
+.xblock--drag-and-drop--editor .remove-zone .icon,
+.xblock--drag-and-drop--editor .remove-item .icon {
+    display: block;
 }
 
 .xblock--drag-and-drop--editor .add-zone:hover,
@@ -364,12 +388,3 @@
     -ms-transform: rotate(45deg);
     transform: rotate(45deg);
 }
-
-.xblock--drag-and-drop--editor .remove-item .icon.remove {
-    background-color: #fff;
-    color: #0072a7;  /* Override default color from Studio to ensure contrast is large enough */
-}
-.xblock--drag-and-drop--editor .remove-item .icon.remove:before,
-.xblock--drag-and-drop--editor .remove-item .icon.remove:after {
-    background-color: #1d5280;
-}

drag_and_drop_v2/public/js/drag_and_drop_edit.js

@@ -222,7 +222,7 @@ function DragAndDropEditBlock(runtime, element, params) {
                             _fn.build.form.item.add();
                         })
                         .on('click', '.remove-item', _fn.build.form.item.remove)
-                        .on('click', '.advanced-link a', _fn.build.form.item.showAdvancedSettings)
+                        .on('click', '.advanced-link button', _fn.build.form.item.showAdvancedSettings)
                         .on('input', '.item-image-url', _fn.build.form.item.imageURLChanged);
                 },
                 form: {
@@ -430,7 +430,7 @@ function DragAndDropEditBlock(runtime, element, params) {
                             }
                             ctx.checkboxes = _fn.build.form.createCheckboxes(ctx.zones);
 
-                            _fn.build.form.item.count++;
+                            ctx.index = _fn.build.form.item.count++;
                             $form.append(tpl(ctx));
                             _fn.build.form.item.enableDelete();
 

drag_and_drop_v2/templates/html/drag_and_drop_edit.html

@@ -12,18 +12,18 @@
 
             <section class="tab-content">
                 <form class="feedback-form">
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans fields.display_name.display_name %}</span>
                         <input class="display-name" value="{{ self.display_name }}" />
                     </label>
-                    <label title="{% trans fields.show_title.help %}">
+                    <label class="checkbox-label" title="{% trans fields.show_title.help %}">
                       <input class="show-title" type="checkbox"
                         {% if self.show_title %}checked="checked"{% endif %}>
                       {% trans fields.show_title.display_name %}
                       <span class="sr">{% trans fields.show_title.help %}</span>
                     </label>
 
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans fields.mode.display_name %}</span>
                         <select class="problem-mode" aria-describedby="problem-mode-description-{{id_suffix}}">
                             {% for field_value in fields.mode.values %}
@@ -37,8 +37,8 @@
                       {% trans fields.mode.help %}
                     </div>
 
-                    <label class="h3 assessment-setting">
-                      {% trans fields.max_attempts.display_name %}
+                    <label class="h4 assessment-setting">
+                      <span>{% trans fields.max_attempts.display_name %}</span>
                       <input class="max-attempts"
                              type="number"
                              min="1"
@@ -50,16 +50,16 @@
                       {% trans fields.max_attempts.help %}
                     </div>
 
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans fields.weight.display_name %}</span>
                         <input class="weight" type="number" step="0.1" value="{{ self.weight|unlocalize }}" />
                     </label>
 
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans fields.question_text.display_name %}</span>
                         <textarea class="problem-text">{{ self.question_text }}</textarea>
                     </label>
-                    <label>
+                    <label class="checkbox-label">
                       <input class="show-problem-header"
                              type="checkbox"
                              aria-describedby="show-problem-header-description-{{id_suffix}}"
@@ -70,12 +70,12 @@
                       {% trans fields.show_question_header.help %}
                     </div>
 
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans "Introductory Feedback" %}</span>
                         <textarea class="intro-feedback">{{ self.data.feedback.start }}</textarea>
                     </label>
 
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans "Final Feedback" %}</span>
                         <textarea class="final-feedback">{{ self.data.feedback.finish }}</textarea>
                     </label>
@@ -89,16 +89,19 @@
             </header>
             <section class="tab-content">
                 <form class="target-image-form">
-                    <label class="h3" for="background-url-{{id_suffix}}">
+                    <label class="h4" for="background-url-{{id_suffix}}">
                         <span>{% trans "Background URL" %}</span>
                     </label>
                     <input class="background-url"
                            id="background-url-{{id_suffix}}"
                            type="text"
-                           placeholder="{% trans 'For example, http://example.com/background.png or /static/background.png' %}">
+                           aria-describedby="background-url-{{id_suffix}}" />
                     </label>
                     <button class="btn">{% trans "Change background" %}</button>
-                    <label class="h3">
+                    <div id="background-url-description-{{id_suffix}}" class="form-help">
+                        {% trans "For example, 'http://example.com/background.png' or '/static/background.png'." %}
+                    </div>
+                    <label class="h4">
                         <span>{% trans "Background description" %}</span>
                         <textarea required class="background-description"
                                   aria-describedby="background-description-description-{{id_suffix}}"></textarea>
@@ -114,25 +117,29 @@
             </section>
             <section class="tab-content">
                 <form class="display-labels-form">
-                    <h3>{% trans "Zone labels" %}</h3>
-                    <label>
-                        <span>{% trans "Display label names on the image" %}:</span>
+                    <h4>{% trans "Zone labels" %}</h4>
+                    <label class="checkbox-label">
                         <input name="display-labels" class="display-labels" type="checkbox" />
+                        <span>{% trans "Display label names on the image" %}</span>
                     </label>
                 </form>
                 <form class="display-borders-form">
-                    <h3>{% trans "Zone borders" %}</h3>
-                    <label>
-                        <span>{% trans "Display zone borders on the image" %}:</span>
+                    <h4>{% trans "Zone borders" %}</h4>
+                    <label class="checkbox-label">
                         <input name="display-borders" class="display-borders" type="checkbox" />
+                        <span>{% trans "Display zone borders on the image" %}</span>
                     </label>
                 </form>
             </section>
             <section class="tab-content">
+                <h4>{% trans "Zone definitions" %}</h4>
                 <div class="zone-editor">
                     <div class="controls">
                         <form class="zones-form"></form>
-                        <a href="#" class="add-zone add-element"><div class="icon add"></div>{% trans "Add a zone" %}</a>
+                        <button class="btn add-zone add-element">
+                            <span class="icon add"></span>
+                            {% trans "Add a zone" %}
+                        </button>
                     </div>
                     <div class="target">
                         <img class="target-img">
@@ -149,20 +156,18 @@
             </header>
             <section class="tab-content">
                 <form class="item-styles-form">
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans fields.item_background_color.display_name %}</span>
                         <input class="item-background-color"
-                               placeholder="{% blocktrans with example1='blue' example2='#0000ff' %}e.g. {{example1}} or {{example2}}{% endblocktrans %}"
                                value="{{ self.item_background_color }}"
                                aria-describedby="item-background-color-description-{{id_suffix}}">
                     </label>
                     <div id="item-background-color-description-{{id_suffix}}" class="form-help">
                       {% trans fields.item_background_color.help %}
                     </div>
-                    <label class="h3">
+                    <label class="h4">
                         <span>{% trans fields.item_text_color.display_name %}</span>
                         <input class="item-text-color"
-                               placeholder="{% blocktrans with example1='white' example2='#ffffff' %}e.g. {{example1}} or {{example2}}{% endblocktrans %}"
                                value="{{ self.item_text_color}}"
                                aria-describedby="item-text-color-description-{{id_suffix}}">
                     </label>
@@ -172,11 +177,13 @@
                 </form>
             </section>
             <section class="tab-content">
+                <h4>{% trans "Item definitions" %}</h4>
                 <form class="items-form"></form>
+                <button class="btn add-item add-element">
+                    <span class="icon add"></span>
+                    {% trans "Add an item" %}
+                </button>
             </section>
-            <footer class="tab-footer">
-                <a href="#" class="add-item add-element"><div class="icon add"></div>{% trans "Add an item" %}</a>
-            </footer>
         </div>
 
     </section>

drag_and_drop_v2/templates/html/js_templates.html

@@ -10,10 +10,10 @@
 </script>
 
 <script class="zone-input-tpl" type="text/html">
-    <div class="zone-row" data-uid="{{zone.uid}}">
-        <a class="remove-zone hidden" title="{{i18n 'Remove zone'}}">
-            <div class="icon remove"></div>
-        </a>
+    <fieldset class="zone-row" data-uid="{{zone.uid}}">
+        <button type="button" class="btn remove-zone hidden" title="{{i18n 'Remove zone'}}">
+            <span class="icon remove"></span>
+        </button>
         <label>
             <span>{{i18n "Title"}}</span>
             <input type="text"
@@ -26,8 +26,11 @@
             <input type="text"
                    class="zone-description"
                    value="{{ zone.description }}"
-                   placeholder="{{i18n 'Describe this zone to non-visual users'}}"
+                   aria-describedby="zone-description-description-{{zone.uid}}-{{id_suffix}}"
                    required />
+            <div id="zone-description-description-{{zone.uid}}-{{id_suffix}}" class="form-help">
+                {{i18n "Describe this zone to non-visual users."}}
+            </div>
         </label>
         <div class="layout">
             <label>
@@ -83,12 +86,12 @@
                 {{i18n "Align dropped items to the left, center, or right.  Default is no alignment (items stay exactly where the user drops them)."}}
             </div>
         </div>
-    </div>
+    </fieldset>
 </script>
 
 <script class="zone-checkbox-tpl" type="text/html">
     <div class="zone-checkbox-row">
-        <label>
+        <label class="checkbox-label">
             <input type="checkbox"
                    value="{{ zoneUid }}"
                    class="zone-checkbox"
@@ -99,69 +102,77 @@
 </script>
 
 <script class="item-input-tpl" type="text/html">
-    <div class="item">
+    <fieldset class="item">
+        <button type="button" class="btn remove-item hidden" title="{{i18n 'Remove item'}}">
+            <span class="icon remove"></span>
+        </button>
         <div class="row">
-            <label class="h3">
+            <label class="h4">
                 {{i18n "Text"}}
                 <input type="text"
-                       placeholder="{{i18n 'Use text that is clear and descriptive of the item to be placed'}}"
+                       aria-describedby="item-text-description-{{index}}-{{id_suffix}}"
                        class="item-text"
                        value="{{ displayName }}" />
             </label>
-            <a class="remove-item hidden">
-                <div class="icon remove"></div>
-            </a>
+            <div id="item-text-description-{{index}}-{{id_suffix}}" class="form-help">
+                {{i18n "Use text that is clear and descriptive of the item to be placed."}}
+            </div>
         </div>
         <div class="row">
             <fieldset>
-                <legend class="h3">
+                <legend class="h4">
                     {{ i18n "Zones" }}
                 </legend>
                 {{ checkboxes }}
             </fieldset>
         </div>
         <div class="row">
-            <label class="h3">
+            <label class="h4">
                 {{i18n "Image URL (alternative to the text)"}}
                 <input type="text"
-                       placeholder="{{i18n 'For example, http://example.com/image.png or /static/image.png'}}"
+                       aria-describedby="item-image-url-description-{{index}}-{{id_suffix}}"
                        class="item-image-url"
                        value="{{ imageURL }}" />
             </label>
+            <div id="item-image-url-description-{{index}}-{{id_suffix}}" class="form-help">
+                {{i18n "For example, 'http://example.com/image.png' or '/static/image.png'."}}
+            </div>
         </div>
         <div class="row">
-            <label class="h3">
+            <label class="h4">
                 <span>{{i18n "Image description (should provide sufficient information to place the item even if the image did not load)"}}</span>
                 <textarea {{#if imageURL}}required{{/if}} class="item-image-description">{{ imageDescription }}</textarea>
             </label>
         </div>
         <div class="row">
-            <label class="h3">
+            <label class="h4">
                 <span>{{i18n "Success Feedback"}}</span>
                 <textarea class="success-feedback">{{ feedback.correct }}</textarea>
             </label>
         </div>
         <div class="row">
-            <label class="h3">
+            <label class="h4">
                 <span>{{i18n "Error Feedback"}}</span>
                 <textarea class="error-feedback">{{ feedback.incorrect }}</textarea>
             </label>
         </div>
         <div class="row advanced-link">
-            <a>{{i18n "Show advanced settings" }}</a>
+            <button type="button">{{i18n "Show advanced settings" }}</button>
         </div>
         <div class="row advanced">
-            <label>
-                <span>
-                  {{i18n "Preferred width as a percentage of the background image width (or blank for automatic width):"}}
-                </span>
+            <label class="h4">
+                <span>{{i18n "Preferred width"}}</span>
                 <input type="number"
                        class="item-width"
                        value="{{ singleDecimalFloat widthPercent }}"
+                       aria-describedby="item-width-description-{{index}}-{{id_suffix}}"
                        step="0.1"
                        min="1"
                        max="99" />%
             </label>
+            <div id="item-width-description-{{index}}-{{id_suffix}}" class="form-help">
+                {{i18n "Specify preferred width as percentage of the background image width. Leave blank for automatic width."}}
             </div>
         </div>
+    </fieldset>
 </script>