Merge pull request #24 from hypothesis/selection_tabs

Selection tabs tweaks.

h/static/scripts/annotation-metadata.js

@@ -103,12 +103,12 @@ function isNew(annotation) {
 
 /** Return `true` if the given annotation is a page note, `false` otherwise. */
 function isPageNote(annotation) {
-  return !isAnnotation(annotation) && !isReply(annotation)
+  return !isAnnotation(annotation) && !isReply(annotation);
 }
 
 /** Return `true` if the given annotation is a top level annotation, `false` otherwise. */
 function isAnnotation(annotation) {
-  return (annotation.target && annotation.target.length > 0 && annotation.target[0].selector);
+  return !!(annotation.target && annotation.target.length > 0 && annotation.target[0].selector);
 }
 
 /** Return a numeric key that can be used to sort annotations by location.

h/static/scripts/annotation-ui.js

@@ -10,7 +10,6 @@
 
 var immutable = require('seamless-immutable');
 var redux = require('redux');
-var uiConstants = require('./ui-constants');
 
 function freeze(selection) {
   if (Object.keys(selection).length) {
@@ -63,7 +62,7 @@ function initialState(settings) {
 
     filterQuery: null,
 
-    selectedTab: uiConstants.TAB_ANNOTATIONS,
+    selectedTab: null,
 
     // Key by which annotations are currently sorted.
     sortKey: 'Location',
@@ -126,6 +125,7 @@ function annotationsReducer(state, action) {
 }
 
 function reducer(state, action) {
+  /*jshint maxcomplexity: false*/
   state = annotationsReducer(state, action);
 
   switch (action.type) {

h/static/scripts/app-controller.js

@@ -5,6 +5,7 @@ var scrollIntoView = require('scroll-into-view');
 var events = require('./events');
 var parseAccountID = require('./filter/persona').parseAccountID;
 var scopeTimeout = require('./util/scope-timeout');
+var uiConstants = require('./ui-constants');
 
 function authStateFromUserID(userid) {
   if (userid) {
@@ -130,6 +131,9 @@ module.exports = function AppController(
   };
 
   $scope.clearSelection = function () {
+    if (!annotationUI.getState().selectedTab) {
+        annotationUI.selectTab(uiConstants.TAB_ANNOTATIONS);
+    }
     annotationUI.clearSelectedAnnotations();
   };
 

h/static/scripts/directive/search-status-bar.js

 'use strict';
 
+var uiConstants = require('../ui-constants');
+
 // @ngInject
 module.exports = function () {
   return {
-    controller: function () {},
+    bindToController: true,
+    controllerAs: 'vm',
+    controller: function () {
+      this.TAB_ANNOTATIONS = uiConstants.TAB_ANNOTATIONS;
+      this.TAB_NOTES = uiConstants.TAB_NOTES;
+    },
     restrict: 'E',
     scope: {
       filterActive: '<',
@@ -12,8 +19,6 @@ module.exports = function () {
       searchQuery: '<',
       selectedTab: '<',
       selectionCount: '<',
-      tabAnnotations: '<',
-      tabNotes: '<',
       totalAnnotations: '<',
       totalNotes: '<',
     },

h/static/scripts/directive/selection-tabs.js

 'use strict';
 
+var uiConstants = require('../ui-constants');
+
 module.exports = function () {
   return {
     bindToController: true,
     controllerAs: 'vm',
     //@ngInject
     controller: function (annotationUI) {
+      this.TAB_ANNOTATIONS = uiConstants.TAB_ANNOTATIONS;
+      this.TAB_NOTES = uiConstants.TAB_NOTES;
+
       this.selectTab = function (type) {
         annotationUI.clearSelectedAnnotations();
         annotationUI.selectTab(type);
@@ -13,11 +18,10 @@ module.exports = function () {
     },
     restrict: 'E',
     scope: {
+      isLoading: '<',
       selectedTab: '<',
       totalAnnotations: '<',
       totalNotes: '<',
-      tabAnnotations: '<',
-      tabNotes: '<',
     },
     template: require('../../../templates/client/selection_tabs.html'),
   };

h/static/scripts/directive/test/search-status-bar-test.js

@@ -27,13 +27,11 @@ describe('searchStatusBar', function () {
   context('when there is a selection', function () {
     it('should display the "Show all annotations (2)" message when there are 2 annotations', function () {
       var msg = 'Show all annotations';
-      var msgCount = '(2)'
+      var msgCount = '(2)';
       var elem = util.createDirective(document, 'searchStatusBar', {
         selectionCount: 1,
         totalAnnotations: 2,
         selectedTab: 'annotation',
-        tabAnnotations: 'annotation',
-        tabNotes: 'note',
       });
       var clearBtn = elem[0].querySelector('button');
       assert.include(clearBtn.textContent, msg);
@@ -47,8 +45,6 @@ describe('searchStatusBar', function () {
         selectionCount: 1,
         totalNotes: 3,
         selectedTab: 'note',
-        tabAnnotations: 'annotation',
-        tabNotes: 'note',
       });
       var clearBtn = elem[0].querySelector('button');
       assert.include(clearBtn.textContent, msg);

h/static/scripts/directive/test/selection-tabs-test.js

@@ -23,17 +23,14 @@ describe('selectionTabs', function () {
         selectedTab: 'annotation',
         totalAnnotations: '123',
         totalNotes: '456',
-        tabAnnotations: 'annotation',
-        tabNotes: 'note',
       });
 
       var tabs = elem[0].querySelectorAll('li');
-      var sups = elem[0].querySelectorAll('sup');
 
       assert.include(tabs[0].textContent, "Annotations");
       assert.include(tabs[1].textContent, "Notes");
-      assert.include(sups[0].textContent, "123");
-      assert.include(sups[1].textContent, "456");
+      assert.include(tabs[0].textContent, "123");
+      assert.include(tabs[1].textContent, "456");
     });
 
     it('should display annotations tab as selected', function () {
@@ -41,8 +38,6 @@ describe('selectionTabs', function () {
         selectedTab: 'annotation',
         totalAnnotations: '123',
         totalNotes: '456',
-        tabAnnotations: 'annotation',
-        tabNotes: 'note',
       });
 
       var tabs = elem[0].querySelectorAll('li');
@@ -55,8 +50,6 @@ describe('selectionTabs', function () {
         selectedTab: 'note',
         totalAnnotations: '123',
         totalNotes: '456',
-        tabAnnotations: 'annotation',
-        tabNotes: 'note',
       });
 
       var tabs = elem[0].querySelectorAll('li');

h/static/scripts/root-thread.js

@@ -59,7 +59,7 @@ function RootThread($rootScope, annotationUI, features, searchFilter, viewFilter
     }
 
     var threadFilterFn;
-    if (features.flagEnabled('selection_tabs') && !state.filterQuery) {
+    if (features.flagEnabled('selection_tabs') && !state.filterQuery && state.selectedTab) {
       threadFilterFn = function (thread) {
         if (state.selectedTab === uiConstants.TAB_ANNOTATIONS) {
           return thread.annotation && metadata.isAnnotation(thread.annotation);

h/static/scripts/test/annotation-metadata-test.js

@@ -237,12 +237,12 @@ describe('annotation-metadata', function () {
 
   describe ('.isAnnotation', function () {
     it ('returns true if an annotation is a top level annotation', function () {
-      assert(annotationMetadata.isAnnotation({
+      assert.isTrue(annotationMetadata.isAnnotation({
         target: [{selector: []}]
       }));
     });
     it ('returns false if an annotation has no target', function () {
-      assert.equal(annotationMetadata.isAnnotation({}), undefined);
+      assert.isFalse(annotationMetadata.isAnnotation({}));
     });
   });
 });

h/static/scripts/test/root-thread-test.js

@@ -196,7 +196,7 @@ describe('rootThread', function () {
       var threadFilterFn = fakeBuildThread.args[0][1].threadFilterFn;
 
       var annotation = {target: [{ selector: {} }]};
-      assert(threadFilterFn({annotation: annotation}));
+      assert.isDefined(threadFilterFn({annotation: annotation}));
     });
 
     it('generates a thread filter function to match notes', function () {
@@ -209,7 +209,7 @@ describe('rootThread', function () {
       rootThread.thread(fakeAnnotationUI.state);
       var threadFilterFn = fakeBuildThread.args[0][1].threadFilterFn;
 
-      assert.equal(threadFilterFn(fakeBuildThread), true);
+      assert.isTrue(threadFilterFn(fakeBuildThread));
     });
 
     it('generates a thread filter function for annotations, when all annotations are of type notes', function () {
@@ -222,7 +222,7 @@ describe('rootThread', function () {
       rootThread.thread(fakeAnnotationUI.state);
       var threadFilterFn = fakeBuildThread.args[0][1].threadFilterFn;
 
-      assert.equal(threadFilterFn(fakeBuildThread), undefined);
+      assert.isFalse(threadFilterFn(fakeBuildThread));
     });
   });
 
@@ -238,7 +238,7 @@ describe('rootThread', function () {
       var filterFn = fakeBuildThread.args[0][1].filterFn;
 
       fakeViewFilter.filter.returns([annotation]);
-      assert.equal(filterFn(annotation), true);
+      assert.isTrue(filterFn(annotation));
       assert.calledWith(fakeViewFilter.filter, sinon.match([annotation]),
         filters);
     });

h/static/scripts/widget-controller.js

@@ -95,9 +95,10 @@ module.exports = function WidgetController(
   var visibleThreads = new VirtualThreadList($scope, window, thread());
   annotationUI.subscribe(function () {
     visibleThreads.setRootThread(thread());
+    $scope.selectedTab = annotationUI.getState().selectedTab;
+
     $scope.totalAnnotations = countAnnotations(annotationUI.getState().annotations);
     $scope.totalNotes = countNotes(annotationUI.getState().annotations);
-    $scope.selectedTab = annotationUI.getState().selectedTab;
   });
 
   visibleThreads.on('changed', function (state) {
@@ -381,9 +382,7 @@ module.exports = function WidgetController(
   };
 
   $scope.isLoading = isLoading;
-  $scope.tabAnnotations = uiConstants.TAB_ANNOTATIONS;
-  $scope.tabNotes = uiConstants.TAB_NOTES;
-  $scope.selectionTabsFlagEnabled = features.flagEnabled('selection_tabs');
+  annotationUI.selectTab(uiConstants.TAB_ANNOTATIONS);
 
   var visibleCount = memoize(function (thread) {
     return thread.children.reduce(function (count, child) {

h/static/styles/app.scss

@@ -38,7 +38,7 @@ body {
 
   @include grey-background;
   font-family: $sans-font-family;
-  font-weight: 300;
+  font-weight: normal;
   padding: $sidebar-h-padding;
   padding-top: $sidebar-h-padding + $top-bar-height;
   -webkit-overflow-scrolling: touch;

h/static/styles/selection-tabs.scss

 .selection-tabs {
   display: flex;
   flex-direction: row;
-  color: $grey-4;
+  color: $grey-5;
   @include font-normal;
-  font-weight: bold;
-  margin: 10px 0px;
+  margin: 0px 0px 10px 0px;
+
+  &:hover {
+    color: $grey-6;
+  }
 }
 
 .selection-tabs--selected {
   color: $grey-6;
+  font-weight: bold;
 }
 
 .selection-tabs__type {
   margin-right: 20px;
   cursor: pointer;
+  min-width: 85px;
+  min-height: 18px;
+}
+
+.selection-tabs__count {
+  position: relative;
+  bottom: 3px;
+  font-size: 10px;
+}
+
+.selection-loading {
+  text-align: center;
 }
\ No newline at end of file

h/templates/client/search_status_bar.html

-<div class="search-status-bar" ng-if="filterActive">
+<div class="search-status-bar" ng-if="vm.filterActive">
   <button class="primary-action-btn primary-action-btn--short"
-          ng-click="onClearSelection()"
+          ng-click="vm.onClearSelection()"
           title="Clear the search filter and show all annotations"
   >
     <i class="primary-action-btn__icon h-icon-close"></i> Clear search
   </button>
   <span ng-pluralize
-           count="filterMatchCount"
-           when="{'0': 'No results for “{{searchQuery}}”',
+           count="vm.filterMatchCount"
+           when="{'0': 'No results for “{{vm.searchQuery}}”',
                   'one': '1 search result',
                   'other': '{} search results'}"></span>
 </div>
-<div class="search-status-bar" ng-if="!filterActive && selectionCount > 0">
+<div class="search-status-bar" ng-if="!vm.filterActive && vm.selectionCount > 0">
   <button class="primary-action-btn primary-action-btn--short"
-          ng-click="onClearSelection()"
+          ng-click="vm.onClearSelection()"
           title="Clear the selection and show all annotations">
-    <span ng-if="!selectedTab">
+    <span ng-if="!vm.selectedTab">
       Show all annotations and notes
     </span>
-    <span ng-if="selectedTab === tabAnnotations">
+    <span ng-if="vm.selectedTab === vm.TAB_ANNOTATIONS">
       Show all annotations
-      <span ng-if="totalAnnotations > 1">
-        ({{totalAnnotations}})
+      <span ng-if="vm.totalAnnotations > 1">
+        ({{vm.totalAnnotations}})
       </span>
     </span>
-    <span ng-if="selectedTab === tabNotes">
+    <span ng-if="vm.selectedTab === vm.TAB_NOTES">
       Show all notes
-      <span ng-if="totalNotes > 1">
-        ({{totalNotes}})
+      <span ng-if="vm.totalNotes > 1">
+        ({{vm.totalNotes}})
       </span>
     </span>
   </button>

h/templates/client/selection_tabs.html

 <!-- Tabbed display of annotations and notes. -->
 <ul class="selection-tabs">
-  <li class="selection-tabs__type" ng-class="{'selection-tabs--selected': vm.selectedTab === vm.tabAnnotations}" ng-click="vm.selectTab('annotation')">
+  <li class="selection-tabs__type" ng-class="{'selection-tabs--selected': vm.selectedTab === vm.TAB_ANNOTATIONS}" ng-click="vm.selectTab('annotation')">
     Annotations
-    <sup ng-if="vm.totalAnnotations > 0">{{ vm.totalAnnotations }}</sup>
+    <span class="selection-tabs__count" ng-if="vm.totalAnnotations > 0">{{ vm.totalAnnotations }}</sup>
   </li>
-  <li class="selection-tabs__type" ng-class="{'selection-tabs--selected': vm.selectedTab === vm.tabNotes}" ng-click="vm.selectTab('note')">
+  <li class="selection-tabs__type" ng-class="{'selection-tabs--selected': vm.selectedTab === vm.TAB_NOTES}" ng-click="vm.selectTab('note')">
     Notes
-    <sup ng-if="vm.totalNotes > 0">{{ vm.totalNotes }}</sup>
+    <span class="selection-tabs__count" ng-if="vm.totalNotes > 0">{{ vm.totalNotes }}</sup>
   </li>
 </ul>
+<div ng-if="!vm.isLoading()">
+  <div ng-if="vm.selectedTab === vm.TAB_NOTES && vm.totalNotes === 0"  class="annotation-unavailable-message">
+    <p class="annotation-unavailable-message__label">
+      There are no page notes in this group.
+      <br />
+      Create one by clicking the
+      <i class="help-icon h-icon-note"></i>
+      button.
+    </p>
+  </div>
+  <div ng-if="vm.selectedTab === vm.TAB_ANNOTATIONS && vm.totalAnnotations === 0"  class="annotation-unavailable-message">
+    <p class="annotation-unavailable-message__label">
+      There are no annotations in this group.
+    </p>
+  </div>
+</div>

h/templates/client/viewer.html

-<!-- Annotation thread view
-
-(See gh2642 for rationale for 'ng-show="true"')
- -->
-<selection-tabs ng-if="selectionTabsFlagEnabled && !search.query() && selectedAnnotationCount() <= 0"
+<selection-tabs ng-if="feature('selection_tabs') && !search.query() && selectedAnnotationCount() === 0"
+  is-loading="isLoading"
   selected-tab="selectedTab"
   total-annotations="totalAnnotations"
-  total-notes="totalNotes"
-  tab-annotations="tabAnnotations"
-  tab-notes="tabNotes">
+  total-notes="totalNotes">
 </selection-tabs>
 
+<!-- Annotation thread view
+
+(See gh2642 for rationale for 'ng-show="true"')
+ -->
 <ul class="thread-list ng-hide"
     ng-show="true"
     window-scroll="loadMore(20)">
@@ -23,8 +22,6 @@
     selection-count="selectedAnnotationCount()"
     total-count="topLevelThreadCount()"
     selected-tab="selectedTab"
-    tab-annotations="tabAnnotations"
-    tab-notes="tabNotes"
     total-annotations="totalAnnotations"
     total-notes="totalNotes">
   </search-status-bar>