Merge pull request #1709 from hypothesis/1238-restore-visual-diff

[WIP] Resurrect the removed visual diff support

h/static/scripts/directives/annotation.coffee

@@ -45,6 +45,8 @@ AnnotationController = [
     @preview = 'no'
     @editing = false
     @embedded = false
+    @hasDiff = false
+    @showDiff = undefined
 
     highlight = annotator.tool is 'highlight'
     model = $scope.annotationGet()
@@ -177,6 +179,9 @@ AnnotationController = [
       # Note that copy is used so that deep properties aren't shared.
       angular.extend @annotation, angular.copy model
 
+      # Set the URI
+      @annotationURI = documentHelpers.absoluteURI("/a/#{@annotation.id}")
+
       # Extract the document metadata.
       if model.document
         uri = model.uri
@@ -205,6 +210,12 @@ AnnotationController = [
       # Form the tags for ngTagsInput.
       @annotation.tags = ({text} for text in (model.tags or []))
 
+      # Calculate the visual diff flags
+      @hasDiff = false
+      for t in @annotation.target or [] when t.diffHTML? and not t.diffCaseOnly
+        @hasDiff = t.hasDiff = true
+      @showDiff ?= @hasDiff or undefined
+
     # Export the baseURI for the share link
     this.baseURI = documentHelpers.baseURI
 
@@ -212,23 +223,23 @@ AnnotationController = [
     $scope.$on '$destroy', ->
       drafts.remove model
 
-    # Render on updates.
-    $scope.$watch (-> model.updated), (updated) =>
-      if updated then drafts.remove model
-      this.render()  # XXX: TODO: don't clobber the view when collaborating
+    # Watch the model.
+    # XXX: TODO: don't clobber the view when collaborating
+    $scope.$watch (-> model), (model, old) =>
+      # Discard saved drafts
+      if model.updated != old.updated
+        drafts.remove model
 
-    # Update once logged in.
-    $scope.$watch (-> model.user), (user) =>
+      # Save highlights once logged in.
       if highlight and this.isHighlight()
-        if user
+        if model.user
           annotator.publish 'annotationCreated', model
+          highlight = false  # skip this on future updates
         else
-          drafts.add model, => this.revert()
-      else
-        this.render()
+          drafts.add model, -> this.revert()
 
-    $scope.$watch (=> @annotation.id), =>
-      vm.annotationURI = documentHelpers.absoluteURI("/a/#{@annotation.id}")
+      this.render()
+    , true
 
     # Start editing brand new annotations immediately
     unless model.id? or (highlight and this.isHighlight()) then this.edit()

h/static/scripts/services.coffee

@@ -313,6 +313,9 @@ class Hypothesis extends Annotator
       # Check the root
       update scope.threading.root
 
+      # Update the view
+      this.digest()
+
   considerSocialView: (query) ->
     switch @socialView.name
       when "none"

h/templates/client/annotation.html

@@ -44,11 +44,22 @@
 <section class="annotation-target"
          ng-repeat="target in vm.annotation.target track by $index">
   <blockquote class="annotation-quote"
+              ng-hide="target.hasDiff && vm.showDiff"
               ng-bind-html="selector.exact"
               ng-repeat="selector in target.selector
                          | filter : {'type': 'TextQuoteSelector'}
                          track by $index"></blockquote>
+  <blockquote class="annotation-quote"
+              ng-bind-html="target.diffHTML"
+              ng-show="target.hasDiff && vm.showDiff"></blockquote>
 </section>
+<div class="small pull-right"
+     ng-show="vm.hasDiff">
+  <input type="checkbox"
+         ng-model="vm.showDiff"
+         ng-click="$event.stopPropagation()"> Show differences</input>
+</div>
+
 <!-- / Excerpts -- >
 
 <!-- Body -->

tests/js/directives/annotation-test.coffee

@@ -31,46 +31,6 @@ describe 'h.directives.annotation', ->
   afterEach ->
     sandbox.restore()
 
-  it 'provides a document title', ->
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'A special document')
-
-  it 'uses the first title when there are more than one', ->
-    annotation.document.title = ['first title', 'second title']
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'first title')
-
-  it 'truncates long titles', ->
-    annotation.document.title = '''A very very very long title that really
-    shouldn't be found on a page on the internet.'''
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'A very very very long title th…')
-
-  it 'provides a document uri', ->
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.uri, 'http://example.com')
-
-  it 'provides an extracted domain from the uri', ->
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.domain, 'example.com')
-
-  it 'uses the domain for the title if the title is not present', ->
-    delete annotation.document.title
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.document.title, 'example.com')
-
-  it 'skips the document object if no document is present on the annotation', ->
-    delete annotation.document
-    controller = createController()
-    $scope.$digest()
-    assert.isNull(controller.document)
-
   describe '#reply', ->
     controller = null
     container = null
@@ -107,3 +67,102 @@ describe 'h.directives.annotation', ->
       controller.reply()
       newAnnotation = annotator.publish.lastCall.args[1]
       assert.notInclude(newAnnotation.permissions.read, 'group:__world__')
+
+  describe '#render', ->
+    controller = null
+
+    beforeEach ->
+      controller = createController()
+      sandbox.spy(controller, 'render')
+
+    afterEach ->
+      sandbox.restore()
+
+    it 'is called exactly once during the first digest', ->
+      $scope.$digest()
+      assert.calledOnce(controller.render)
+
+    it 'is called exactly once on model changes', ->
+      $scope.$digest()
+      assert.calledOnce(controller.render)
+      $scope.$digest()
+      assert.calledOnce(controller.render)  # still
+
+      annotation.booz = 'baz'
+      $scope.$digest()
+      assert.calledTwice(controller.render)
+
+    it 'provides a document title', ->
+      controller.render()
+      assert.equal(controller.document.title, 'A special document')
+
+    it 'uses the first title when there are more than one', ->
+      annotation.document.title = ['first title', 'second title']
+      controller.render()
+      assert.equal(controller.document.title, 'first title')
+
+    it 'truncates long titles', ->
+      annotation.document.title = '''A very very very long title that really
+      shouldn't be found on a page on the internet.'''
+      controller.render()
+      assert.equal(controller.document.title, 'A very very very long title th…')
+
+    it 'provides a document uri', ->
+      controller.render()
+      assert.equal(controller.document.uri, 'http://example.com')
+
+    it 'provides an extracted domain from the uri', ->
+      controller.render()
+      assert.equal(controller.document.domain, 'example.com')
+
+    it 'uses the domain for the title if the title is not present', ->
+      delete annotation.document.title
+      controller.render()
+      assert.equal(controller.document.title, 'example.com')
+
+    it 'skips the document object if no document is present on the annotation', ->
+      delete annotation.document
+      controller.render()
+      assert.isNull(controller.document)
+
+    describe 'when targets have the same selection text as the anchor', ->
+      it 'sets `showDiff` to undefined and `hasDiff` to false', ->
+        controller.render()
+        assert.isFalse(controller.hasDiff)
+        assert.isUndefined(controller.showDiff)
+
+    describe 'when targets have different selection text from the anchor', ->
+      targets = null
+
+      beforeEach ->
+        annotation.target = [
+          {otherProperty: 'bar'},
+          {diffHTML: "things"},
+          {diffHTML: "stuff", diffCaseOnly: true},
+        ]
+        controller.render()
+        targets = controller.annotation.target
+
+      it 'sets `hasDiff` to true', ->
+        assert.isTrue(controller.hasDiff)
+
+      it 'sets `showDiff` to true', ->
+        assert.isTrue(controller.showDiff)
+
+      it 'sets `hasDiff` to true on targets with differences', ->
+        assert.match(targets[0].hasDiff, sinon.match.falsy)
+        assert.match(targets[1].hasDiff, sinon.match.truthy)
+
+      it 'sets `hasDiff` to false on targets with only case differences', ->
+        assert.match(targets[2].hasDiff, sinon.match.falsy)
+
+      it 'preserves the `showDiff` value on update', ->
+        controller.showDiff = false
+        annotation.target = annotation.target.slice(1)
+        controller.render()
+        assert.isFalse(controller.showDiff)
+
+      it 'unsets `hasDiff` if differences go away', ->
+        annotation.target = annotation.target.splice(0, 1)
+        controller.render()
+        assert.isFalse(controller.hasDiff)