From 45e4c0cfac600e5ad0c15f79ec8fb0e641659790 Mon Sep 17 00:00:00 2001
From: Arthur Barrett <abarrett@edx.org>
Date: Tue, 5 Feb 2013 12:37:58 -0500
Subject: [PATCH] Moved discussion thread html to its own template and only
 scroll when not in view.

---
 .../lib/xmodule/xmodule/annotatable_module.py | 14 ++++------
 .../xmodule/js/src/annotatable/display.coffee | 27 ++++++++++++++++---
 lms/templates/annotatable_discussion.html     |  6 +++++
 3 files changed, 35 insertions(+), 12 deletions(-)
 create mode 100644 lms/templates/annotatable_discussion.html

diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py
index 8d7dc5f917..5b26f4d953 100644
--- a/common/lib/xmodule/xmodule/annotatable_module.py
+++ b/common/lib/xmodule/xmodule/annotatable_module.py
@@ -63,15 +63,11 @@ class AnnotatableModule(XModule):
 
     def _get_discussion_html(self, discussion_id, discussion_title):
         """ Returns html to display the discussion thread """
-
-        tpl = u'<div class="annotatable-discussion" data-discussion-id="{0}">'
-        tpl += '<div class="annotatable-icon"> </div>'
-        tpl += '<span class="annotatable-discussion-label">Guided Discussion: </span>'
-        tpl += '<span class="annotatable-discussion-thread">{1}</span>'
-        tpl += '<a class="annotatable-show-discussion" href="javascript:void(0);">Show Discussion</a>'
-        tpl += '</div>'
-  
-        return tpl.format(discussion_id, discussion_title)
+        context = {
+                   'discussion_id': discussion_id,
+                   'discussion_title': discussion_title
+        }
+        return self.system.render_template('annotatable_discussion.html', context)
     
     def _attach_discussion(self, span, index, xmltree):
         """ Attaches a discussion thread to the annotation span. """
diff --git a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee
index 47ba52673f..52feb686ba 100644
--- a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee
+++ b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee
@@ -59,13 +59,34 @@ class @Annotatable
         $(@discussionSelector, @el).not(el).toggleClass('opaque', state)
 
     scrollToDiscussion: (el) ->
+        padding = 20
         complete = @makeHighlighter(el)
-        top = el.offset().top - 20 # with some padding
+        animOpts = {
+            scrollTop : el.offset().top - padding
+        }
+        
+        if @canScrollToDiscussion(el)
+            $('html, body').animate(animOpts, 500, 'swing', complete)
+        else
+            complete()
 
-        $('html, body').animate({ scrollTop: top }, 750, 'swing', complete)
+    canScrollToDiscussion: (el) ->
+        scrollTop = el.offset().top
+        docHeight = $(document).height()
+        winHeight = $(window).height()
+        winScrollTop = window.scrollY
+
+        viewStart = winScrollTop
+        viewEnd = winScrollTop + (.75 * winHeight)
+        inView = viewStart < scrollTop < viewEnd
+
+        scrollable = !inView
+        atDocEnd = viewStart + winHeight >= docHeight
+
+        return (if atDocEnd then false else scrollable)
     
     makeHighlighter: (el) ->
-        return @_once -> el.effect('highlight', {}, 750)
+        return @_once -> el.effect('highlight', {}, 500)
             
     _once: (fn) ->
         done = false
diff --git a/lms/templates/annotatable_discussion.html b/lms/templates/annotatable_discussion.html
new file mode 100644
index 0000000000..1525cc7b6b
--- /dev/null
+++ b/lms/templates/annotatable_discussion.html
@@ -0,0 +1,6 @@
+<div class="annotatable-discussion" data-discussion-id="${discussion_id}">
+    <div class="annotatable-icon"> </div>
+    <span class="annotatable-discussion-label">Guided Discussion: </span>
+    <span class="annotatable-discussion-thread">${discussion_title}</span>
+    <a class="annotatable-show-discussion" href="javascript:void(0);">Show Discussion</a>
+</div>