added a box around the instructions text

common/lib/xmodule/xmodule/annotatable_module.py

@@ -12,7 +12,6 @@ from xmodule.contentstore.content import StaticContent
 log = logging.getLogger(__name__)
 
 class AnnotatableModule(XModule):
-    # Note: js and css in common/lib/xmodule/xmodule
     js = {'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee'),
                      resource_string(__name__, 'js/src/collapsible.coffee'),
                      resource_string(__name__, 'js/src/html/display.coffee'),
@@ -77,22 +76,32 @@ class AnnotatableModule(XModule):
             attr = {}
             attr.update(self._get_annotation_class_attr(index, el))
             attr.update(self._get_annotation_data_attr(index, el))
+
             for key in attr.keys():
                 el.set(key, attr[key]['value'])
-                if '_delete' in attr[key]:
+                if '_delete' in attr[key] and attr[key]['_delete'] is not None:
                     delete_key = attr[key]['_delete']
                     del el.attrib[delete_key]
+
             index += 1
 
         return etree.tostring(xmltree, encoding='unicode')
 
+    def _extract_instructions(self, xmltree):
+        """ Removes <instructions> from the xmltree and returns them as a string, otherwise None. """
+        instructions = xmltree.find('instructions')
+        if instructions is not None:
+            instructions.tag = 'div'
+            xmltree.remove(instructions)
+            return etree.tostring(instructions, encoding='unicode')
+        return None
+
     def get_html(self):
         """ Renders parameters to template. """
         context = {
             'display_name': self.display_name,
             'element_id': self.element_id,
-            'discussion_id': self.discussion_id,
-            'instructions_html': self.instructions_html,
+            'instructions_html': self.instructions,
             'content_html': self._render_content()
         }
 
@@ -103,25 +112,11 @@ class AnnotatableModule(XModule):
         XModule.__init__(self, system, location, definition, descriptor,
                          instance_state, shared_state, **kwargs)
 
-        self.element_id = self.location.html_id()
-
         xmltree = etree.fromstring(self.definition['data'])
 
-        # extract discussion id
-        self.discussion_id = xmltree.get('discussion', '')
-        del xmltree.attrib['discussion']
-
-        # extract instructions text (if any)
-        instructions = xmltree.find('instructions')
-        instructions_html = None
-        if instructions is not None:
-            instructions.tag = 'div'
-            instructions_html = etree.tostring(instructions, encoding='unicode')
-            xmltree.remove(instructions)
-        self.instructions_html = instructions_html
-
-        # everything else is annotatable content
+        self.instructions = self._extract_instructions(xmltree)
         self.content = etree.tostring(xmltree, encoding='unicode')
+        self.element_id = self.location.html_id()
 
 class AnnotatableDescriptor(RawDescriptor):
     module_class = AnnotatableModule

common/lib/xmodule/xmodule/css/annotatable/display.scss

@@ -7,19 +7,29 @@
   }
 }
 
-.annotatable-description {
+.annotatable-section {
   position: relative;
-  padding: 2px 4px;
+  padding: .5em 1em;
   border: 1px solid $border-color;
-  border-radius: 3px;
+  border-radius: .5em;
   margin-bottom: .5em;
-  .annotatable-toggle {
-    position: absolute;
-    right: 0;
-    margin: 2px 7px 2px 0;
+
+  .annotatable-section-title {}
+  .annotatable-section-body {
+    border-top: 1px solid $border-color;
+    margin-top: .5em;
+    padding-top: .5em;
   }
 }
 
+.annotatable-toggle {
+  position: absolute;
+  right: 0;
+  margin: 2px 1em 2px 0;
+  &.expanded:after { content: " \2191" }
+  &.collapsed:after { content: " \2193" }
+}
+
 .annotatable-span {
   display: inline;
   cursor: pointer;

common/lib/xmodule/xmodule/js/src/annotatable/display.coffee

@@ -5,6 +5,7 @@ class @Annotatable
     toggleAnnotationsSelector:  '.annotatable-toggle-annotations'
     toggleInstructionsSelector: '.annotatable-toggle-instructions'
     instructionsSelector:       '.annotatable-instructions'
+    sectionSelector:            '.annotatable-section'
     spanSelector:               '.annotatable-span'
     replySelector:              '.annotatable-reply'
 
@@ -109,12 +110,13 @@ class @Annotatable
 
     toggleInstructions: () ->
       hide = (@instructionsHidden = not @instructionsHidden)
-      @toggleInstructionsButtonText hide
+      @toggleInstructionsButton hide
       @toggleInstructionsText hide
 
-    toggleInstructionsButtonText: (hide) ->
-        buttonText = (if hide then 'Show' else 'Hide')+' Instructions'
-        @$(@toggleInstructionsSelector).text(buttonText)
+    toggleInstructionsButton: (hide) ->
+        txt = (if hide then 'Expand' else 'Collapse')+' Instructions'
+        cls = (if hide then ['expanded', 'collapsed'] else ['collapsed','expanded'])
+        @$(@toggleInstructionsSelector).text(txt).removeClass(cls[0]).addClass(cls[1])
 
     toggleInstructionsText: (hide) ->
         @$(@instructionsSelector)[if hide then 'slideUp' else 'slideDown']()

common/lib/xmodule/xmodule/tests/test_annotatable_module.py

+"""Module annotatable tests"""
+
+import unittest
+from xmodule import annotatable
+
+class AnnotatableModuleTestCase(unittest.TestCase):
+

lms/templates/annotatable.html

-<div class="annotatable-wrapper" data-discussion-id="${discussion_id}">
+<div class="annotatable-wrapper">
 	<div class="annotatable-header">
 		% if display_name is not UNDEFINED and display_name is not None:
 		  <div class="annotatable-title">${display_name}</div>
@@ -6,16 +6,21 @@
     </div>
 
     % if instructions_html is not UNDEFINED and instructions_html is not None:
-    <div class="annotatable-description">
-        Instructions
-        <a class="annotatable-toggle annotatable-toggle-instructions" href="javascript:void(0)">Hide Instructions</a>
+    <div class="annotatable-section">
+        <div class="annotatable-section-title">
+            Instructions
+            <a class="annotatable-toggle annotatable-toggle-instructions expanded" href="javascript:void(0)">Collapse Instructions</a>
+        </div>
+        <div class="annotatable-section-body annotatable-instructions">
+            ${instructions_html}
+        </div>
     </div>
-    <div class="annotatable-instructions">${instructions_html}</div>
     % endif
 
-    <div class="annotatable-description">
+    <div class="annotatable-section">
         Guided Discussion
         <a class="annotatable-toggle annotatable-toggle-annotations" href="javascript:void(0)">Hide Annotations</a>
     </div>
+
 	<div class="annotatable-content">${content_html}</div>
 </div>