From 4d136f8d3bc1d247941b68f0af8b2caac4e947b0 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Thu, 7 Mar 2013 11:51:45 -0500 Subject: [PATCH 1/8] fixed annotation tooltip styling issue in studio --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index 308b379ec1..c462d4806e 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -127,6 +127,7 @@ $body-font-size: em(14); font-weight: 400; padding: 0 10px 10px 10px; background-color: transparent; + border-color: transparent; } p { color: inherit; From f5c3775b5dcbb8b16e6a0fcd27fd8b835516a56e Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Thu, 7 Mar 2013 12:21:47 -0500 Subject: [PATCH 2/8] fixed the annotation tooltip line height so it is the same in studio and the lms. --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index c462d4806e..b5739b28fc 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -144,6 +144,7 @@ $body-font-size: em(14); margin: 0px 0px 10px 0; max-height: 225px; overflow: auto; + line-height: normal; } .annotatable-reply { display: block; From 60b060263c15bb90fc658349224c50158609e31d Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Thu, 7 Mar 2013 16:02:22 -0500 Subject: [PATCH 3/8] refactor highlight css to prevent issues with cascade --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index b5739b28fc..f8ae779b8c 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -55,6 +55,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)), @@ -62,12 +63,13 @@ $body-font-size: em(14); (green rgba(25,255,132,0.3) rgba(25,255,132,0.9)), (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; } @@ -167,5 +169,3 @@ $body-font-size: em(14); border-top-color: rgba(0, 0, 0, .85); } } - - From 094458dd6f0e4437a71dcbcd990d31286725dc16 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Mon, 11 Mar 2013 16:19:36 -0400 Subject: [PATCH 4/8] Modified tooltip positioning on non-overlapping annotation spans. --- .../xmodule/js/src/annotatable/display.coffee | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee index 2ad49ae6d7..523b0e99cf 100644 --- a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee @@ -75,6 +75,7 @@ class @Annotatable classes: 'ui-tooltip-annotatable' events: show: @onShowTip + move: @onMoveTip onClickToggleAnnotations: (e) => @toggleAnnotations() @@ -87,6 +88,40 @@ 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 ...... + + + 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 + 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 + focus_rect = rects[0] + rect_center = focus_rect.left + (focus_rect.width / 2) + rect_top = focus_rect.top + tip_width = $(tip).width() + tip_height = $(tip).height() + tip_left = rect_center - (tip_width / 2) + tip_top = window.pageYOffset + rect_top - tip_height + adjust_y + win_width = $(window).width() + if tip_left + tip_width > win_width + tip_left = win_width - tip_width + position.left = tip_left + position.top = tip_top + getSpanForProblemReturn: (el) -> problem_id = $(@problemReturnSelector).index(el) @$(@spanSelector).filter("[data-problem-id='#{problem_id}']") From fcf82ba2bc44cb701c020d0494e2537139635f27 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Mon, 11 Mar 2013 18:02:22 -0400 Subject: [PATCH 5/8] fixed pep8 violations for annotation module --- common/lib/xmodule/xmodule/annotatable_module.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py index f093b76f52..1385296ddf 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -11,13 +11,13 @@ from xmodule.contentstore.content import StaticContent log = logging.getLogger(__name__) + class AnnotatableModule(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'), 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' @@ -34,11 +34,11 @@ class AnnotatableModule(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 @@ -58,7 +58,7 @@ class AnnotatableModule(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 @@ -76,7 +76,6 @@ class AnnotatableModule(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) @@ -123,9 +122,9 @@ class AnnotatableModule(XModule): self.element_id = self.location.html_id() self.highlight_colors = ['yellow', 'orange', 'purple', 'blue', 'green'] + class AnnotatableDescriptor(RawDescriptor): module_class = AnnotatableModule stores_state = True template_dir_name = "annotatable" mako_template = "widgets/raw-edit.html" - From d860b167d6838443d05b5b67805047a7e032f6a3 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Tue, 12 Mar 2013 14:09:56 -0400 Subject: [PATCH 6/8] fixed tooltip positioning for non-overlapping spans in studio --- .../xmodule/css/annotatable/display.scss | 4 +++ .../xmodule/js/src/annotatable/display.coffee | 34 ++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index f8ae779b8c..6e1a38ee31 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -1,6 +1,10 @@ $border-color: #C8C8C8; $body-font-size: em(14); +.annotatable-wrapper { + position: relative; +} + .annotatable-header { margin-bottom: .5em; .annotatable-title { diff --git a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee index 523b0e99cf..e38e48eeda 100644 --- a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee @@ -1,7 +1,8 @@ class @Annotatable _debug: false - # selectors for the annotatable xmodule + # 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: @@ -104,23 +105,38 @@ class @Annotatable 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 - focus_rect = rects[0] + # 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() - tip_left = rect_center - (tip_width / 2) - tip_top = window.pageYOffset + rect_top - tip_height + adjust_y + + # 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 = $('body').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 + tip_width > win_width - tip_left = win_width - tip_width - position.left = tip_left - position.top = tip_top + 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) From bf6ca1b0e759252795ca89ad905828d30ceada28 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Tue, 12 Mar 2013 17:32:00 -0400 Subject: [PATCH 7/8] use document to get scrollTop for firefox --- common/lib/xmodule/xmodule/js/src/annotatable/display.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee index e38e48eeda..8a32c8f51e 100644 --- a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee @@ -123,7 +123,7 @@ class @Annotatable # 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 = $('body').scrollTop() - container_offset.top + 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 From 7507f91d2274a64c2993717bcbecb078346461a3 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Mon, 18 Mar 2013 15:43:43 -0400 Subject: [PATCH 8/8] Added TODO and explanation for the top-level scss variables. --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index 6e1a38ee31..e2c095de2d 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -1,3 +1,9 @@ +/* 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);