Selection tabs tweaks.

Fix PR comments including some UI tweaks and bug fixes.

Hide no annotations/notes message when annotations are still loading.
When the annotations are still loading, a 0 annotations message shows for a flash of a
second before the list loads. Avoid this by only displaying the 0 state after the
annotations have finished loading.

If selected annotation/note is deleted, default selected tab to annotation.
If a user lands on a direct link for a deleted annotation, the button to take the user
back to the list of annotations/notes should by default be for annotations.
On clearning the selection, filter annotations according to the selected tab type.

Fix broken standalone page for notes.
selectedTab was set to annotations by default, even for standalone pages.
selectedTab should be undefined for a standalone page because there are no tabs to select from.
select the annotations tab by default for the sidebar alone.

Change copy for empty annotaitons/notes state.

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>