Merge pull request #1953 from hypothesis/add-highlight-test-and-change-scrolling

Add highlight test and change scrolling

h/static/scripts/annotator/plugin/bucket-bar.coffee

@@ -142,7 +142,7 @@ class Annotator.Plugin.BucketBar extends Annotator.Plugin
 
     # Get an anchor from the page we want to go to
     anchor = next[0]
-    anchor.scrollIntoView()
+    anchor.scrollToView()
 
   _update: =>
     wrapper = @annotator.wrapper

h/static/scripts/annotator/plugin/enhancedanchoring.coffee

@@ -60,10 +60,8 @@ class Anchor
     # If we are supposed to scroll to the highlight on a page,
     # and it's available now, go scroll there.
     if @pendingScrollTargetPage? and (hl = @highlight[@pendingScrollTargetPage])
-      hl.scrollIntoView().then =>
-        @pendingScrollResolve()
-        delete @pendingScrollTargetPage
-        delete @pendingScrollResolve
+      hl.scrollToView()
+      delete @pendingScrollTargetPage
 
   # Remove the highlights for the given set of pages
   virtualize: (pageIndex) =>
@@ -94,12 +92,12 @@ class Anchor
       delete @anchoring.anchors[index] unless anchors.length
 
   # Scroll to this anchor
-  scrollIntoView: ->
+  scrollToView: ->
     currentPage = @anchoring.document.getPageIndex()
 
     if @startPage is @endPage and currentPage is @startPage
       # It's all in one page. Simply scrolling
-      @highlight[@startPage].scrollIntoView()
+      @highlight[@startPage].scrollToView()
     else
       if currentPage < @startPage
         # We need to go forward
@@ -118,13 +116,12 @@ class Anchor
       # Is this rendered?
       if @anchoring.document.isPageMapped wantedPage
         # The wanted page is already rendered, we can simply go there
-        @highlight[wantedPage].scrollIntoView()
+        @highlight[wantedPage].scrollToView()
       else
         # Not rendered yet. Go to the page, we will continue from there
         @pendingScrollTargetPage = wantedPage
-        new Promise (resolve, reject) =>
-          @pendingScrollResolve = resolve
-          @anchoring.document.setPageIndex scrollPage
+        @anchoring.document.setPageIndex scrollPage
+        null
 
 Annotator.Anchor = Anchor
 

h/static/scripts/annotator/plugin/texthighlights.coffee

 $ = Annotator.$
 
+# Public: Wraps the DOM Nodes within the provided range with a highlight
+# element of the specified class and returns the highlight Elements.
+#
+# normedRange - A NormalizedRange to be highlighted.
+# cssClass - A CSS class to use for the highlight (default: 'annotator-hl')
+#
+# Returns an array of highlight Elements.
+highlightRange = (normedRange, cssClass='annotator-hl') ->
+  white = /^\s*$/
+
+  hl = $("<span class='#{cssClass}'></span>")
+
+  # Ignore text nodes that contain only whitespace characters. This prevents
+  # spans being injected between elements that can only contain a restricted
+  # subset of nodes such as table rows and lists. This does mean that there
+  # may be the odd abandoned whitespace node in a paragraph that is skipped
+  # but better than breaking table layouts.
+
+  nodes = $(normedRange.textNodes()).filter((i) -> not white.test @nodeValue)
+  r = nodes.wrap(hl).parent().show().toArray()
+  for node in nodes
+    event = document.createEvent "UIEvents"
+    event.initUIEvent "domChange", true, false, window, 0
+    event.reason = "created hilite"
+    node.dispatchEvent event
+  r
+
 class TextHighlight
 
+  @highlightRange: highlightRange
+
   @createFrom: (segment, anchor, page) ->
     return null if segment.type isnt "magic range"
 
@@ -44,7 +73,7 @@ class TextHighlight
     TextHighlight._init @annotator
 
     # Create highlights and link them with the annotation
-    @_highlights = highlightRange(normedRange)
+    @_highlights = TextHighlight.highlightRange(normedRange)
     $(@_highlights).data "annotation", @annotation
 
   # Is this a temporary hl?
@@ -86,38 +115,9 @@ class TextHighlight
   getHeight: -> $(@_highlights).outerHeight true
 
   # Scroll the highlight into view
-  scrollIntoView: ->
-    new Promise (resolve, reject) =>
-      $(@_highlights).scrollintoview complete: ->
-        resolve()
-
-# Public: Wraps the DOM Nodes within the provided range with a highlight
-# element of the specified class and returns the highlight Elements.
-#
-# normedRange - A NormalizedRange to be highlighted.
-# cssClass - A CSS class to use for the highlight (default: 'annotator-hl')
-#
-# Returns an array of highlight Elements.
-highlightRange = (normedRange, cssClass='annotator-hl') ->
-  white = /^\s*$/
-
-  hl = $("<span class='#{cssClass}'></span>")
-
-  # Ignore text nodes that contain only whitespace characters. This prevents
-  # spans being injected between elements that can only contain a restricted
-  # subset of nodes such as table rows and lists. This does mean that there
-  # may be the odd abandoned whitespace node in a paragraph that is skipped
-  # but better than breaking table layouts.
-
-  nodes = $(normedRange.textNodes()).filter((i) -> not white.test @nodeValue)
-  r = nodes.wrap(hl).parent().show().toArray()
-  for node in nodes
-    event = document.createEvent "UIEvents"
-    event.initUIEvent "domChange", true, false, window, 0
-    event.reason = "created hilite"
-    node.dispatchEvent event
-  r
-
+  scrollToView: ->
+    $(@_highlights).scrollintoview()
+    null
 
 class Annotator.Plugin.TextHighlights extends Annotator.Plugin
 

h/static/scripts/guest.coffee

@@ -166,7 +166,7 @@ class Annotator.Guest extends Annotator
     crossframe.on 'scrollToAnnotation', (ctx, tag) =>
       for a in @anchoring.getAnchors()
         if a.annotation.$$tag is tag
-          a.scrollIntoView()
+          a.scrollToView()
           return
     crossframe.on 'getDocumentInfo', (trans) =>
       (@plugins.PDF?.getMetaData() ? Promise.reject())

tests/js/guest-test.coffee

@@ -197,11 +197,11 @@ describe 'Annotator.Guest', ->
       it 'scrolls to the anchor with the matching tag', ->
         guest = createGuest()
         anchors = [
-          {annotation: {$$tag: 'tag1'}, scrollIntoView: sandbox.stub()}
+          {annotation: {$$tag: 'tag1'}, scrollToView: sandbox.stub()}
         ]
         sandbox.stub(guest.anchoring, 'getAnchors').returns(anchors)
         emitGuestEvent('scrollToAnnotation', 'ctx', 'tag1')
-        assert.called(anchors[0].scrollIntoView)
+        assert.called(anchors[0].scrollToView)
 
     describe 'on "getDocumentInfo" event', ->
       guest = null

tests/js/plugin/texthighlight-test.coffee

+assert = chai.assert
+sinon.assert.expose(assert, prefix: '')
+
+# In order to be able to create highlights,
+# the Annotator.TextHighlight class must exist.
+# This class is registered then the TextHighlights plugin
+# is initialized, so we will do that.
+th = new Annotator.Plugin.TextHighlights()
+th.pluginInit()
+
+describe 'Annotator.Plugin.TextHighlight', ->
+  sandbox = null
+  scrollTarget = null
+
+  createTestHighlight = ->
+    anchor =
+      id: "test anchor"
+      annotation: "test annotation"
+      anchoring:
+        id: "test anchoring manager"
+        annotator:
+          id: "test annotator"
+          element:
+            delegate: sinon.spy()
+
+    new Annotator.TextHighlight anchor, "test page", "test range"
+
+  beforeEach ->
+    sandbox = sinon.sandbox.create()
+    sandbox.stub Annotator.TextHighlight, 'highlightRange',
+      (normedRange, cssClass) ->
+        hl = document.createElement "hl"
+        hl.appendChild document.createTextNode "test highlight span"
+        hl
+
+    Annotator.$.fn.scrollintoview = sinon.spy (options) ->
+      scrollTarget = this[0]
+      options?.complete?()
+
+  afterEach ->
+    sandbox.restore()
+    scrollTarget = null
+
+  describe "constructor", ->
+    it 'wraps a highlight span around the given range', ->
+      hl = createTestHighlight()
+      assert.calledWith Annotator.TextHighlight.highlightRange, "test range"
+
+    it 'stores the created highlight spans in _highlights', ->
+      hl = createTestHighlight()
+      assert.equal hl._highlights.textContent, "test highlight span"
+
+    it "assigns the annotation as data to the highlight span", ->
+      hl = createTestHighlight()
+      annotation = $(hl._highlights).data "annotation"
+      assert.equal annotation, "test annotation"
+
+  describe "scrollToView", ->
+
+    it 'calls jQuery scrollintoview', ->
+      hl = createTestHighlight()
+      hl.scrollToView()
+      assert.called Annotator.$.fn.scrollintoview
+
+    it 'scrolls to the created highlight span', ->
+      hl = createTestHighlight()
+      hl.scrollToView()
+      assert.equal scrollTarget, hl._highlights