BLD-658: Add view for field type Dict in Studio.

CHANGELOG.rst

@@ -5,6 +5,11 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Blades: Fix for the list metadata editor that gets into a bad state where "Add"
+  is disabled. BLD-821.
+
+Blades: Add view for field type Dict in Studio. BLD-658.
+
 Blades: Refactor stub implementation of LTI Provider. BLD-601.
 
 LMS: In left accordion and progress page, due dates are now displayed in time

cms/static/coffee/spec/views/metadata_edit_spec.coffee

@@ -14,6 +14,7 @@ define ["js/models/metadata", "js/collections/metadata", "js/views/metadata", "c
       stringEntryTemplate = readFixtures('metadata-string-entry.underscore')
       optionEntryTemplate = readFixtures('metadata-option-entry.underscore')
       listEntryTemplate = readFixtures('metadata-list-entry.underscore')
+      dictEntryTemplate = readFixtures('metadata-dict-entry.underscore')
 
       beforeEach ->
           setFixtures($("<script>", {id: "metadata-editor-tpl", type: "text/template"}).text(editorTemplate))
@@ -21,6 +22,7 @@ define ["js/models/metadata", "js/collections/metadata", "js/views/metadata", "c
           appendSetFixtures($("<script>", {id: "metadata-string-entry", type: "text/template"}).text(stringEntryTemplate))
           appendSetFixtures($("<script>", {id: "metadata-option-entry", type: "text/template"}).text(optionEntryTemplate))
           appendSetFixtures($("<script>", {id: "metadata-list-entry", type: "text/template"}).text(listEntryTemplate))
+          appendSetFixtures($("<script>", {id: "metadata-dict-entry", type: "text/template"}).text(dictEntryTemplate))
 
       genericEntry = {
           default_value: 'default value',
@@ -92,6 +94,24 @@ define ["js/models/metadata", "js/collections/metadata", "js/views/metadata", "c
           value: "12:12:12"
       }
 
+      dictEntry = {
+          default_value: {
+            'en': 'English',
+            'ru': 'Русский'
+          },
+          display_name: "New Dict",
+          explicitly_set: false,
+          field_name: "dict",
+          help: "Specifies the name for this component.",
+          type: MetadataModel.DICT_TYPE,
+          value: {
+            'en': 'English',
+            'ru': 'Русский',
+            'ua': 'Українська',
+            'fr': 'Français'
+          }
+      }
+
 
       # Test for the editor that creates the individual views.
       describe "MetadataView.Editor creates editors for each field", ->
@@ -116,17 +136,18 @@ define ["js/models/metadata", "js/collections/metadata", "js/views/metadata", "c
                           value: null
                       },
                       listEntry,
-                      timeEntry
+                      timeEntry,
+                      dictEntry
                   ]
               )
 
           it "creates child views on initialize, and sorts them alphabetically", ->
               view = new MetadataView.Editor({collection: @model})
               childModels = view.collection.models
-              expect(childModels.length).toBe(7)
+              expect(childModels.length).toBe(8)
               # Be sure to check list view as well as other input types
               childViews = view.$el.find('.setting-input, .list-settings')
-              expect(childViews.length).toBe(7)
+              expect(childViews.length).toBe(8)
 
               verifyEntry = (index, display_name, type) ->
                   expect(childModels[index].get('display_name')).toBe(display_name)
@@ -135,10 +156,11 @@ define ["js/models/metadata", "js/collections/metadata", "js/views/metadata", "c
               verifyEntry(0, 'Display Name', 'text')
               verifyEntry(1, 'Inputs', 'number')
               verifyEntry(2, 'List', '')
-              verifyEntry(3, 'Show Answer', 'select-one')
-              verifyEntry(4, 'Time', 'text')
-              verifyEntry(5, 'Unknown', 'text')
-              verifyEntry(6, 'Weight', 'number')
+              verifyEntry(3, 'New Dict', '')
+              verifyEntry(4, 'Show Answer', 'select-one')
+              verifyEntry(5, 'Time', 'text')
+              verifyEntry(6, 'Unknown', 'text')
+              verifyEntry(7, 'Weight', 'number')
 
           it "returns its display name", ->
               view = new MetadataView.Editor({collection: @model})
@@ -351,7 +373,9 @@ define ["js/models/metadata", "js/collections/metadata", "js/views/metadata", "c
           assertCanUpdateView(@listView, ['a new item', 'another new item', 'a third'])
 
         it "has a clear method to revert to the model default", ->
+          @el.find('.create-setting').click()
           assertClear(@listView, ['a thing', 'another thing'])
+          expect(@el.find('.create-setting')).not.toHaveClass('is-disabled')
 
         it "has an update model method", ->
           assertUpdateModel(@listView, null, ['a new value'])
@@ -486,3 +510,99 @@ define ["js/models/metadata", "js/collections/metadata", "js/views/metadata", "c
 
           it "has an update model method", ->
               assertUpdateModel(@view, '12:12:12', '23:59:59')
+
+      describe "MetadataView.Dict allows the user to enter key-value pairs of strings", ->
+        beforeEach ->
+          dictModel = new MetadataModel($.extend(true, {}, dictEntry))
+          @dictView = new MetadataView.Dict({model: dictModel})
+          @el = @dictView.$el
+          main()
+
+        it "returns the initial value upon initialization", ->
+          assertValueInView(@dictView, {
+            'en': 'English',
+            'ru': 'Русский',
+            'ua': 'Українська',
+            'fr': 'Français'
+          })
+
+        it "updates its value correctly", ->
+          assertCanUpdateView(@dictView, {
+            'ru': 'Русский',
+            'ua': 'Українська',
+            'fr': 'Français'
+          })
+
+        it "has a clear method to revert to the model default", ->
+          @el.find('.create-setting').click()
+          assertClear(@dictView, {
+            'en': 'English',
+            'ru': 'Русский'
+          })
+          expect(@el.find('.create-setting')).not.toHaveClass('is-disabled')
+
+        it "has an update model method", ->
+          assertUpdateModel(@dictView, null, {'fr': 'Français'})
+
+        it "can add an entry", ->
+          expect(_.keys(@dictView.model.get('value')).length).toEqual(4)
+          @el.find('.create-setting').click()
+          expect(@el.find('input.input-key').length).toEqual(5)
+
+        it "can remove an entry", ->
+          expect(_.keys(@dictView.model.get('value')).length).toEqual(4)
+          @el.find('.remove-setting').first().click()
+          expect(_.keys(@dictView.model.get('value')).length).toEqual(3)
+
+        it "only allows one blank entry at a time", ->
+          expect(@el.find('input.input-key').length).toEqual(4)
+          @el.find('.create-setting').click()
+          @el.find('.create-setting').click()
+          expect(@el.find('input.input-key').length).toEqual(5)
+
+        it "only allows unique keys", ->
+          data = [
+            {
+              expectedValue: {'ru': 'Русский'},
+              initialValue: {'ru': 'Русский'},
+              testValue: {
+                'key': 'ru'
+                'value': ''
+              }
+            },
+            {
+              expectedValue: {'ru': 'Русский'},
+              initialValue: {'ru': 'Some value'},
+              testValue: {
+                'key': 'ru'
+                'value': 'Русский'
+              }
+            },
+            {
+              expectedValue: {'ru': 'Русский'},
+              initialValue: {'ru': 'Русский'},
+              testValue: {
+                'key': ''
+                'value': ''
+              }
+            }
+          ]
+
+          _.each data, ((d, index) ->
+            @dictView.setValueInEditor(d.initialValue)
+            @dictView.updateModel();
+            @el.find('.create-setting').click()
+            item = @el.find('.list-settings-item').last()
+            item.find('.input-key').val(d.testValue.key);
+            item.find('.input-value').val(d.testValue.value);
+
+            expect(@dictView.getValueFromEditor()).toEqual(d.expectedValue)
+          ).bind(@)
+
+        it "re-enables the add setting button after entering a new value", ->
+          expect(@el.find('input.input-key').length).toEqual(4)
+          @el.find('.create-setting').click()
+          expect(@el.find('.create-setting')).toHaveClass('is-disabled')
+          @el.find('input.input-key').last().val('third setting')
+          @el.find('input.input-key').last().trigger('input')
+          expect(@el.find('.create-setting')).not.toHaveClass('is-disabled')

cms/static/js/models/metadata.js

@@ -107,6 +107,7 @@ define(["backbone"], function(Backbone) {
     Metadata.FLOAT_TYPE = "Float";
     Metadata.GENERIC_TYPE = "Generic";
     Metadata.LIST_TYPE = "List";
+    Metadata.DICT_TYPE = "Dict";
     Metadata.VIDEO_LIST_TYPE = "VideoList";
     Metadata.RELATIVE_TIME_TYPE = "RelativeTime";
 

cms/static/js/views/metadata.js

@@ -25,24 +25,21 @@ function(BaseView, _, MetadataModel, AbstractEditor, VideoList) {
                     var data = {
                             el: self.$el.find('.metadata_entry')[counter++],
                             model: model
-                    };
-                    if (model.getType() === MetadataModel.SELECT_TYPE) {
-                        new Metadata.Option(data);
-                    }
-                    else if (model.getType() === MetadataModel.INTEGER_TYPE ||
-                        model.getType() === MetadataModel.FLOAT_TYPE) {
-                        new Metadata.Number(data);
-                    }
-                    else if(model.getType() === MetadataModel.LIST_TYPE) {
-                        new Metadata.List(data);
-                    }
-                    else if(model.getType() === MetadataModel.VIDEO_LIST_TYPE) {
-                        new VideoList(data);
-                    }
-                    else if(model.getType() === MetadataModel.RELATIVE_TIME_TYPE) {
-                        new Metadata.RelativeTime(data);
+                        },
+                        conversions = {
+                            'Select': 'Option',
+                            'Float': 'Number',
+                            'Integer': 'Number'
+                        },
+                        type = model.getType();
+
+                    if (conversions[type]) {
+                        type = conversions[type];
                     }
-                    else {
+
+                    if (_.isFunction(Metadata[type])) {
+                        new Metadata[type](data);
+                    } else {
                         // Everything else is treated as GENERIC_TYPE, which uses String editor.
                         new Metadata.String(data);
                     }
@@ -84,6 +81,8 @@ function(BaseView, _, MetadataModel, AbstractEditor, VideoList) {
         }
     });
 
+    Metadata.VideoList = VideoList;
+
     Metadata.String = AbstractEditor.extend({
 
         events : {
@@ -277,6 +276,7 @@ function(BaseView, _, MetadataModel, AbstractEditor, VideoList) {
 
         setValueInEditor: function (value) {
             var list = this.$el.find('ol');
+
             list.empty();
             _.each(value, function(ele, index) {
                 var template = _.template(
@@ -308,6 +308,13 @@ function(BaseView, _, MetadataModel, AbstractEditor, VideoList) {
 
         enableAdd: function() {
             this.$el.find('.create-setting').removeClass('is-disabled');
+        },
+
+        clear: function() {
+            AbstractEditor.prototype.clear.apply(this, arguments);
+            if (_.isNull(this.model.getValue())) {
+                this.$el.find('.create-setting').removeClass('is-disabled');
+            }
         }
     });
 
@@ -386,5 +393,89 @@ function(BaseView, _, MetadataModel, AbstractEditor, VideoList) {
         }
     });
 
+    Metadata.Dict = AbstractEditor.extend({
+
+        events : {
+            "click .setting-clear" : "clear",
+            "keypress .setting-input" : "showClearButton",
+            "change input" : "updateModel",
+            "input input" : "enableAdd",
+            "click .create-setting" : "addEntry",
+            "click .remove-setting" : "removeEntry"
+        },
+
+        templateName: "metadata-dict-entry",
+
+        getValueFromEditor: function () {
+            var dict = {};
+
+            _.each(this.$el.find('li'), function(li, index) {
+                var key = $(li).find('.input-key').val().trim(),
+                    value = $(li).find('.input-value').val().trim();
+
+                // Keys should be unique, so if our keys are duplicated and
+                // second key is empty or key and value are empty just do
+                // nothing. Otherwise, it'll be overwritten by the new value.
+                if (value === '') {
+                    if (key === '' || key in dict) {
+                        return false;
+                    }
+                }
+
+                dict[key] = value;
+            });
+
+            return dict;
+        },
+
+        setValueInEditor: function (value) {
+            var list = this.$el.find('ol'),
+                frag = document.createDocumentFragment();
+
+            _.each(value, function(value, key) {
+                var template = _.template(
+                    '<li class="list-settings-item">' +
+                        '<input type="text" class="input input-key" value="<%= key %>">' +
+                        '<input type="text" class="input input-value" value="<%= value %>">' +
+                        '<a href="#" class="remove-action remove-setting" data-value="<%= value %>"><i class="icon-remove-sign"></i><span class="sr">Remove</span></a>' +
+                    '</li>'
+                );
+
+                frag.appendChild($(template({'key': key, 'value': value}))[0]);
+            });
+
+            list.html([frag]);
+        },
+
+        addEntry: function(event) {
+            event.preventDefault();
+            // We don't call updateModel here since it's bound to the
+            // change event
+            var dict = $.extend(true, {}, this.model.get('value')) || {};
+            dict[''] = '';
+            this.setValueInEditor(dict);
+            this.$el.find('.create-setting').addClass('is-disabled');
+        },
+
+        removeEntry: function(event) {
+            event.preventDefault();
+            var entry = $(event.currentTarget).siblings('.input-key').val();
+            this.setValueInEditor(_.omit(this.model.get('value'), entry));
+            this.updateModel();
+            this.$el.find('.create-setting').removeClass('is-disabled');
+        },
+
+        enableAdd: function() {
+            this.$el.find('.create-setting').removeClass('is-disabled');
+        },
+
+        clear: function() {
+            AbstractEditor.prototype.clear.apply(this, arguments);
+            if (_.isNull(this.model.getValue())) {
+                this.$el.find('.create-setting').removeClass('is-disabled');
+            }
+        }
+    });
+
     return Metadata;
 });

cms/static/sass/views/_unit.scss

@@ -619,6 +619,7 @@ body.course.unit,.view-unit {
 
     //component-setting-entry
     .field.comp-setting-entry {
+      @include transition(opacity $tmg-f2 ease-in-out 0s);
       background-color: $white;
       padding: $baseline;
       border-bottom: 1px solid $gray-l2;
@@ -640,7 +641,6 @@ body.course.unit,.view-unit {
       }
 
       &:hover {
-        @include transition(opacity $tmg-f2 ease-in-out 0s);
         opacity: 1.0;
       }
 
@@ -654,9 +654,7 @@ body.course.unit,.view-unit {
       }
 
       .wrapper-comp-setting {
-        display: inline-block;
         min-width: 300px;
-        width: 55%;
         top: 0;
         vertical-align: top;
         margin-bottom:5px;
@@ -670,7 +668,7 @@ body.course.unit,.view-unit {
         display: inline-block;
         position: relative;
         left: 0;
-        width: 33%;
+        width: 25%;
         min-width: 100px;
         margin-right: ($baseline/2);
         font-weight: 600;
@@ -774,7 +772,6 @@ body.course.unit,.view-unit {
         display: inline-block;
         font-color: $gray-l6;
         min-width: ($baseline*10);
-        width: 35%;
         vertical-align: top;
       }
 
@@ -861,6 +858,77 @@ body.course.unit,.view-unit {
           }
         }
       }
+
+      // TYPE: Dict
+      .metadata-dict {
+        * {
+          @include box-sizing(border-box);
+        }
+
+        // label
+        .setting-label {
+            vertical-align: top;
+            margin-top: ($baseline*.75);
+        }
+
+        // inputs and labels
+        .wrapper-dict-settings {
+          width: 55%;
+          display: inline-block;
+          min-width: 240px;
+
+          // enumerated fields
+          .list-settings {
+            margin: ($baseline/2) 0 0;
+
+            .list-settings-item {
+              margin-bottom: ($baseline/2);
+            }
+
+            // inputs
+            .input {
+              width: 43%;
+              margin-right: ($baseline/4);
+              vertical-align: middle;
+              display: inline-block;
+
+              &.input-value {
+                margin-right: ($baseline/2);
+              }
+            }
+          }
+        }
+
+        .setting-clear {
+          vertical-align: top;
+          margin: ($baseline*.75) 0 0 0;
+        }
+
+        .create-setting {
+          @extend %ui-btn-flat-outline;
+          @extend %t-action3;
+          display: block;
+          width: 88%;
+          padding: ($baseline/2);
+          font-weight: 600;
+
+          *[class^="icon-"] {
+            margin-right: ($baseline/4);
+          }
+        }
+
+        .remove-setting {
+          @include transition(color 0.25s ease-in-out);
+          @include font-size(20);
+          display: inline-block;
+          background: transparent;
+          color: $blue-l3;
+
+          &:hover {
+            color: $blue;
+          }
+        }
+      }
     }
   }
 }

cms/templates/js/metadata-dict-entry.underscore

+<div class="wrapper-comp-setting metadata-dict">
+  <label class="label setting-label" for="<%= uniqueId %>"><%= model.get('display_name')%></label>
+  <div id="<%= uniqueId %>" class="wrapper-dict-settings">
+    <ol class="list-settings"></ol>
+    <a href="#" class="create-action create-setting">
+      <i class="icon-plus"></i><%= gettext("Add") %> <span class="sr"><%= model.get('display_name')%></span>
+    </a>
+  </div>
+  <button class="action setting-clear inactive" type="button" name="setting-clear" value="<%= gettext("Clear") %>" data-tooltip="<%= gettext("Clear") %>">
+    <i class="icon-undo"></i>
+    <span class="sr">"<%= gettext("Clear Value") %>"</span>
+  </button>
+</div>
+<span class="tip setting-help"><%= model.get('help') %></span>

cms/templates/widgets/metadata-edit.html

@@ -13,21 +13,11 @@
     <%static:include path="js/metadata-editor.underscore" />
 </script>
 
-<script id="metadata-number-entry" type="text/template">
-    <%static:include path="js/metadata-number-entry.underscore" />
-</script>
-
-<script id="metadata-string-entry" type="text/template">
-    <%static:include path="js/metadata-string-entry.underscore" />
-</script>
-
-<script id="metadata-option-entry" type="text/template">
-    <%static:include path="js/metadata-option-entry.underscore" />
-</script>
-
-<script id="metadata-list-entry" type="text/template">
-    <%static:include path="js/metadata-list-entry.underscore" />
-</script>
+% for template_name in ["metadata-number-entry", "metadata-string-entry", "metadata-option-entry", "metadata-list-entry", "metadata-dict-entry"]:
+    <script id="${template_name}" type="text/template">
+        <%static:include path="js/${template_name}.underscore" />
+    </script>
+% endfor
 
 <% showHighLevelSource='source_code' in editable_metadata_fields and editable_metadata_fields['source_code']['explicitly_set'] and enable_latex_compiler %>
 <% metadata_field_copy = copy.copy(editable_metadata_fields) %>

cms/templates/widgets/tabs/metadata-edit-tab.html

@@ -8,21 +8,11 @@
     <%static:include path="js/metadata-editor.underscore" />
 </script>
 
-<script id="metadata-number-entry" type="text/template">
-    <%static:include path="js/metadata-number-entry.underscore" />
-</script>
-
-<script id="metadata-string-entry" type="text/template">
-    <%static:include path="js/metadata-string-entry.underscore" />
-</script>
-
-<script id="metadata-option-entry" type="text/template">
-    <%static:include path="js/metadata-option-entry.underscore" />
-</script>
-
-<script id="metadata-list-entry" type="text/template">
-    <%static:include path="js/metadata-list-entry.underscore" />
-</script>
+% for template_name in ["metadata-number-entry", "metadata-string-entry", "metadata-option-entry", "metadata-list-entry", "metadata-dict-entry"]:
+    <script id="${template_name}" type="text/template">
+        <%static:include path="js/${template_name}.underscore" />
+    </script>
+% endfor
 
 <div class="wrapper-comp-settings metadata_edit" id="settings-tab" data-metadata='${json.dumps(editable_metadata_fields) | h}'/>
 

common/lib/xmodule/xmodule/x_module.py

@@ -16,7 +16,7 @@ from webob import Response
 from webob.multidict import MultiDict
 
 from xblock.core import XBlock
-from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String
+from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String, Dict
 from xblock.fragment import Fragment
 from xblock.plugin import default_select
 from xblock.runtime import Runtime
@@ -790,6 +790,8 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
                 editor_type = "Float"
             elif isinstance(field, List):
                 editor_type = "List"
+            elif isinstance(field, Dict):
+                editor_type = "Dict"
             elif isinstance(field, RelativeTime):
                 editor_type = "RelativeTime"
             metadata_fields[field.name]['type'] = editor_type