Merge pull request #2003 from hypothesis/fix-replies-hover-highlight

Fix replies hover highlight

h/static/scripts/directives/annotation.coffee

@@ -314,10 +314,15 @@ annotationDirective = [
             ctrl.save()
 
       scope.share = (event) ->
-        scope.$evalAsync ->
-          $container = angular.element(event.target).parent()
-          $container.addClass('open').find('input').focus().select()
-          $document.one('click', (event) -> $container.removeClass('open'))
+        $container = angular.element(event.currentTarget).parent()
+        $container.addClass('open').find('input').focus().select()
+
+        # We have to stop propagation here otherwise this click event will
+        # re-close the share dialog immediately.
+        event.stopPropagation()
+
+        $document.one('click', (event) -> $container.removeClass('open'))
+        return
 
       # Keep track of edits going on in the thread.
       if counter?
@@ -346,6 +351,9 @@ annotationDirective = [
     require: ['annotation', '?^thread', '?^threadFilter', '?^deepCount']
     scope:
       annotationGet: '&annotation'
+      replyCount: '@annotationReplyCount'
+      replyCountClick: '&annotationReplyCountClick'
+      showReplyCount: '@annotationShowReplyCount'
     templateUrl: 'annotation.html'
 ]
 

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

@@ -24,15 +24,22 @@ describe 'h:directives.thread', ->
         controller
 
     describe '#toggleCollapsed', ->
-      it 'toggles whether or not the thread is collapsed', ->
+      controller = null
+      count = null
+
+      beforeEach ->
         controller = createController()
+        count = sinon.stub().returns(0)
+        count.withArgs('message').returns(2)
+        controller.counter = {count: count}
+
+      it 'toggles whether or not the thread is collapsed', ->
         before = controller.collapsed
         controller.toggleCollapsed()
         after = controller.collapsed
         assert.equal(before, !after)
 
       it 'can accept an argument to force a particular state', ->
-        controller = createController()
         controller.toggleCollapsed(true)
         assert.isTrue(controller.collapsed)
         controller.toggleCollapsed(true)
@@ -42,6 +49,15 @@ describe 'h:directives.thread', ->
         controller.toggleCollapsed(false)
         assert.isFalse(controller.collapsed)
 
+      it 'does not allow uncollapsing the thread if there are no replies', ->
+        count.withArgs('message').returns(1)
+        controller.toggleCollapsed()
+        assert.isTrue(controller.collapsed)
+        controller.toggleCollapsed()
+        assert.isTrue(controller.collapsed)
+        controller.toggleCollapsed(false)
+        assert.isTrue(controller.collapsed)
+
     describe '#shouldShowAsReply', ->
       controller = null
       count = null

h/static/scripts/directives/thread.coffee

@@ -27,10 +27,16 @@ ThreadController = [
     # thread filter, if present.
     ###
     this.toggleCollapsed = (value) ->
-      @collapsed = if value?
-                     !!value
-                   else
-                     not @collapsed
+      newval = if value?
+                 !!value
+               else
+                 not @collapsed
+
+      # We only allow uncollapsing of the thread if there are some replies to
+      # display.
+      if newval == false and this.numReplies() <= 0
+        return
+      @collapsed = newval
 
     ###*
     # @ngdoc method
@@ -195,31 +201,6 @@ thread = [
       ctrl.counter = counter
       ctrl.filter = filter
 
-      # Toggle collapse on click.
-      elem.on 'click', (event) ->
-        event.stopPropagation()
-
-        # Ignore if the target scope has been destroyed.
-        # Prevents collapsing when e.g. a child is deleted by a click event.
-        if angular.element(event.target).scope() is undefined
-          return
-
-        # Ignore if the user just created a non-empty selection.
-        sel = $window.getSelection()  # XXX: Portable containsNode?
-        if sel.containsNode?(event.target, true) and sel.toString().length
-          return
-
-        # Ignore if the user clicked a link
-        if event.target.tagName in ['A', 'INPUT']
-          return unless angular.element(event.target).hasClass 'reply-count'
-
-        # Ignore a collapse if edit interactions are present in the view.
-        if counter?.count('edit') > 0 and not ctrl.collapsed
-          return
-
-        scope.$evalAsync ->
-          ctrl.toggleCollapsed()
-
       # Track the number of messages in the thread
       if counter?
         counter.count 'message', 1

h/static/styles/annotations.scss

@@ -7,7 +7,12 @@
   font-weight: 300;
   position: relative;
 
-  &:hover .annotation-timestamp, &:hover .reply-count {
+  .reply-count {
+    color: $gray-light;
+    &:focus { outline: 0; }
+  }
+
+  &:hover .annotation-timestamp, &:hover .reply-count  {
     color: $link-color;
   }
 }
@@ -40,6 +45,10 @@
   }
 }
 
+.annotation-replies {
+  display: inline-block;
+}
+
 .annotation-actions {
   float: right;
   margin-top: 0;

h/static/styles/threads.scss

@@ -22,10 +22,6 @@ $threadexp-width: .6em;
   }
 }
 
-.thread-reply {
-  display: inline-block
-}
-
 .thread {
   @include pie-clearfix;
   cursor: pointer;
@@ -36,11 +32,6 @@ $threadexp-width: .6em;
     margin-left: -$thread-padding;
   }
 
-  .reply-count {
-    color: $gray-light;
-    &:focus { outline: 0; }
-  }
-
   @-webkit-keyframes pulse {
     10% { background-color: #ffc; }
   }
@@ -93,16 +84,6 @@ $threadexp-width: .6em;
   margin: .8em 0;
 }
 
-.thread-message:hover + .thread-reply {
-  .reply-count {
-    color: $link-color;
-
-    &:hover, &:focus {
-      color: $link-color-hover;
-    }
-  }
-}
-
 .thread-load-more {
   clear: both;
 }

h/templates/client/annotation.html

@@ -121,27 +121,37 @@
   </a>
 </div>
 
-<footer class="annotation-footer annotation-actions"
+<footer class="annotation-footer"
         ng-if="!vm.editing && vm.annotation.id">
-  <a class="small magicontrol" href="" title="Reply"
-    ng-click="vm.reply()"
-    ><i class="h-icon-reply"></i> Reply</a>
-  <span class="magicontrol share-dialog-wrapper">
-    <a class="small" href="" title="Share" ng-click="share($event)"
-      ><i class="h-icon-export"></i> Share</a>
-    <span class="share-dialog" ng-click="$event.stopPropagation()">
-      <a class="h-icon-export"
-        target="_blank"
-        ng-href="{{vm.annotationURI}}"></a>
-      <input type="text" value="{{vm.annotationURI}}" readonly>
+
+  <div class="annotation-replies">
+    <a class="reply-count small" href=""
+       ng-click="replyCountClick()"
+       ng-pluralize count="replyCount"
+       when="{'0': '', 'one': '1 reply', 'other': '{} replies'}"></a>
+  </div>
+
+  <div class="annotation-actions">
+    <a class="small magicontrol" href="" title="Reply"
+      ng-click="vm.reply()"
+      ><i class="h-icon-reply"></i> Reply</a>
+    <span class="magicontrol share-dialog-wrapper">
+      <a class="small" href="" title="Share" ng-click="share($event)"
+        ><i class="h-icon-export"></i> Share</a>
+      <span class="share-dialog" ng-click="$event.stopPropagation()">
+        <a class="h-icon-export"
+          target="_blank"
+          ng-href="{{vm.annotationURI}}"></a>
+        <input type="text" value="{{vm.annotationURI}}" readonly>
+      </span>
     </span>
-  </span>
-  <a class="small magicontrol" href="" title="Edit"
-    ng-show="vm.authorize('update')"
-    ng-click="vm.edit()"
-    ><i class="h-icon-copy"></i> Edit</a>
-  <a class="small magicontrol" href="" title="Delete"
-    ng-show="vm.authorize('delete')"
-    ng-click="vm.delete()"
-    ><i class="h-icon-x"></i> Delete…</a>
+    <a class="small magicontrol" href="" title="Edit"
+      ng-show="vm.authorize('update')"
+      ng-click="vm.edit()"
+      ><i class="h-icon-copy"></i> Edit</a>
+    <a class="small magicontrol" href="" title="Delete"
+      ng-show="vm.authorize('delete')"
+      ng-click="vm.delete()"
+      ><i class="h-icon-x"></i> Delete…</a>
+  </div>
 </footer>

h/templates/client/thread.html

-<a href="" class="threadexp"
+<a href=""
+   class="threadexp"
    title="{{vm.collapsed && 'Expand' || 'Collapse'}}"
-   ><span ng-class="{'h-icon-plus': !!vm.collapsed,
-                     'h-icon-minus': !vm.collapsed}"></span></a>
+   ng-click="vm.toggleCollapsed()">
+  <span ng-class="{'h-icon-plus': vm.collapsed,
+                   'h-icon-minus': !vm.collapsed}"></span>
+</a>
 
 <!-- Annotation -->
 <div ng-if="vm.container && !vm.container.message" class="thread-deleted">
@@ -12,20 +15,13 @@
          name="annotation"
          annotation="vm.container.message"
          annotation-embedded="{{isEmbedded}}"
+         annotation-show-reply-count="{{vm.shouldShowNumReplies()}}"
+         annotation-reply-count="{{vm.numReplies()}}"
+         annotation-reply-count-click="vm.toggleCollapsed()"
          ng-if="vm.container.message"
          ng-show="vm.matchesFilter()">
 </article>
 
-<!-- Reply count -->
-<div class="thread-reply"
-     ng-show="vm.shouldShowNumReplies()">
-  <a class="reply-count small"
-     href=""
-     ng-pluralize
-     count="vm.numReplies()"
-     when="{'0': '', one: '1 reply', other: '{} replies'}"></a>
-</div>
-
 <div class="thread-load-more" ng-show="vm.shouldShowLoadMore()">
   <a class="load-more small"
      href=""