Add a directive for tracking nested aggregates

Rather than continuing to track the message counts in the thread
controller, make a separate deep-count directive that exports a
getter / setter from its controller that can be used to query and
increment counters that bubble aggreggations up to any deep-count
directive above.

Instead of firing a `threadCollapse` event when threads collapse
and canceling it from the annotation directive when the annotation
is being edited, the deep-count directive is used to track the
total number of messages in a thead, the number of matches found
by the thread-filter directive, and the number of annotations that
are being edited.

Remove most of the injectables from the thread controller and handle
more of the DOM-specific bits in the link function. The controller
becomes even simpler, but the directive needs tests now.

h/static/scripts/directives/annotation.coffee

@@ -96,8 +96,8 @@ AnnotationController = [
         annotator.publish 'annotationDeleted', model
       else
         this.render()
-      @action = 'view'
-      @editing = false
+        @action = 'view'
+        @editing = false
 
     ###*
     # @ngdoc method
@@ -152,10 +152,6 @@ AnnotationController = [
     $scope.$on '$destroy', ->
       drafts.remove model
 
-    # Prevent threads from collapsing during editing.
-    $scope.$on 'threadCollapse', (event) ->
-      event.preventDefault() if vm.editing
-
     # Render on updates.
     $scope.$watch (-> model.updated), (updated) ->
       if updated then drafts.remove model
@@ -196,14 +192,14 @@ AnnotationController = [
 # an embedded widget.
 ###
 annotation = ['annotator', (annotator) ->
-  linkFn = (scope, elem, attrs, [ctrl, threadCtrl]) ->
+  linkFn = (scope, elem, attrs, [ctrl, thread, counter]) ->
     # Helper function to remove the temporary thread created for a new reply.
     prune = (message) ->
       return if message.id?  # threading plugin will take care of it
-      return unless threadCtrl.container.message is message
-      threadCtrl.container.parent?.removeChild(threadCtrl.container)
+      return unless thread.container.message is message
+      thread.container.parent?.removeChild(thread.container)
 
-    if threadCtrl?
+    if thread?
       annotator.subscribe 'annotationDeleted', prune
       scope.$on '$destroy', ->
         annotator.unsubscribe 'annotationDeleted', prune
@@ -219,10 +215,19 @@ annotation = ['annotator', (annotator) ->
         scope.$evalAsync ->
           ctrl.save()
 
+    scope.$on '$destroy', ->
+      if ctrl.editing then counter?.count 'edit', -1
+
+    scope.$watch (-> ctrl.editing), (editing, old) ->
+      if editing
+        counter?.count 'edit', 1
+      else if old
+        counter?.count 'edit', -1
+
   controller: 'AnnotationController'
   controllerAs: 'vm'
   link: linkFn
-  require: ['annotation', '?^thread']
+  require: ['annotation', '?^thread', '?^deepCount']
   scope:
     annotationGet: '&annotation'
   templateUrl: 'annotation.html'
@@ -230,5 +235,5 @@ annotation = ['annotator', (annotator) ->
 
 
 angular.module('h.directives')
-.controller('AnnotationController', AnnotationController)
-.directive('annotation', annotation)
+.controller('AnnotationController', AnnotationController).
+directive('annotation', annotation)

h/static/scripts/directives/deepCount.coffee

+###*
+# @ngdoc type
+# @name deepCount.DeepCountController
+#
+# @description
+# `DeepCountController` exports a single getter / setter that can be used
+# to retrieve and manipulate a set of counters. Changes to these counters
+# are bubbled and aggregated by any instances higher up in the DOM. Digests
+# are performed from the top down, scheduled during animation frames, and
+# debounced for performance.
+###
+DeepCountController = [
+  '$element', '$scope', 'render',
+  ($element,   $scope,   render) ->
+    counters = {}
+    parent = $element.parent().controller('deepCount')
+
+    ###*
+    # @ngdoc method
+    # @name deepCount.DeepCountController#count
+    #
+    # @param {string} key An aggregate key.
+    # @param {number} delta If provided, the amount by which the aggregate
+    # for the given key should be incremented.
+    # @return {number} The value of the aggregate for the given key.
+    #
+    # @description
+    # Modify an aggregate when called with an argument and return its current
+    # value.
+    ###
+    this.count = do -> __cancelFrame = null; (key, delta) ->
+      if delta is undefined or delta is 0 then return counters[key] or 0
+      counters[key] ?= 0
+      counters[key] += delta
+
+      unless counters[key] then delete counters[key]
+
+      if parent
+        # Bubble updates.
+        parent.count key, delta
+      else
+        # Debounce digests from the top.
+        if __cancelFrame then __cancelFrame()
+        __cancelFrame = render ->
+          $scope.$digest()
+          __cancelFrame = null
+
+      counters[key] or 0
+
+    this
+]
+
+
+
+###*
+# @ngdoc directive
+# @name deepCount
+# @restrict A
+# @description
+# Directive that instantiates
+# {@link deepCount.DeepCountController DeepCountController} and exports it
+# to the current scope under the name specified by the attribute parameter.
+###
+deepCount = [
+  '$parse',
+  ($parse) ->
+    controller: 'DeepCountController'
+    link: (scope, elem, attrs, ctrl) ->
+      parsedCounterName = $parse attrs.deepCount
+      if parsedCounterName.assign
+        parsedCounterName.assign scope, angular.bind ctrl, ctrl.count
+]
+
+
+angular.module('h.directives')
+.controller('DeepCountController', DeepCountController)
+.directive('deepCount', deepCount)

h/static/scripts/directives/thread.coffee

@@ -17,15 +17,12 @@ COLLAPSED_CLASS = 'thread-collapsed'
 # replying and sharing.
 ###
 ThreadController = [
-  '$attrs', '$element', '$parse', '$scope', 'flash', 'render',
-  ($attrs,   $element,   $parse,   $scope,   flash,   render) ->
+  ->
     @container = null
     @collapsed = false
     @hover = false
     @shared = false
-    @messageCount = all: 0
 
-    parentThread = $element.parent().controller('thread')
     vm = this
 
     ###*
@@ -35,8 +32,6 @@ ThreadController = [
     # Creates a new message in reply to this thread.
     ###
     this.reply = ->
-      unless @container.message.id
-        return flash 'error', 'You must publish this before replying to it.'
       if @collapsed then this.toggleCollapsed()
 
       # Extract the references value from this container.
@@ -57,16 +52,10 @@ ThreadController = [
     # @ngdoc method
     # @name thread.ThreadController#toggleCollapsed
     # @description
-    # Fire a `threadCollapse` event and toggle the collapsed property
-    # unless a listener has called the `preventDefault` method on the event.
+    # Toggle the collapsed property.
     ###
     this.toggleCollapsed = ->
-      return if ($scope.$broadcast 'threadCollapse').defaultPrevented
       @collapsed = not @collapsed
-      if @collapsed
-        $attrs.$addClass(COLLAPSED_CLASS)
-      else
-        $attrs.$removeClass(COLLAPSED_CLASS)
 
     ###*
     # @ngdoc method
@@ -75,57 +64,11 @@ ThreadController = [
     # Toggle the shared property.
     ###
     this.toggleShared = ->
-      unless @container.message.id
-        return flash 'error', 'You must publish this before sharing it.'
       @shared = not @shared
-      if @shared
-        # Focus and select the share link
-        $scope.$evalAsync ->
-          $element.find('input').focus().select()
-
-    this.addMessageCount = do -> __cancelFrame = null; (delta, key='all') ->
-      @messageCount[key] ?= 0
-      @messageCount[key] += delta
-
-      if parentThread
-        # Bubble updates.
-        parentThread.addMessageCount delta, key
-      else
-        # Debounce digests from the top.
-        if __cancelFrame then __cancelFrame()
-        __cancelFrame = render ->
-          $scope.$digest()
-          __cancelFrame = null
-
-    # Hide the view initially
-    $element.hide()
-
-    $scope.$on '$destroy', ->
-      if parentThread then parentThread.addMessageCount -1
-
-    # Render the view in a future animation frame
-    render ->
-      vm.container = $parse($attrs.thread)($scope)
-      vm.addMessageCount(if vm.container.message then 1 else 0)
-      $scope.$digest()
-      $element.show()
-
-    # Watch the thread-collapsed attribute.
-    if $attrs.threadCollapsed
-      $scope.$watch $parse($attrs.threadCollapsed), (collapsed) ->
-        vm.toggleCollapsed() if !!collapsed != vm.collapsed
 
     this
 ]
 
-###*
-# @ngdoc event
-# @name thread#threadCollapse
-# @eventType broadcast on the current thread scope
-# @description
-# Broadcast before a thread collapse state is changed. The change can be
-# prevented by calling the `preventDefault` method of the event.
-###
 
 ###*
 # @ngdoc directive
@@ -139,10 +82,10 @@ ThreadController = [
 # the collapsed state of the thread.
 ###
 thread = [
-  '$document', '$window',
-  ($document,   $window) ->
-    linkFn = (scope, elem, attrs, ctrl) ->
-      # Toggle collapse on click
+  '$document', '$parse', '$window', 'render',
+  ($document,   $parse,   $window,   render) ->
+    linkFn = (scope, elem, attrs, [ctrl, counter]) ->
+      # Toggle collapse on click.
       elem.on 'click', (event) ->
         event.stopPropagation()
 
@@ -156,16 +99,50 @@ thread = [
         if sel.containsNode(event.target, true) and sel.toString().length
           return
 
-        # Ignore if the user just activated a form element
+        # Ignore if the user just activated a form element.
         if $document.activeElement is event.target
           return
 
+        # Ignore if edit interactions are present in the view.
+        if counter?.count('edit') > 0
+          return
+
         scope.$evalAsync ->
           ctrl.toggleCollapsed()
 
+      # Hide the element initially.
+      elem.hide()
+
+      # Queue a render frame to complete the binding and show the element.
+      render ->
+        ctrl.container = $parse(attrs.thread)(scope)
+        if ctrl.container.message? and counter?
+          counter.count 'message', 1
+          scope.$on '$destroy', -> counter.count 'message', -1
+        scope.$digest()
+        elem.show()
+
+      # Add and remove the collapsed class when the collapsed property changes.
+      scope.$watch (-> ctrl.collapsed), (collapsed) ->
+        if collapsed
+          attrs.$addClass COLLAPSED_CLASS
+        else
+          attrs.$removeClass COLLAPSED_CLASS
+
+      # Focus and select the share link when it becomes available.
+      scope.$watch (-> ctrl.shared), (shared) ->
+        if shared then scope.$evalAsync ->
+          elem.find('footer').find('input').focus().select()
+
+      # Watch the thread-collapsed attribute.
+      if attrs.threadCollapsed
+        scope.$watch $parse(attrs.threadCollapsed), (collapsed) ->
+          ctrl.toggleCollapsed() if !!collapsed != ctrl.collapsed
+
     controller: 'ThreadController'
     controllerAs: 'vm'
     link: linkFn
+    require: ['thread', '?^deepCount']
     scope: true
 ]
 

h/static/scripts/directives/threadFilter.coffee

@@ -48,13 +48,12 @@ ThreadFilterController = [
     # This method is a getter / setter.
     #
     # Set the filter configuration when called with an argument and return
-    # return the configuration. Resets the `hits` property to zero.
+    # return the configuration.
     ###
     this.filters = (filters) ->
       if filters is undefined then return @_filters
       else if @filters == filters then return @_filters
       else
-        @hits = 0
         child.filters filters for child in @_children
         @_filters = filters
 
@@ -69,21 +68,18 @@ ThreadFilterController = [
     # @description
     # Check whether a message container carries a message matching the
     # configured filters. If filtering is not active then the result is
-    # always `true`. Similarly, messages without an `id` are always treated
-    # as matches so that they are always visible for editing. Updates the
-    # `hits` property when a match is found that has not been seen before.
+    # always `true`.  Updates the `hits` property to reflect changes in match
+    # status of the container by caching the result is the `__filterResult`
+    # property of the container.
     ###
     this.check = (container) ->
-      unless this.active() then return true
-      unless container.__filters is @_filters
-        if container.message.id
-          match = !!viewFilter.filter([container.message], @_filters).length
-        else
-          match = true
-        if match then @hits++
-        container.__filters = @_filters
-        container.__filterResult = match
-      container.__filterResult
+      unless container? then return false
+      if this.active()
+        match = !!viewFilter.filter([container.message], @_filters).length
+      else
+        match = true
+      @hits += (match - !!container.__filterResult)
+      container.__filterResult = match
 
     ###*
     # @ngdoc method
@@ -133,16 +129,13 @@ ThreadFilterController = [
 threadFilter = [
   '$parse', 'searchfilter'
   ($parse,   searchfilter) ->
-    linkFn = (scope, elem, attrs, [ctrl, threadCtrl]) ->
+    linkFn = (scope, elem, attrs, [ctrl, thread, counter]) ->
       scope.$watch (-> ctrl.hits), (newValue=0, oldValue=0) ->
-        if (delta = newValue - oldValue) == 0 then return
-        threadCtrl.addMessageCount delta, 'filter'
+        counter?.count 'match', newValue - oldValue
 
       scope.$on '$destroy', ->
-        threadCtrl.addMessageCount -ctrl.hits, 'filter'
-        if threadCtrl.container?
-          delete threadCtrl.container.__filters
-          delete threadCtrl.container.__filterResult
+        if thread.container then delete thread.container.__filterResult
+        counter?.count 'match', -ctrl.hits
 
       if parentCtrl = elem.parent().controller('threadFilter')
         ctrl.filters parentCtrl.filters()
@@ -159,7 +152,7 @@ threadFilter = [
     controller: 'ThreadFilterController'
     controllerAs: 'threadFilter'
     link: linkFn
-    require: ['threadFilter', 'thread']
+    require: ['threadFilter', 'thread', '?^deepCount']
 ]
 
 

tests/js/directives/thread-test.coffee

@@ -12,33 +12,18 @@ describe 'h.directives.thread', ->
   beforeEach module('h.directives')
 
   beforeEach inject ($controller, $rootScope) ->
-    $attrs =
-      $addClass: sinon.spy()
-      $removeClass: sinon.spy()
-      thread: 'thread'
     $scope = $rootScope.$new()
     flash = sinon.spy()
-    render = (cb) -> cb()
 
     createController = ->
-      $scope.thread = mail.messageContainer()
-      $scope.thread.message = id: 'foo', uri: 'http://example.com/'
-      $element = angular.element('<div thread="thread"><input /></div>')
-      $controller 'ThreadController', {$attrs, $element, $scope, flash, render}
+      controller = $controller 'ThreadController'
+      controller.container = mail.messageContainer()
+      controller.container.message = id: 'foo', uri: 'http://example.com/'
+      controller
 
   afterEach ->
     sandbox.restore()
 
-  it 'sets its initial collapsed state', ->
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.collapsed, false, 'defaults to false')
-
-    $attrs.threadCollapsed = 'true'
-    controller = createController()
-    $scope.$digest()
-    assert.equal(controller.collapsed, true, 'accepts a boolean value')
-
   describe '#reply', ->
     controller = null
     container = null
@@ -47,8 +32,8 @@ describe 'h.directives.thread', ->
 
     beforeEach ->
       controller = createController()
-      container = $scope.thread
-      message = container.message
+      {container} = controller
+      {message} = container
 
     it 'expands if collapsed', ->
       controller.reply()
@@ -82,24 +67,6 @@ describe 'h.directives.thread', ->
       after = controller.collapsed
       assert.equal(before, !after)
 
-    it 'adds and removes the collapsed class', ->
-      controller = createController()
-      controller.toggleCollapsed()
-      assert.calledWith($attrs.$addClass, 'thread-collapsed')
-      controller.toggleCollapsed()
-      assert.calledWith($attrs.$removeClass, 'thread-collapsed')
-
-    it 'broadcasts the threadCollapse event', ->
-      $scope.$on 'threadCollapse', (event) ->
-        event.preventDefault()
-      sandbox.spy($scope, '$broadcast')
-      controller = createController()
-      before = controller.collapsed
-      controller.toggleCollapsed()
-      after = controller.collapsed
-      assert.called($scope.$broadcast)
-      assert.equal(before, after)
-
   describe '#toggleShared', ->
     it 'sets the shared property', ->
       controller = createController()
@@ -107,11 +74,3 @@ describe 'h.directives.thread', ->
       controller.toggleShared()
       after = controller.shared
       assert.equal(before, !after)
-
-    it 'focuses the first input element', ->
-      sandbox.spy(jQuery.fn, 'focus')
-      controller = createController()
-      controller.toggleShared()
-      inject(($timeout) -> $timeout.flush())
-      input = $element.find('input')
-      assert.deepEqual(jQuery.fn.focus.thisValues[0], input)