Merge pull request #1662 from MITx/feature/abarrett/annotation-styling-fixes

Annotation styling fixes

common/lib/xmodule/xmodule/annotatable_module.py

@@ -20,8 +20,7 @@ class AnnotatableModule(AnnotatableFields, XModule):
                      resource_string(__name__, 'js/src/collapsible.coffee'),
                      resource_string(__name__, 'js/src/html/display.coffee'),
                      resource_string(__name__, 'js/src/annotatable/display.coffee')],
-          'js': []
-         }
+          'js': []}
     js_module_name = "Annotatable"
     css = {'scss': [resource_string(__name__, 'css/annotatable/display.scss')]}
     icon_class = 'annotatable'
@@ -49,11 +48,11 @@ class AnnotatableModule(AnnotatableFields, XModule):
 
         if color is not None:
             if color in self.highlight_colors:
-                cls.append('highlight-'+color)
+                cls.append('highlight-' + color)
             attr['_delete'] = highlight_key
         attr['value'] = ' '.join(cls)
 
-        return { 'class' : attr }
+        return {'class': attr}
 
     def _get_annotation_data_attr(self, index, el):
         """ Returns a dict in which the keys are the HTML data attributes
@@ -73,7 +72,7 @@ class AnnotatableModule(AnnotatableFields, XModule):
             if xml_key in el.attrib:
                 value = el.get(xml_key, '')
                 html_key = attrs_map[xml_key]
-                data_attrs[html_key] = { 'value': value, '_delete': xml_key }
+                data_attrs[html_key] = {'value': value, '_delete': xml_key}
 
         return data_attrs
 
@@ -91,7 +90,6 @@ class AnnotatableModule(AnnotatableFields, XModule):
                 delete_key = attr[key]['_delete']
                 del el.attrib[delete_key]
 
-
     def _render_content(self):
         """ Renders annotatable content with annotation spans and returns HTML. """
         xmltree = etree.fromstring(self.content)
@@ -132,4 +130,3 @@ class AnnotatableDescriptor(AnnotatableFields, RawDescriptor):
     stores_state = True
     template_dir_name = "annotatable"
     mako_template = "widgets/raw-edit.html"
-

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

+/* TODO: move top-level variables to a common _variables.scss. 
+ * NOTE: These variables were only added here because when this was integrated with the CMS,
+ * SASS compilation errors were triggered because the CMS didn't have the same variables defined
+ * that the LMS did, so the quick fix was to localize the LMS variables not shared by the CMS.
+ * -Abarrett and Vshnayder
+ */
 $border-color: #C8C8C8;
 $body-font-size: em(14);
 
+.annotatable-wrapper {
+  position: relative;
+}
+
 .annotatable-header {
   margin-bottom: .5em;
   .annotatable-title {
@@ -55,6 +65,7 @@ $body-font-size: em(14);
   display: inline;
   cursor: pointer;
 
+  $highlight_index: 0;
   @each $highlight in (
     (yellow rgba(255,255,10,0.3) rgba(255,255,10,0.9)),
     (red rgba(178,19,16,0.3) rgba(178,19,16,0.9)),
@@ -63,11 +74,12 @@ $body-font-size: em(14);
     (blue rgba(35,163,255,0.3) rgba(35,163,255,0.9)),
     (purple rgba(115,9,178,0.3) rgba(115,9,178,0.9))) {
 
+    $highlight_index: $highlight_index + 1;
     $marker: nth($highlight,1);
     $color: nth($highlight,2);
     $selected_color: nth($highlight,3);
 
-    @if $marker == yellow {
+    @if $highlight_index == 1 {
       &.highlight {
         background-color: $color;
         &.selected { background-color: $selected_color; }
@@ -127,6 +139,7 @@ $body-font-size: em(14);
     font-weight: 400;
     padding: 0 10px 10px 10px;
     background-color: transparent;
+    border-color: transparent;
   } 
   p { 
       color: inherit; 
@@ -143,6 +156,7 @@ $body-font-size: em(14);
       margin: 0px 0px 10px 0;
       max-height: 225px;
       overflow: auto;
+      line-height: normal;
     }
     .annotatable-reply {
       display: block;
@@ -165,5 +179,3 @@ $body-font-size: em(14);
     border-top-color: rgba(0, 0, 0, .85);
   }
 }
-
-

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

@@ -2,6 +2,7 @@ class @Annotatable
     _debug: false
 
     # selectors for the annotatable xmodule
+    wrapperSelector:            '.annotatable-wrapper'
     toggleAnnotationsSelector:  '.annotatable-toggle-annotations'
     toggleInstructionsSelector: '.annotatable-toggle-instructions'
     instructionsSelector:       '.annotatable-instructions'
@@ -61,7 +62,7 @@ class @Annotatable
             my: 'bottom center' # of tooltip
             at: 'top center' # of target
             target: $(el) # where the tooltip was triggered (i.e. the annotation span)
-            container: @$el
+            container: @$(@wrapperSelector)
             adjust:
                 y: -5
         show:
@@ -75,6 +76,7 @@ class @Annotatable
             classes: 'ui-tooltip-annotatable'
         events:
             show: @onShowTip
+            move: @onMoveTip
 
     onClickToggleAnnotations: (e) => @toggleAnnotations()
 
@@ -87,6 +89,55 @@ class @Annotatable
     onShowTip: (event, api) =>
         event.preventDefault() if @annotationsHidden
 
+    onMoveTip: (event, api, position) =>
+        ###
+        This method handles an edge case in which a tooltip is displayed above
+        a non-overlapping span like this:
+
+                             (( TOOLTIP ))
+                                  \/
+        text text text ... text text text ...... <span span span>
+        <span span span>
+
+        The problem is that the tooltip looks disconnected from both spans, so
+        we should re-position the tooltip to appear above the span.
+        ###
+
+        tip = api.elements.tooltip
+        adjust_y = api.options.position?.adjust?.y || 0
+        container = api.options.position?.container || $('body')
+        target = api.elements.target
+
+        rects = $(target).get(0).getClientRects()
+        is_non_overlapping = (rects?.length == 2 and rects[0].left > rects[1].right)
+
+        if is_non_overlapping
+            # we want to choose the largest of the two non-overlapping spans and display
+            # the tooltip above the center of it (see api.options.position settings)
+            focus_rect = (if rects[0].width > rects[1].width then rects[0] else rects[1])
+            rect_center = focus_rect.left + (focus_rect.width / 2)
+            rect_top = focus_rect.top
+            tip_width = $(tip).width()
+            tip_height = $(tip).height()
+
+            # tooltip is positioned relative to its container, so we need to factor in offsets
+            container_offset = $(container).offset()
+            offset_left = -container_offset.left
+            offset_top = $(document).scrollTop() - container_offset.top
+
+            tip_left = offset_left + rect_center - (tip_width / 2)
+            tip_top =  offset_top + rect_top - tip_height + adjust_y
+
+            # make sure the new tip position doesn't clip the edges of the screen
+            win_width = $(window).width()
+            if tip_left < offset_left
+                tip_left = offset_left
+            else if tip_left + tip_width > win_width + offset_left
+                tip_left = win_width + offset_left - tip_width
+
+            # final step: update the position object (used by qtip2 to show the tip after the move event)
+            $.extend position, 'left': tip_left, 'top': tip_top
+
     getSpanForProblemReturn: (el) ->
         problem_id = $(@problemReturnSelector).index(el)
         @$(@spanSelector).filter("[data-problem-id='#{problem_id}']")