Merge pull request #3489 from hypothesis/t375-auto-expand-direct-linked-annots

Auto-expand replies for direct-linked annotations

h/static/scripts/annotation-ui.js

@@ -44,7 +44,7 @@ function initialState(settings) {
     // present in the map, the default state is used which depends on whether
     // the annotation is a top-level annotation or a reply, whether it is
     // selected and whether it matches the current filter.
-    expanded: {},
+    expanded: initialSelection(settings) || {},
 
     // Set of IDs of annotations that have been explicitly shown
     // by the user even if they do not match the current search filter
@@ -63,6 +63,7 @@ function initialState(settings) {
 }
 
 var types = {
+  CLEAR_SELECTION: 'CLEAR_SELECTION',
   SELECT_ANNOTATIONS: 'SELECT_ANNOTATIONS',
   FOCUS_ANNOTATIONS: 'FOCUS_ANNOTATIONS',
   HIGHLIGHT_ANNOTATIONS: 'HIGHLIGHT_ANNOTATIONS',
@@ -107,6 +108,11 @@ function reducer(state, action) {
   state = annotationsReducer(state, action);
 
   switch (action.type) {
+    case types.CLEAR_SELECTION:
+      return Object.assign({}, state, {
+        filterQuery: null,
+        selectedAnnotationMap: null,
+      });
     case types.SELECT_ANNOTATIONS:
       return Object.assign({}, state, {selectedAnnotationMap: action.selection});
     case types.FOCUS_ANNOTATIONS:
@@ -278,7 +284,7 @@ module.exports = function (settings) {
 
     /** De-select all annotations. */
     clearSelectedAnnotations: function () {
-      select({});
+      store.dispatch({type: 'CLEAR_SELECTION'});
     },
 
     /** Add annotations to the currently displayed set. */

h/static/scripts/app-controller.js

 'use strict';
 
-var angular = require('angular');
 var scrollIntoView = require('scroll-into-view');
 
 var events = require('./events');
@@ -131,20 +130,15 @@ module.exports = function AppController(
   };
 
   $scope.clearSelection = function () {
-    $scope.search.query = '';
     annotationUI.clearSelectedAnnotations();
   };
 
   $scope.search = {
-    query: $location.search().q,
-    clear: function () {
-      $location.search('q', null);
+    query: function () {
+      return annotationUI.getState().filterQuery;
     },
     update: function (query) {
-      if (!angular.equals($location.search().q, query)) {
-        $location.search('q', query || null);
-        annotationUI.clearSelectedAnnotations();
-      }
-    }
+      annotationUI.setFilterQuery(query);
+    },
   };
 };

h/static/scripts/app.js

@@ -148,7 +148,7 @@ module.exports = angular.module('h', [
   .directive('shareDialog', require('./directive/share-dialog'))
   .directive('sidebarTutorial', require('./directive/sidebar-tutorial').directive)
   .directive('signinControl', require('./directive/signin-control'))
-  .directive('simpleSearch', require('./directive/simple-search'))
+  .directive('searchInput', require('./directive/search-input'))
   .directive('sortDropdown', require('./directive/sort-dropdown'))
   .directive('spinner', require('./directive/spinner'))
   .directive('statusButton', require('./directive/status-button'))

h/static/scripts/directive/search-input.js

+'use strict';
+
+// @ngInject
+function SearchInputController($element, $http, $scope) {
+  var self = this;
+  var button = $element.find('button');
+  var input = $element.find('input')[0];
+  var form = $element.find('form')[0];
+
+  button.on('click', function () {
+    input.focus();
+  });
+
+  $scope.$watch(
+    function () { return $http.pendingRequests.length; },
+    function (count) { self.loading = count > 0; }
+  );
+
+  form.onsubmit = function (e) {
+    e.preventDefault();
+    self.onSearch({$query: input.value});
+  };
+
+  this.inputClasses = function () {
+    return {'is-expanded': self.alwaysExpanded || self.query};
+  };
+
+  this.$onChanges = function (changes) {
+    if (changes.query) {
+      input.value = changes.query.currentValue;
+    }
+  };
+}
+
+// @ngInject
+module.exports = function () {
+  return {
+    bindToController: true,
+    controller: SearchInputController,
+    controllerAs: 'vm',
+    restrict: 'E',
+    scope: {
+      // Specifies whether the search input field should always be expanded,
+      // regardless of whether the it is focused or has an active query.
+      //
+      // If false, it is only expanded when focused or when 'query' is non-empty
+      alwaysExpanded: '<',
+      query: '<',
+      onSearch: '&',
+    },
+    template: require('../../../templates/client/search_input.html'),
+  };
+};

h/static/scripts/directive/simple-search.coffee

-module.exports = ['$http', '$parse', ($http, $parse) ->
-  link: (scope, elem, attr, ctrl) ->
-    button = elem.find('button')
-    input = elem.find('input')
-
-    button.on('click', -> input[0].focus())
-
-    scope.reset = (event) ->
-      event.preventDefault()
-      scope.query = ''
-      scope.searchtext = ''
-
-    scope.search = (event) ->
-      event.preventDefault()
-      scope.query = scope.searchtext
-
-    scope.$watch (-> $http.pendingRequests.length), (pending) ->
-      scope.loading = (pending > 0)
-
-    scope.$watch 'query', (query) ->
-      return if query is undefined
-      scope.searchtext = query
-      if query
-        scope.onSearch?(query: scope.searchtext)
-      else
-        scope.onClear?()
-
-  restrict: 'E'
-  scope:
-    # Specifies whether the search input field should always be expanded,
-    # regardless of whether the it is focused or has an active query.
-    #
-    # If false, it is only expanded when focused or when 'query' is non-empty
-    alwaysExpanded: '<'
-    query: '='
-    onSearch: '&'
-    onClear: '&'
-  template: '''
-            <form class="simple-search-form" ng-class="!searchtext && 'simple-search-inactive'" name="searchBox" ng-submit="search($event)">
-              <input class="simple-search-input" type="text" ng-model="searchtext" name="searchText"
-                     placeholder="{{loading && 'Loading' || 'Search'}}…"
-                     ng-disabled="loading"
-                     ng-class="(alwaysExpanded || searchtext.length > 0) ? 'is-expanded' : ''"/>
-              <button type="button" class="simple-search-icon top-bar__btn" ng-hide="loading">
-                <i class="h-icon-search"></i>
-              </button>
-              <button type="button" class="simple-search-icon btn btn-clean" ng-show="loading" disabled>
-                <span class="btn-icon"><span class="spinner"></span></span>
-              </button>
-            </form>
-            '''
-]

h/static/scripts/directive/test/search-input-test.js

+'use strict';
+
+var angular = require('angular');
+
+var util = require('./util');
+
+describe('searchInput', function () {
+  var fakeHttp;
+
+  before(function () {
+    angular.module('app', [])
+      .directive('searchInput', require('../search-input'));
+  });
+
+  beforeEach(function () {
+    fakeHttp = {pendingRequests: []};
+    angular.mock.module('app', {
+      $http: fakeHttp,
+    });
+  });
+
+  it('displays the search query', function () {
+    var el = util.createDirective(document, 'searchInput', {
+      query: 'foo',
+    });
+    var input = el.find('input')[0];
+    assert.equal(input.value, 'foo');
+  });
+
+  it('invokes #onSearch() when the query changes', function () {
+    var onSearch = sinon.stub();
+    var el = util.createDirective(document, 'searchInput', {
+      query: 'foo',
+      onSearch: {
+        args: ['$query'],
+        callback: onSearch,
+      },
+    });
+    var input = el.find('input')[0];
+    var form = el.find('form');
+    input.value = 'new-query';
+    form.submit();
+    assert.calledWith(onSearch, 'new-query');
+  });
+
+  describe('loading indicator', function () {
+    it('is hidden when there are no network requests in flight', function () {
+      var el = util.createDirective(document, 'search-input', {});
+      var spinner = el[0].querySelector('.spinner');
+      assert.equal(util.isHidden(spinner), true);
+    });
+
+    it('is visible when there are network requests in flight', function () {
+      var el = util.createDirective(document, 'search-input', {});
+      var spinner = el[0].querySelector('.spinner');
+      fakeHttp.pendingRequests.push([{}]);
+      el.scope.$digest();
+      assert.equal(util.isHidden(spinner), false);
+    });
+  });
+});

h/static/scripts/directive/test/simple-search-test.coffee

-{module, inject} = angular.mock
-
-describe 'simple-search', ->
-  $compile = null
-  $element = null
-  $scope = null
-  fakeHttp = null
-  fakeWindow = null
-  isolate = null
-
-  before ->
-    angular.module('h', [])
-    .directive('simpleSearch', require('../simple-search'))
-
-  beforeEach module('h')
-
-  beforeEach module ($provide) ->
-    fakeHttp = {pendingRequests: []}
-    $provide.service('$http', -> fakeHttp)
-    return
-
-  beforeEach inject (_$compile_, _$rootScope_) ->
-    $compile = _$compile_
-    $scope = _$rootScope_.$new()
-
-    $scope.update = sinon.spy()
-    $scope.clear = sinon.spy()
-
-    template= '''
-    <simple-search
-          query="query"
-          on-search="update(query)"
-          on-clear="clear()">
-    </simple-search>
-    '''
-
-    $element = $compile(angular.element(template))($scope)
-
-    # add element to document so that it becomes focusable
-    # and we get default form behaviors
-    document.body.appendChild($element[0])
-
-    $scope.$digest()
-    isolate = $element.isolateScope()
-
-  afterEach ->
-    document.body.removeChild($element[0])
-
-  it 'updates the search-bar', ->
-    $scope.query = "Test query"
-    $scope.$digest()
-    assert.equal(isolate.searchtext, $scope.query)
-
-  it 'calls the given search function', ->
-    isolate.searchtext = "Test query"
-    isolate.$digest()
-    $element.find('form').triggerHandler('submit')
-    assert.calledWith($scope.update, "Test query")
-
-  it 'invokes callbacks when the input model changes', ->
-    $scope.query = "Test query"
-    $scope.$digest()
-    assert.calledOnce($scope.update)
-    $scope.query = ""
-    $scope.$digest()
-    assert.calledOnce($scope.clear)
-
-  it 'adds a class to the form when there is no input value', ->
-    $form = $element.find('.simple-search-form')
-    assert.include($form.prop('className'), 'simple-search-inactive')
-
-  it 'removes the class from the form when there is an input value', ->
-    $scope.query = "Test query"
-    $scope.$digest()
-
-    $form = $element.find('.simple-search-form')
-    assert.notInclude($form.prop('className'), 'simple-search-inactive')
-
-  it 'sets the `loading` scope key when http requests are in progress', ->
-    fakeHttp.pendingRequests = []
-    isolate.$digest()
-    assert.isFalse(isolate.loading)
-    fakeHttp.pendingRequests = ['bogus']
-    isolate.$digest()
-    assert.isTrue(isolate.loading)
-
-  it 'expands the search field when the input is non-empty', ->
-    input = $element.find('.simple-search-input')
-    assert.isFalse(input.hasClass('is-expanded'))
-    input.val('query')
-    input.trigger('change')
-    isolate.$digest()
-    assert.isTrue(input.hasClass('is-expanded'))
-
-  it 'focuses the search field when clicking the search button', ->
-    input = $element.find('.simple-search-input')
-    searchBtn = $element.find('button')
-    assert.ok(document.activeElement != input[0])
-    searchBtn.click()
-    assert.ok(document.activeElement == input[0])
-
-  it 'does not update the search when clicking the search button', ->
-    searchBtn = $element.find('button')
-    input = $element.find('.simple-search-input')
-    input.val('query')
-    input.trigger('change')
-    searchBtn.click()
-    assert.notCalled($scope.update)

h/static/scripts/directive/test/util.js

@@ -172,8 +172,34 @@ function sendEvent(element, eventType) {
   element.dispatchEvent(event);
 }
 
+/**
+ * Return true if a given element is hidden on the page.
+ *
+ * There are many possible ways of hiding DOM elements on a page, this just
+ * looks for approaches that are common in our app.
+ */
+function isHidden(element) {
+  var style = window.getComputedStyle(element);
+
+  if (style.display === 'none') {
+    return true;
+  }
+
+  // Test for element or ancestor being hidden with `ng-hide` directive
+  var el = element;
+  while (el) {
+    if (el.classList.contains('ng-hide')) {
+      return true;
+    }
+    el = el.parentElement;
+  }
+
+  return false;
+}
+
 module.exports = {
   createDirective: createDirective,
+  isHidden: isHidden,
   ngModule: ngModule,
   sendEvent: sendEvent,
 };

h/static/scripts/root-thread.js

@@ -49,15 +49,9 @@ function RootThread($rootScope, annotationUI, searchFilter, viewFilter) {
   function buildRootThread(state) {
     var sortFn = sortFns[state.sortKey];
 
-    var filters;
-    var filterQuery = state.filterQuery;
-
-    if (filterQuery) {
-      filters = searchFilter.generateFacetedFilter(filterQuery);
-    }
-
     var filterFn;
-    if (filterQuery) {
+    if (state.filterQuery) {
+      var filters = searchFilter.generateFacetedFilter(state.filterQuery);
       filterFn = function (annot) {
         return viewFilter.filter([annot], filters).length > 0;
       };

h/static/scripts/stream-controller.coffee

@@ -2,13 +2,13 @@ angular = require('angular')
 
 module.exports = class StreamController
   this.$inject = [
-    '$scope', '$route', '$rootScope', '$routeParams',
+    '$scope', '$location', '$route', '$rootScope', '$routeParams',
     'annotationUI',
     'queryParser', 'rootThread', 'searchFilter', 'store',
     'streamer', 'streamFilter', 'annotationMapper'
   ]
   constructor: (
-     $scope,   $route,   $rootScope,   $routeParams
+     $scope,  $location,   $route,   $rootScope,   $routeParams
      annotationUI,
      queryParser,   rootThread,   searchFilter,   store,
      streamer,   streamFilter,   annotationMapper
@@ -52,6 +52,11 @@ module.exports = class StreamController
     $scope.forceVisible = (id) ->
       annotationUI.setForceVisible(id, true)
 
+    Object.assign $scope.search, {
+      query: -> $routeParams.q || ''
+      update: (q) -> $location.search({q: q})
+    }
+
     thread = ->
       rootThread.thread(annotationUI.getState())
 

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

@@ -16,6 +16,7 @@ describe('annotationUI', function () {
   describe('initialization', function () {
     it('does not set a selection when settings.annotations is null', function () {
       assert.isFalse(annotationUI.hasSelectedAnnotations());
+      assert.equal(Object.keys(annotationUI.getState().expanded).length, 0);
     });
 
     it('sets the selection when settings.annotations is set', function () {
@@ -24,6 +25,13 @@ describe('annotationUI', function () {
         testid: true,
       });
     });
+
+    it('expands the selected annotations when settings.annotations is set', function () {
+      annotationUI = annotationUIFactory({annotations: 'testid'});
+      assert.deepEqual(annotationUI.getState().expanded, {
+        testid: true,
+      });
+    });
   });
 
   describe('#addAnnotations()', function () {
@@ -202,6 +210,12 @@ describe('annotationUI', function () {
       annotationUI.clearSelectedAnnotations();
       assert.isNull(annotationUI.getState().selectedAnnotationMap);
     });
+
+    it('clears the current search query', function () {
+      annotationUI.setFilterQuery('foo');
+      annotationUI.clearSelectedAnnotations();
+      assert.isNull(annotationUI.getState().filterQuery);
+    });
   });
 
   describe('#setFilterQuery()', function () {

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

@@ -96,6 +96,7 @@ describe 'StreamController', ->
   beforeEach inject (_$controller_, $rootScope) ->
     $controller = _$controller_
     $scope = $rootScope.$new()
+    $scope.search = {}
 
   afterEach ->
     sandbox.restore()

h/static/scripts/widget-controller.js

@@ -264,12 +264,6 @@ module.exports = function WidgetController(
     return crossframe.frames;
   }, loadAnnotations);
 
-  // Watch the inputs that determine which annotations are currently
-  // visible and how they are sorted and rebuild the thread when they change
-  $scope.$watch('search.query', function (query) {
-    annotationUI.setFilterQuery(query);
-  });
-
   $scope.setCollapsed = function (id, collapsed) {
     annotationUI.setCollapsed(id, collapsed);
   };

h/templates/client/search_input.html

+<form class="simple-search-form"
+      name="searchForm"
+      ng-class="!vm.query && 'simple-search-inactive'">
+  <input class="simple-search-input"
+         type="text"
+         name="query"
+         placeholder="{{vm.loading && 'Loading' || 'Search'}}…"
+         ng-disabled="vm.loading"
+         ng-class="vm.inputClasses()"/>
+  <button type="button" class="simple-search-icon top-bar__btn" ng-hide="vm.loading">
+    <i class="h-icon-search"></i>
+  </button>
+  <button type="button" class="simple-search-icon btn btn-clean" ng-show="vm.loading" disabled>
+    <span class="btn-icon"><span class="spinner"></span></span>
+  </button>
+</form>

h/templates/client/top_bar.html

@@ -3,13 +3,12 @@
 <div class="top-bar" ng-class="frame.visible && 'shown'" ng-cloak>
   <!-- Legacy design for top bar, as used in the stream !-->
   <div class="top-bar__inner content" ng-if="::!isSidebar">
-    <simple-search
-      class="simple-search"
-      query="searchController.query"
-      on-search="searchController.update(query)"
-      on-clear="searchController.clear()"
+    <search-input
+      class="search-input"
+      query="searchController.query()"
+      on-search="searchController.update($query)"
       always-expanded="true">
-    </simple-search>
+    </search-input>
     <div class="top-bar__expander"></div>
     <signin-control
       auth="auth"
@@ -27,13 +26,12 @@
   <div class="top-bar__inner content" ng-if="::isSidebar">
     <group-list class="group-list" auth="auth"></group-list>
     <div class="top-bar__expander"></div>
-    <simple-search
-      class="simple-search"
-      query="searchController.query"
-      on-search="searchController.update(query)"
-      on-clear="searchController.clear()"
+    <search-input
+      class="search-input"
+      query="searchController.query()"
+      on-search="searchController.update($query)"
       title="Filter the annotation list">
-    </simple-search>
+    </search-input>
     <sort-dropdown
       sort-keys-available="sortKeysAvailable"
       sort-key="sortKey"

h/templates/client/viewer.html

@@ -8,7 +8,7 @@
   <search-status-bar
     ng-show="!isLoading()"
     ng-if="!isStream"
-    filter-active="search.query"
+    filter-active="!!search.query()"
     filter-match-count="visibleCount()"
     on-clear-selection="clearSelection()"
     search-query="search ? search.query : ''"