Merge pull request #1868 from hypothesis/dedupe-highlights

Clean up unused and duplicate TextHighlight code.

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

@@ -84,7 +84,8 @@ class Annotator.Plugin.BucketBar extends Annotator.Plugin
               acc
           , {}
 
-          next.paddedScrollDownTo()
+          # Scroll down to the annotation from the top of the annotation's page
+          next.paddedScrollTo('down')
           delete @pendingScroll
 
     @annotator.subscribe "highlightRemoved", (highlight) =>
@@ -128,7 +129,7 @@ class Annotator.Plugin.BucketBar extends Annotator.Plugin
     dir = if direction is "up" then +1 else -1
     {next} = annotations.reduce (acc, ann) ->
       {start, next} = acc
-      anchor = ann.anchoring.anchors[0]
+      anchor = ann.anchors[0]
       if not next? or start.page*dir < anchor.startPage*dir
         # This one is obviously better
         start:

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

@@ -83,12 +83,6 @@ class Anchor
       # Kill the list if it's empty
       delete @anchoring.anchors[index] unless anchors.length
 
-  # This is called when the underlying Annotator has been udpated
-  annotationUpdated: ->
-    # Notify the highlights
-    for index in [@startPage .. @endPage]
-      @highlight[index]?.annotationUpdated()
-
 Annotator.Anchor = Anchor
 
 # This plugin contains the enhanced anchoring framework.
@@ -107,13 +101,6 @@ class Annotator.Plugin.EnhancedAnchoring extends Annotator.Plugin
 
     this
 
-  # Sets up handlers to anchor-related events
-  _setupAnchorEvents: ->
-    # When annotations are updated
-    @annotator.on 'annotationUpdated', (annotation) =>
-      # Notify the anchors
-      for anchor in annotation.anchors or []
-        anchor.annotationUpdated()
 
   # Initializes the components used for analyzing the document
   _chooseAccessPolicy: ->
@@ -161,7 +148,6 @@ class Annotator.Plugin.EnhancedAnchoring extends Annotator.Plugin
     @selectorCreators = []
     @strategies = []
     @_setupDocumentAccessStrategies()
-    this._setupAnchorEvents()
 
     self = this
     @annotator.anchoring = this

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

 $ = Annotator.$
 
-# Abstract highlight class
-class Highlight
-
-  constructor: (@anchor, @pageIndex) ->
-    @annotation = @anchor.annotation
-    @anchoring = @anchor.anchoring
-    @annotator = @anchoring.annotator
-
-  # Mark/unmark this hl as temporary (while creating an annotation)
-  setTemporary: (value) ->
-    throw "Operation not implemented."
-
-  # Is this a temporary hl?
-  isTemporary: ->
-    throw "Operation not implemented."
-
-  # TODO: review the usage of the batch parameters.
-
-  # Mark/unmark this hl as focused
-  #
-  # Value specifies whether it should be focused or not
-  #
-  # The 'batch' field specifies whether this call is only one of
-  # many subsequent calls, which should be executed together.
-  #
-  # In this case, a "finalizeHighlights" event will be published
-  # when all the flags have been set, and the changes should be
-  # executed.
-  setFocused: (value, batch = false) ->
-    throw "Operation not implemented."
-
-  # React to changes in the underlying annotation
-  annotationUpdated: ->
-    #console.log "In HL", this, "annotation has been updated."
-
-  # Remove all traces of this hl from the document
-  removeFromDocument: ->
-    throw "Operation not implemented."
-
-  # Get the HTML elements making up the highlight
-  # If you implement this, you get automatic implementation for the functions
-  # below. However, if you need a more sophisticated control mechanism,
-  # you are free to leave this unimplemented, and manually implement the
-  # rest.
-  _getDOMElements: ->
-    throw "Operation not implemented."
-
-  # Get the Y offset of the highlight. Override for more control
-  getTop: -> $(@_getDOMElements()).offset().top
-
-  # Get the height of the highlight. Override for more control
-  getHeight: -> $(@_getDOMElements()).outerHeight true
-
-  # Get the bottom Y offset of the highlight. Override for more control.
-  getBottom: -> @getTop() + @getBottom()
-
-  # Scroll the highlight into view. Override for more control
-  scrollTo: -> $(@_getDOMElements()).scrollintoview()
-
-  # Scroll the highlight into view, with a comfortable margin.
-  # up should be true if we need to scroll up; false otherwise
-  paddedScrollTo: (direction) ->
-    unless direction? then throw "Direction is required"
-    dir = if direction is "up" then -1 else +1
-    where = $(@_getDOMElements())
-    wrapper = @annotator.wrapper
-    defaultView = wrapper[0].ownerDocument.defaultView
-    pad = defaultView.innerHeight * .2
-    where.scrollintoview
-      complete: ->
-        scrollable = if this.parentNode is this.ownerDocument
-          $(this.ownerDocument.body)
-        else
-          $(this)
-        top = scrollable.scrollTop()
-        correction = pad * dir
-        scrollable.stop().animate {scrollTop: top + correction}, 300
-
-  # Scroll up to the highlight, with a comfortable margin.
-  paddedScrollUpTo: -> @paddedScrollTo "up"
-
-  # Scroll down to the highlight, with a comfortable margin.
-  paddedScrollDownTo: -> @paddedScrollTo "down"
-
-
-# This plugin containts the text highlight implementation,
-# required for annotating text.
-class TextHighlight extends Highlight
+class TextHighlight
 
   @createFrom: (segment, anchor, page) ->
     return null if segment.type isnt "magic range"
 
     new TextHighlight anchor, page, segment.data
 
-  # XXX: This is a temporay workaround until the Highlighter extension
-  # PR will be merged which will restore separation properly
-  @highlightClass = 'annotator-hl'
-
-  # Save the Annotator class reference, while we have access to it.
-  # TODO: Is this really the way to go? How do other plugins do it?
-  @Annotator = Annotator
-  @$ = Annotator.$
-  @highlightType = 'TextHighlight'
-
-  # Is this element a text highlight physical anchor ?
-  @isInstance: (element) -> @$(element).hasClass 'annotator-hl'
-
-  # Find the first parent outside this physical anchor
-  @getIndependentParent: (element) ->
-    @$(element).parents(':not([class^=annotator-hl])')[0]
-
   # List of annotators we have already set up events for
   @_inited: []
 
   # Collect the annotations impacted by an event
   @getAnnotations: (event) ->
-    TextHighlight.$(event.target)
+    $(event.target)
       .parents('.annotator-hl')
       .andSelf()
-      .map( -> TextHighlight.$(this).data("annotation"))
+      .map(-> $(this).data("annotation"))
       .toArray()
 
   # Set up events for this annotator
@@ -127,58 +23,29 @@ class TextHighlight extends Highlight
     return if annotator in @_inited
 
     annotator.element.delegate ".annotator-hl", "mouseover", this,
-       (event) => annotator.onAnchorMouseover event
+       (event) -> annotator.onAnchorMouseover event
 
     annotator.element.delegate ".annotator-hl", "mouseout", this,
-       (event) => annotator.onAnchorMouseout event
+       (event) -> annotator.onAnchorMouseout event
 
     annotator.element.delegate ".annotator-hl", "mousedown", this,
-       (event) => annotator.onAnchorMousedown event
+       (event) -> annotator.onAnchorMousedown event
 
     annotator.element.delegate ".annotator-hl", "click", this,
-       (event) => annotator.onAnchorClick event
+       (event) -> annotator.onAnchorClick event
 
     @_inited.push 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
-
-  constructor: (anchor, pageIndex, normedRange) ->
-    super anchor, pageIndex
-    TextHighlight._init @annotator
-
-    @$ = TextHighlight.$
-    @Annotator = TextHighlight.Annotator
+  constructor: (@anchor, @pageIndex, normedRange) ->
+    @annotation = @anchor.annotation
+    @anchoring = @anchor.anchoring
+    @annotator = @anchoring.annotator
 
-    # Create a highlights, and link them with the annotation
-    @_highlights = @_highlightRange normedRange
-    @$(@_highlights).data "annotation", @annotation
+    TextHighlight._init @annotator
 
-  # Implementing the required APIs
+    # Create highlights and link them with the annotation
+    @_highlights = highlightRange(normedRange)
+    $(@_highlights).data "annotation", @annotation
 
   # Is this a temporary hl?
   isTemporary: -> @_temporary
@@ -187,16 +54,16 @@ class TextHighlight extends Highlight
   setTemporary: (value) ->
     @_temporary = value
     if value
-      @$(@_highlights).addClass('annotator-hl-temporary')
+      $(@_highlights).addClass('annotator-hl-temporary')
     else
-      @$(@_highlights).removeClass('annotator-hl-temporary')
+      $(@_highlights).removeClass('annotator-hl-temporary')
 
   # Mark/unmark this hl as focused
   setFocused: (value) ->
     if value
-      @$(@_highlights).addClass('annotator-hl-focused')
+      $(@_highlights).addClass('annotator-hl-focused')
     else
-      @$(@_highlights).removeClass('annotator-hl-focused')
+      $(@_highlights).removeClass('annotator-hl-focused')
 
   # Remove all traces of this hl from the document
   removeFromDocument: ->
@@ -205,15 +72,68 @@ class TextHighlight extends Highlight
       if hl.parentNode? and @anchoring.document.isPageMapped @pageIndex
         # We should restore original state
         child = hl.childNodes[0]
-        @$(hl).replaceWith hl.childNodes
+        $(hl).replaceWith hl.childNodes
 
         event = document.createEvent "UIEvents"
         event.initUIEvent "domChange", true, false, window, 0
         event.reason = "removed hilite (annotation deleted)"
         child.parentNode.dispatchEvent event
 
-  # Get the HTML elements making up the highlight
-  _getDOMElements: -> @_highlights
+  # Get the Y offset of the highlight.
+  getTop: -> $(@_highlights).offset().top
+
+  # Get the height of the highlight.
+  getHeight: -> $(@_highlights).outerHeight true
+
+  # Scroll the highlight into view.
+  scrollTo: -> $(@_highlights).scrollintoview()
+
+  # Scroll the highlight into view, with a comfortable margin.
+  # up should be true if we need to scroll up; false otherwise
+  paddedScrollTo: (direction) ->
+    unless direction? then throw "Direction is required"
+    dir = if direction is "up" then -1 else +1
+    wrapper = @annotator.wrapper
+    defaultView = wrapper[0].ownerDocument.defaultView
+    pad = defaultView.innerHeight * .2
+    $(@_highlights).scrollintoview
+      complete: ->
+        scrollable = if this.parentNode is this.ownerDocument
+          $(this.ownerDocument.body)
+        else
+          $(this)
+        top = scrollable.scrollTop()
+        correction = pad * dir
+        scrollable.stop().animate {scrollTop: top + correction}, 300
+
+
+# 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 Annotator.Plugin.TextHighlights extends Annotator.Plugin
 

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

@@ -62,7 +62,7 @@ class Annotator.Plugin.TextRange extends Annotator.Plugin
   # Create a RangeSelector around a range
   _getRangeSelector: (selection) =>
     return [] unless selection.type is "text range"
-    sr = selection.range.serialize @annotator.wrapper[0], '.' + @Annotator.TextHighlight.highlightClass
+    sr = selection.range.serialize @annotator.wrapper[0], '.annotator-hl'
     [
       type: "RangeSelector"
       startContainer: sr.start

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

@@ -2,10 +2,6 @@
 class Annotator.Plugin.TextSelection extends Annotator.Plugin
 
   pluginInit: ->
-    # We need text highlights
-    unless @annotator.plugins.TextHighlights
-      throw new Error "The TextSelection Annotator plugin requires the TextHighlights plugin."
-
     @Annotator = Annotator
     @$ = Annotator.$
 
@@ -88,9 +84,6 @@ class Annotator.Plugin.TextSelection extends Annotator.Plugin
 
     for range in selectedRanges
       container = range.commonAncestor
-      # TODO: what is selection ends inside a different type of highlight?
-      if @Annotator.TextHighlight.isInstance container
-        container = @Annotator.TextHighlight.getIndependentParent container
       return if @annotator.isAnnotator(container)
 
     if selectedRanges.length

h/static/scripts/vendor/annotator.auth.js

 
 /*
-** Annotator v1.2.9-dev-b091a74
+** Annotator v1.2.9-dev-368b5cf
 ** https://github.com/okfn/annotator/
 **
 ** Copyright 2015, the Annotator project contributors.
 ** Dual licensed under the MIT and GPLv3 licenses.
 ** https://github.com/okfn/annotator/blob/master/LICENSE
 **
-** Built at: 2015-01-19 11:35:26Z
+** Built at: 2015-01-19 16:53:10Z
  */
 
 

h/static/scripts/vendor/annotator.document.js

 
 /*
-** Annotator v1.2.9-dev-b091a74
+** Annotator v1.2.9-dev-368b5cf
 ** https://github.com/okfn/annotator/
 **
 ** Copyright 2015, the Annotator project contributors.
 ** Dual licensed under the MIT and GPLv3 licenses.
 ** https://github.com/okfn/annotator/blob/master/LICENSE
 **
-** Built at: 2015-01-19 11:35:26Z
+** Built at: 2015-01-19 16:53:10Z
  */
 
 

h/static/scripts/vendor/annotator.js

 
 /*
-** Annotator v1.2.9-dev-b091a74
+** Annotator v1.2.9-dev-368b5cf
 ** https://github.com/okfn/annotator/
 **
 ** Copyright 2015, the Annotator project contributors.
 ** Dual licensed under the MIT and GPLv3 licenses.
 ** https://github.com/okfn/annotator/blob/master/LICENSE
 **
-** Built at: 2015-01-19 11:35:26Z
+** Built at: 2015-01-19 16:53:10Z
  */
 
 
@@ -1236,9 +1236,6 @@
       for (_k = 0, _len2 = _ref1.length; _k < _len2; _k++) {
         range = _ref1[_k];
         container = range.commonAncestor;
-        if ($(container).hasClass('annotator-hl')) {
-          container = $(container).parents('[class!=annotator-hl]')[0];
-        }
         if (this.isAnnotator(container)) {
           return;
         }
@@ -1251,7 +1248,7 @@
     };
 
     Annotator.prototype.isAnnotator = function(element) {
-      return !!$(element).parents().addBack().filter('[class^=annotator-]').not(this.wrapper).length;
+      return !!$(element).parents().addBack().filter('[class^=annotator-]').not('[class=annotator-hl]').not(this.wrapper).length;
     };
 
     Annotator.prototype.onHighlightMouseover = function(event) {