Merge pull request #2569 from hypothesis/clean-up-thread-show-logic

Consolidate thread show/hide logic into ThreadController

h/static/scripts/directive/test/thread-test.coffee

@@ -100,6 +100,52 @@ describe 'thread', ->
         controller.toggleCollapsed()
         assert.isTrue(controller.collapsed)
 
+    describe '#shouldShow()', ->
+      count = null
+
+      beforeEach ->
+        createDirective()
+        count = sinon.stub().returns(0)
+        controller.counter = {count: count}
+
+      it 'is true by default', ->
+        assert.isTrue(controller.shouldShow())
+
+      describe 'when the thread root is an orphan', ->
+        beforeEach ->
+          $scope.feature = -> false
+          controller.container =
+            message:
+              $orphan: true
+
+        it 'returns false', ->
+          assert.isFalse(controller.shouldShow())
+
+        it 'returns true if show_unanchored_annotations is on', ->
+          $scope.feature = -> true
+          assert.isTrue(controller.shouldShow())
+
+      describe 'when the thread filter is active', ->
+        beforeEach ->
+          controller.filter = {active: -> true}
+
+        it 'is false when there are no matches in the thread', ->
+          assert.isFalse(controller.shouldShow())
+
+        it 'is true when there are matches in the thread', ->
+          count.withArgs('match').returns(1)
+          assert.isTrue(controller.shouldShow())
+
+        it 'is true when there are edits in the thread', ->
+          count.withArgs('edit').returns(1)
+          assert.isTrue(controller.shouldShow())
+
+        it 'is true when the thread is new', ->
+          controller.container =
+            # message exists but there is no id field
+            message: {}
+          assert.isTrue(controller.shouldShow())
+
     describe '#shouldShowAsReply', ->
       count = null
 

h/static/scripts/directive/thread.coffee

@@ -13,7 +13,8 @@ uuid = require('node-uuid')
 # the collapsing behavior.
 ###
 ThreadController = [
-  ->
+  '$scope',
+  ($scope) ->
     @container = null
     @collapsed = true
     @parent = null
@@ -35,6 +36,26 @@ ThreadController = [
                  not @collapsed
       @collapsed = newval
 
+    ###*
+    # @ngdoc method
+    # @name thread.ThreadController#shouldShow
+    # @description
+    # Return a boolean indicating whether this thread should be shown given the
+    # current system state.
+    ###
+    this.shouldShow = ->
+      if this.container?.message?.$orphan == true
+        # Hide unless show_unanchored_annotations is turned on
+        if not $scope.feature('show_unanchored_annotations')
+          return false
+
+      editing = this._count('edit') > 0
+      matching = this._count('match') > 0
+      if this._isFilterActive() and not (this.isNew() or editing or matching)
+        return false
+
+      return true
+
     ###*
     # @ngdoc method
     # @name thread.ThreadController#shouldShowAsReply

h/static/scripts/test/widget-controller-test.coffee

@@ -86,34 +86,3 @@ describe 'WidgetController', ->
       assert.calledWith(loadSpy, [40..59])
       assert.calledWith(loadSpy, [60..79])
       assert.calledWith(loadSpy, [80..99])
-
-  describe 'shouldShowThread()', ->
-    it 'returns false for orphan annotations', ->
-      # Turn the 'show_unanchored_annotations' feature off.
-      $scope.feature = -> false
-
-      container =
-        message:
-          $orphan: true
-
-      assert($scope.shouldShowThread(container) is false)
-
-    it 'returns true for non-orphan annotations', ->
-      # Turn the 'show_unanchored_annotations' feature off.
-      $scope.feature = -> false
-
-      container =
-        message:
-          $orphan: false
-
-      assert($scope.shouldShowThread(container) is true)
-
-    it 'returns true for orphan annotations if show_unanchored_annotations is on', ->
-      # Turn the 'show_unanchored_annotations' feature on.
-      $scope.feature = -> true
-
-      container =
-        message:
-          $orphan: true
-
-      assert($scope.shouldShowThread(container) is true)

h/static/scripts/widget-controller.coffee

@@ -57,15 +57,5 @@ module.exports = class WidgetController
       if angular.isObject annotation
         crossframe.call('scrollToAnnotation', annotation.$$tag)
 
-    $scope.shouldShowThread = (container) ->
-      # Show regardless of $orphan if that feature is turned on
-      if $scope.feature('show_unanchored_annotations')
-        return true
-
-      if container?.message?.$orphan == true
-        return false
-
-      return true
-
     $scope.hasFocus = (annotation) ->
       !!($scope.focusedAnnotations ? {})[annotation?.$$tag]

h/templates/client/viewer.html

@@ -44,7 +44,7 @@
       ng-click="scrollTo(child.message)"
       ng-mouseleave="focus()"
       ng-repeat="child in threadRoot.children | orderBy : sort.predicate"
-      ng-show="shouldShowThread(child) && (count('edit') || count('match') || !threadFilter.active()) || vm.isNew()">
+      ng-show="vm.shouldShow()">
   </li>
 </ul>
 <!-- / Thread view -->