Merge branch 'master' into feature/abarrett/lms-notes-app

cms/djangoapps/contentstore/module_info_model.py

@@ -75,11 +75,7 @@ def set_module_info(store, location, post_data):
         # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it'
         for metadata_key, value in posted_metadata.items():
 
-            # let's strip out any metadata fields from the postback which have been identified as system metadata
-            # and therefore should not be user-editable, so we should accept them back from the client
-            if metadata_key in module.system_metadata_fields:
-                del posted_metadata[metadata_key]
-            elif posted_metadata[metadata_key] is None:
+            if posted_metadata[metadata_key] is None:
                 # remove both from passed in collection as well as the collection read in from the modulestore
                 if metadata_key in module._model_data:
                     del module._model_data[metadata_key]

cms/djangoapps/contentstore/views.py

@@ -677,11 +677,7 @@ def save_item(request):
         # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it'
         for metadata_key, value in posted_metadata.items():
 
-            # let's strip out any metadata fields from the postback which have been identified as system metadata
-            # and therefore should not be user-editable, so we should accept them back from the client
-            if metadata_key in existing_item.system_metadata_fields:
-                del posted_metadata[metadata_key]
-            elif posted_metadata[metadata_key] is None:
+            if posted_metadata[metadata_key] is None:
                 # remove both from passed in collection as well as the collection read in from the modulestore
                 if metadata_key in existing_item._model_data:
                     del existing_item._model_data[metadata_key]

cms/djangoapps/models/settings/course_metadata.py

@@ -14,7 +14,8 @@ class CourseMetadata(object):
     The objects have no predefined attrs but instead are obj encodings of the
     editable metadata.
     '''
-    FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start',
+    FILTERED_LIST = ['xml_attributes',
+                     'start',
                      'end',
                      'enrollment_start',
                      'enrollment_end',

cms/static/coffee/spec/views/module_edit_spec.coffee

@@ -72,3 +72,14 @@ describe "CMS.Views.ModuleEdit", ->
       it "loads the .xmodule-display inside the module editor", ->
         expect(XModule.loadModule).toHaveBeenCalled()
         expect(XModule.loadModule.mostRecentCall.args[0]).toBe($('.xmodule_display'))
+
+    describe "changedMetadata", ->
+      it "returns empty if no metadata loaded", ->
+        expect(@moduleEdit.changedMetadata()).toEqual({})
+
+      it "returns only changed values", ->
+        @moduleEdit.originalMetadata = {'foo', 'bar'}
+        spyOn(@moduleEdit, 'metadata').andReturn({'a': '', 'b': 'before', 'c': ''})
+        @moduleEdit.loadEdit()
+        @moduleEdit.metadata.andReturn({'a': '', 'b': 'after', 'd': 'only_after'})
+        expect(@moduleEdit.changedMetadata()).toEqual({'b' : 'after', 'd' : 'only_after'})

cms/static/coffee/src/views/module_edit.coffee

@@ -20,6 +20,7 @@ class CMS.Views.ModuleEdit extends Backbone.View
   loadEdit: ->
     if not @module
       @module = XModule.loadModule(@$el.find('.xmodule_edit'))
+      @originalMetadata = @metadata()
 
   metadata: ->
     # cdodge: package up metadata which is separated into a number of input fields
@@ -35,6 +36,14 @@ class CMS.Views.ModuleEdit extends Backbone.View
 
     return _metadata
 
+  changedMetadata: ->
+    currentMetadata = @metadata()
+    changedMetadata = {}
+    for key of currentMetadata
+      if currentMetadata[key] != @originalMetadata[key]
+        changedMetadata[key] = currentMetadata[key]
+    return changedMetadata
+
   cloneTemplate: (parent, template) ->
     $.post("/clone_item", {
       parent_location: parent
@@ -60,7 +69,7 @@ class CMS.Views.ModuleEdit extends Backbone.View
       course: course_location_analytics
       id: _this.model.id
 
-    data.metadata = _.extend(data.metadata || {}, @metadata())
+    data.metadata = _.extend(data.metadata || {}, @changedMetadata())
     @hideModal()
     @model.save(data).done( =>
     #   # showToastMessage("Your changes have been saved.", null, 3)

cms/templates/widgets/metadata-edit.html

 <%
   import hashlib
+  from xmodule.fields import StringyInteger, StringyFloat
   hlskey = hashlib.md5(module.location.url()).hexdigest()
 %>
 <section class="metadata_edit">
@@ -7,16 +8,39 @@
     % for field_name, field_value in editable_metadata_fields.items():
     <li>
       % if field_name == 'source_code':
+        % if field_value['is_default'] is False:
             <a href="#hls-modal-${hlskey}" style="color:yellow;" id="hls-trig-${hlskey}" >Edit High Level Source</a>
+        % endif
+      % else:
+        <label>${field_value['field'].display_name}:</label>
+ 	      <input type='text' data-metadata-name='${field_value["field"].display_name}'
+            ## This is a hack to keep current behavior for weight and attempts (empty will parse OK as unset).
+            ## This hack will go away with our custom editors.
+            % if field_value["value"] == None and (isinstance(field_value["field"], StringyFloat) or isinstance(field_value["field"], StringyInteger)):
+              value = ''
             % else:
-        <label>${field_name}:</label>
- 	      <input type='text' data-metadata-name='${field_name}' value='${field_value}' size='60' />
+              value='${field_value["field"].to_json(field_value["value"])}'
+            % endif
+            size='60' />
+        ## Change to True to see all the information being passed through.
+        % if False:
+            <label>Help: ${field_value['field'].help}</label>
+            <label>Type: ${type(field_value['field']).__name__}</label>
+            <label>Inherited: ${field_value['is_inherited']}</label>
+            <label>Default: ${field_value['is_default']}</label>
+            % if field_value['field'].values:
+              <label>Possible values:</label>
+              % for value in field_value['field'].values:
+                <label>${value}</label>
+              % endfor
+            % endif
+        % endif
       % endif
     </li>
     % endfor
   </ul>
 
-  % if 'source_code' in editable_metadata_fields:
+  % if 'source_code' in editable_metadata_fields and not editable_metadata_fields['source_code']['is_default']:
     <%include file="source-edit.html" />
   % endif
 

cms/templates/widgets/source-edit.html

@@ -12,7 +12,7 @@
 
     <form id="hls-form" enctype="multipart/form-data">
       <section class="source-edit">
-          <textarea name="" data-metadata-name="source_code" class="source-edit-box hls-data" rows="8" cols="40">${editable_metadata_fields['source_code']|h}</textarea>
+          <textarea name="" data-metadata-name="source_code" class="source-edit-box hls-data" rows="8" cols="40">${editable_metadata_fields['source_code']['value']|h}</textarea>
       </section>
       <div class="submit">
         <button type="reset" class="hls-compile">Save &amp; Compile to edX XML</button>

common/lib/xmodule/xmodule/capa_module.py

@@ -65,7 +65,8 @@ class CapaFields(object):
     max_attempts = StringyInteger(help="Maximum number of attempts that a student is allowed", scope=Scope.settings)
     due = Date(help="Date that this problem is due by", scope=Scope.settings)
     graceperiod = Timedelta(help="Amount of time after the due date that submissions will be accepted", scope=Scope.settings)
-    showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed")
+    showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed",
+                        values=["answered", "always", "attempted", "closed", "never"])
     force_save_button = Boolean(help="Whether to force the save button to appear on the page", scope=Scope.settings, default=False)
     rerandomize = Randomization(help="When to rerandomize the problem", default="always", scope=Scope.settings)
     data = String(help="XML data for the problem", scope=Scope.content)
@@ -882,16 +883,6 @@ class CapaDescriptor(CapaFields, RawDescriptor):
                          'enable_markdown': self.markdown is not None})
         return _context
 
-    @property
-    def editable_metadata_fields(self):
-        """Remove metadata from the editable fields since it has its own editor"""
-        subset = super(CapaDescriptor, self).editable_metadata_fields
-        if 'markdown' in subset:
-            del subset['markdown']
-        if 'empty' in subset:
-            del subset['empty']
-        return subset
-
     # VS[compat]
     # TODO (cpennington): Delete this method once all fall 2012 course are being
     # edited in the cms
@@ -901,3 +892,10 @@ class CapaDescriptor(CapaFields, RawDescriptor):
             'problems/' + path[8:],
             path[8:],
         ]
+
+    @property
+    def non_editable_metadata_fields(self):
+        non_editable_fields = super(CapaDescriptor, self).non_editable_metadata_fields
+        non_editable_fields.extend([CapaDescriptor.due, CapaDescriptor.graceperiod,
+                                    CapaDescriptor.force_save_button, CapaDescriptor.markdown])
+        return non_editable_fields

common/lib/xmodule/xmodule/discussion_module.py

@@ -37,3 +37,10 @@ class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawD
     metadata_translations = dict(RawDescriptor.metadata_translations)
     metadata_translations['id'] = 'discussion_id'
     metadata_translations['for'] = 'discussion_target'
+
+    @property
+    def non_editable_metadata_fields(self):
+        non_editable_fields = super(DiscussionDescriptor, self).non_editable_metadata_fields
+        # We may choose to enable sort_keys in the future, but while Kevin is investigating....
+        non_editable_fields.extend([DiscussionDescriptor.discussion_id, DiscussionDescriptor.sort_key])
+        return non_editable_fields

common/lib/xmodule/xmodule/html_module.py

@@ -19,6 +19,7 @@ log = logging.getLogger("mitx.courseware")
 
 class HtmlFields(object):
     data = String(help="Html contents to display for this module", scope=Scope.content)
+    source_code = String(help="Source code for LaTeX documents. This feature is not well-supported.", scope=Scope.settings)
 
 
 class HtmlModule(HtmlFields, XModule):
@@ -166,16 +167,6 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
         elt.set("filename", relname)
         return elt
 
-    @property
-    def editable_metadata_fields(self):
-        """Remove any metadata from the editable fields which have their own editor or shouldn't be edited by user."""
-        subset = super(HtmlDescriptor, self).editable_metadata_fields
-
-        if 'empty' in subset:
-            del subset['empty']
-
-        return subset
-
 
 class AboutDescriptor(HtmlDescriptor):
     """

common/lib/xmodule/xmodule/mako_module.py

 from .x_module import XModuleDescriptor, DescriptorSystem
-from .modulestore.inheritance import own_metadata
 
 
 class MakoDescriptorSystem(DescriptorSystem):
@@ -34,20 +33,10 @@ class MakoModuleDescriptor(XModuleDescriptor):
         """
         return {
             'module': self,
-            'editable_metadata_fields': self.editable_metadata_fields,
+            'editable_metadata_fields': self.editable_metadata_fields
         }
 
     def get_html(self):
         return self.system.render_template(
             self.mako_template, self.get_context())
 
-    # cdodge: encapsulate a means to expose "editable" metadata fields (i.e. not internal system metadata)
-    @property
-    def editable_metadata_fields(self):
-        fields = {}
-        for field, value in own_metadata(self).items():
-            if field in self.system_metadata_fields:
-                continue
-
-            fields[field] = value
-        return fields

common/lib/xmodule/xmodule/tests/test_xml_module.py

+from xmodule.x_module import XModuleFields
+from xblock.core import Scope, String, Object
+from xmodule.fields import Date, StringyInteger
+from xmodule.xml_module import XmlDescriptor
+import unittest
+from . import test_system
+from mock import Mock
+
+
+class TestFields(object):
+    # Will be returned by editable_metadata_fields.
+    max_attempts = StringyInteger(scope=Scope.settings)
+    # Will not be returned by editable_metadata_fields because filtered out by non_editable_metadata_fields.
+    due = Date(scope=Scope.settings)
+    # Will not be returned by editable_metadata_fields because is not Scope.settings.
+    student_answers = Object(scope=Scope.user_state)
+    # Will be returned, and can override the inherited value from XModule.
+    display_name = String(scope=Scope.settings)
+
+
+class EditableMetadataFieldsTest(unittest.TestCase):
+
+    def test_display_name_field(self):
+        editable_fields = self.get_xml_editable_fields({})
+        # Tests that the xblock fields (currently tags and name) get filtered out.
+        # Also tests that xml_attributes is filtered out of XmlDescriptor.
+        self.assertEqual(1, len(editable_fields), "Expected only 1 editable field for xml descriptor.")
+        self.assert_display_name_default(editable_fields)
+
+    def test_override_default(self):
+        # Tests that is_default is correct when a value overrides the default.
+        editable_fields = self.get_xml_editable_fields({'display_name': 'foo'})
+        display_name = editable_fields['display_name']
+        self.assertFalse(display_name['is_default'])
+        self.assertEqual('foo', display_name['value'])
+
+    def test_additional_field(self):
+        editable_fields = self.get_module_editable_fields({'max_attempts' : '7'})
+        self.assertEqual(2, len(editable_fields))
+        self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts, False, False, 7)
+        self.assert_display_name_default(editable_fields)
+
+        editable_fields = self.get_module_editable_fields({})
+        self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts, True, False, None)
+
+    def test_inherited_field(self):
+        editable_fields = self.get_module_editable_fields({'display_name' : 'inherited'})
+        self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name, False, True, 'inherited')
+
+    # Start of helper methods
+    def get_xml_editable_fields(self, model_data):
+        system = test_system()
+        system.render_template = Mock(return_value="<div>Test Template HTML</div>")
+        return XmlDescriptor(system=system, location=None, model_data=model_data).editable_metadata_fields
+
+    def get_module_editable_fields(self, model_data):
+        class TestModuleDescriptor(TestFields, XmlDescriptor):
+
+            @property
+            def non_editable_metadata_fields(self):
+                non_editable_fields = super(TestModuleDescriptor, self).non_editable_metadata_fields
+                non_editable_fields.append(TestModuleDescriptor.due)
+                return non_editable_fields
+
+        system = test_system()
+        system.render_template = Mock(return_value="<div>Test Template HTML</div>")
+        descriptor = TestModuleDescriptor(system=system, location=None, model_data=model_data)
+        descriptor._inherited_metadata = {'display_name' : 'inherited'}
+        return descriptor.editable_metadata_fields
+
+    def assert_display_name_default(self, editable_fields):
+        self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name, True, False, None)
+
+    def assert_field_values(self, editable_fields, name, field, is_default, is_inherited, value):
+        test_field = editable_fields[name]
+        self.assertEqual(field, test_field['field'])
+        self.assertEqual(is_default, test_field['is_default'])
+        self.assertEqual(is_inherited, test_field['is_inherited'])
+        self.assertEqual(value, test_field['value'])

common/lib/xmodule/xmodule/x_module.py

@@ -82,7 +82,7 @@ class XModuleFields(object):
     display_name = String(
         help="Display name for this module",
         scope=Scope.settings,
-        default=None,
+        default=None
     )
 
 
@@ -334,12 +334,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
     # (like a practice problem).
     has_score = False
 
-    # cdodge: this is a list of metadata names which are 'system' metadata
-    # and should not be edited by an end-user
-
-    system_metadata_fields = ['data_dir', 'published_date', 'published_by', 'is_draft',
-        'discussion_id', 'xml_attributes']
-
     # A list of descriptor attributes that must be equal for the descriptors to
     # be equal
     equality_attributes = ('_model_data', 'location')
@@ -612,6 +606,48 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
             model_data=self._model_data,
         ))
 
+    @property
+    def non_editable_metadata_fields(self):
+        """
+        Return the list of fields that should not be editable in Studio.
+
+        When overriding, be sure to append to the superclasses' list.
+        """
+        # We are not allowing editing of xblock tag and name fields at this time (for any component).
+        return [XBlock.tags, XBlock.name]
+
+    @property
+    def editable_metadata_fields(self):
+        """
+        Returns the metadata fields to be edited in Studio. These are fields with scope `Scope.settings`.
+
+        Can be limited by extending `non_editable_metadata_fields`.
+        """
+        inherited_metadata = getattr(self, '_inherited_metadata', {})
+        metadata = {}
+        for field in self.fields:
+
+            if field.scope != Scope.settings or field in self.non_editable_metadata_fields:
+                continue
+
+            inherited = False
+            default = False
+            value = getattr(self, field.name)
+            if field.name in self._model_data:
+                default = False
+                if field.name in inherited_metadata:
+                    if self._model_data.get(field.name) == inherited_metadata.get(field.name):
+                        inherited = True
+            else:
+                default = True
+
+            metadata[field.name] = {'field': field,
+                                    'value': value,
+                                    'is_inherited': inherited,
+                                    'is_default': default}
+
+        return metadata
+
 
 class DescriptorSystem(object):
     def __init__(self, load_item, resources_fs, error_tracker, **kwargs):

common/lib/xmodule/xmodule/xml_module.py

@@ -84,7 +84,8 @@ class XmlDescriptor(XModuleDescriptor):
     Mixin class for standardized parsing of from xml
     """
 
-    xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export", default={}, scope=Scope.settings)
+    xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export",
+        default={}, scope=Scope.settings)
 
     # Extension to append to filename paths
     filename_extension = 'xml'
@@ -418,3 +419,9 @@ class XmlDescriptor(XModuleDescriptor):
         """
         raise NotImplementedError(
             "%s does not implement definition_to_xml" % self.__class__.__name__)
+
+    @property
+    def non_editable_metadata_fields(self):
+        non_editable_fields = super(XmlDescriptor, self).non_editable_metadata_fields
+        non_editable_fields.append(XmlDescriptor.xml_attributes)
+        return non_editable_fields

lms/static/sass/shared/_modal.scss

@@ -149,7 +149,7 @@
       }
 
       label {
-        color: #999;
+        color: #646464;
 
         &.field-error {
           display: block;

lms/templates/courseware/courseware.html

@@ -156,7 +156,7 @@
         <div id="calculator_wrapper">
             <form id="calculator">
                 <div class="input-wrapper">
-                    <input type="text" id="calculator_input" />
+                    <input type="text" id="calculator_input" title="Calculator Input Field" />
 
                     <div class="help-wrapper">
                         <a href="#">Hints</a>
@@ -176,8 +176,8 @@
                         </dl>
                     </div>
                 </div>
-                <input id="calculator_button" type="submit" value="="/>
-                <input type="text" id="calculator_output"  readonly />
+                <input id="calculator_button" type="submit" title="Calculate" value="="/>
+                <input type="text" id="calculator_output" title="Calculator Output Field" readonly />
             </form>
 
         </div>

lms/templates/forgot_password_modal.html

@@ -12,19 +12,19 @@
       </div>
 
       <form id="pwd_reset_form" action="${reverse('password_reset')}" method="post" data-remote="true">
-        <label for="id_email">E-mail address:</label>
-        <input id="id_email" type="email" name="email" maxlength="75" placeholder="Your E-mail"/>
+        <label for="pwd_reset_email">E-mail address:</label>
+        <input id="pwd_reset_email" type="email" name="email" maxlength="75" placeholder="Your E-mail"/>
         <div class="submit">
           <input type="submit" id="pwd_reset_button" value="Reset my password" />
         </div>
       </form>
     </div>
 
-    <div class="close-modal">
+    <a href="#" class="close-modal" title="Close Modal">
       <div class="inner">
         <p>&#10005;</p>
       </div>
-    </div>
+    </a>
   </div>
 </section>
 
@@ -40,5 +40,10 @@
        $('#pwd_error').stop().css("display", "block");
      }
    });
+   
+   // removing close link's default behavior
+   $('#login-modal .close-modal').click(function(e) {
+    e.preventDefault();
+   });
   })(this)
 </script>

lms/templates/login_modal.html

@@ -9,14 +9,17 @@
     </header>
 
     <form id="login_form" class="login_form" method="post" data-remote="true" action="/login">
-      <label>E-mail</label>
-      <input name="email" type="email">
-      <label>Password</label>
-      <input name="password" type="password">
-      <label class="remember-me">
-        <input name="remember" type="checkbox" value="true">
+      <label for="login_email">E-mail</label>
+        <input id="login_email" type="email" name="email" placeholder="e.g. yourname@domain.com" />
+      
+      <label for="login_password">Password</label>
+        <input id="login_password" type="password" name="password" placeholder="&bull;&bull;&bull;&bull;&bull;&bull;&bull;&bull;" />  
+      
+      <label for="login_remember_me" class="remember-me">
+        <input id="login_remember_me" type="checkbox" name="remember" value="true" />
         Remember me
       </label>
+      
       <div class="submit">
         <input name="submit" type="submit" value="Access My Courses">
       </div>
@@ -34,11 +37,11 @@
 % endif
     </section>
 
-    <div class="close-modal">
+    <a href="#" class="close-modal" title="Close Modal">
       <div class="inner">
         <p>&#10005;</p>
       </div>
-    </div>
+    </a>
   </div>
 </section>
 
@@ -59,5 +62,10 @@
        $('#login_error').html(json.value).stop().css("display", "block");
      }
    });
+   
+   // removing close link's default behavior
+   $('#login-modal .close-modal').click(function(e) {
+    e.preventDefault();
+   });
 })(this)
 </script>

lms/templates/seq_module.html

@@ -10,7 +10,8 @@
         <li>
         <a class="seq_${item['type']} inactive progress-${item['progress_status']}"
            data-id="${item['id']}"
-           data-element="${idx+1}">
+           data-element="${idx+1}"
+           href="javascript:void(0);">
             <p>${item['title']}</p>
           </a>
         </li>

lms/templates/signup_modal.html

@@ -20,27 +20,31 @@
 
         <div class="input-group">
 	  % if has_extauth_info is UNDEFINED:
-          <label data-field="email">E-mail*</label>
-          <input name="email" type="email" placeholder="eg. yourname@domain.com">
-          <label data-field="password">Password*</label>
-          <input name="password" type="password" placeholder="****">
-          <label data-field="username">Public Username*</label>
-          <input name="username" type="text" placeholder="Shown on forums">
-          <label data-field="name">Full Name*</label>
-          <input name="name" type="text" placeholder="For your certificate">
+          <label data-field="email" for="signup_email">E-mail *</label>
+	    <input id="signup_email" type="email" name="email" placeholder="e.g. yourname@domain.com" required />
+	  
+          <label data-field="password" for="signup_password">Password *</label>
+	    <input id="signup_password" type="password" name="password" placeholder="&bull;&bull;&bull;&bull;&bull;&bull;&bull;&bull;" required />
+          
+	  <label data-field="username" for="signup_username">Public Username *</label>
+	    <input id="signup_username" type="text" name="username" placeholder="e.g. yourname (shown on forums)" required />
+	  
+          <label data-field="name" for="signup_fullname">Full Name *</label>
+	    <input id="signup_fullname" type="text" name="name" placeholder="e.g. Your Name (for certificates)" required />
 	  % else:
 	  <p><i>Welcome</i> ${extauth_email}</p><br/>
 	  <p><i>Enter a public username:</i></p>
-          <label data-field="username">Public Username*</label>
-          <input name="username" type="text" value="${extauth_username}" placeholder="Shown on forums">
+	  
+          <label data-field="username" for="signup_username">Public Username *</label>
+	    <input id="signup_username" type="text" name="username" value="${extauth_username}" placeholder="e.g. yourname (shown on forums)" required />
 	  % endif
         </div>
 
         <div class="input-group">
           <section class="citizenship">
-            <label data-field="level_of_education">Ed. completed</label>
+            <label data-field="level_of_education" for="signup_ed_level">Ed. Completed</label>
             <div class="input-wrapper">
-              <select name="level_of_education">
+              <select id="signup_ed_level" name="level_of_education">
                   <option value="">--</option>
                   %for code, ed_level in UserProfile.LEVEL_OF_EDUCATION_CHOICES:
                       <option value="${code}">${ed_level}</option>
@@ -50,9 +54,9 @@
           </section>
 
           <section class="gender">
-            <label data-field="gender">Gender</label>
+            <label data-field="gender" for="signup_gender">Gender</label>
             <div class="input-wrapper">
-              <select name="gender">
+              <select id="signup_gender" name="gender">
                   <option value="">--</option>
                   %for code, gender in UserProfile.GENDER_CHOICES:
                       <option value="${code}">${gender}</option>
@@ -62,9 +66,9 @@
           </section>
 
           <section class="date-of-birth">
-            <label data-field="date-of-birth">Year of birth</label>
+            <label data-field="date-of-birth" for="signup_birth_year">Year of birth</label>
             <div class="input-wrapper">
-              <select name="year_of_birth">
+              <select id="signup_birth_year" name="year_of_birth">
                 <option value="">--</option>
                 %for year in UserProfile.VALID_YEARS:
                   <option value="${year}">${year}</option>
@@ -74,22 +78,23 @@
             </div>
           </section>
 
-          <label data-field="mailing_address">Mailing address</label>
-          <textarea name="mailing_address"></textarea>
-          <label data-field="goals">Goals in signing up for edX</label>
-          <textarea name="goals"></textarea>
+          <label data-field="mailing_address" for="signup_mailing_address">Mailing address</label>
+	    <textarea id="signup_mailing_address" name="mailing_address"></textarea>
+	  
+          <label data-field="goals" for="signup_goals">Goals in signing up for edX</label>
+	    <textarea name="goals" id="signup_goals"></textarea>
 
         </div>
 
         <div class="input-group">
-          <label data-field="terms_of_service" class="terms-of-service">
-            <input name="terms_of_service" type="checkbox" value="true">
+          <label data-field="terms_of_service" class="terms-of-service" for="signup_tos">
+            <input id="signup_tos" name="terms_of_service" type="checkbox" value="true">
             I agree to the
             <a href="${reverse('tos')}" target="_blank">Terms of Service</a>*
           </label>
 
-          <label data-field="honor_code" class="honor-code">
-            <input name="honor_code" type="checkbox" value="true">
+          <label data-field="honor_code" class="honor-code" for="signup_honor">
+            <input id="signup_honor" name="honor_code" type="checkbox" value="true">
             I agree to the
             <a href="${reverse('honor')}" target="_blank">Honor Code</a>*
           </label>
@@ -110,11 +115,11 @@
 
     </div>
 
-    <div class="close-modal">
+    <a href="#" class="close-modal" title="Close Modal">
       <div class="inner">
         <p>&#10005;</p>
       </div>
-    </div>
+    </a>
   </div>
 </section>
 
@@ -129,5 +134,10 @@
        $("[data-field='"+json.field+"']").addClass('field-error')
      }
     });
+   
+   // removing close link's default behavior
+   $('#login-modal .close-modal').click(function(e) {
+    e.preventDefault();
+   });
   })(this)
 </script>