Merge pull request #2042 from hypothesis/lookahead-tags-3

Autocomplete feature for tags

h/static/scripts/app.coffee

@@ -126,6 +126,7 @@ require('./auth-service')
 require('./cross-frame-service')
 require('./flash-service')
 require('./permissions-service')
+require('./local-storage-service')
 require('./store-service')
 require('./threading-service')
 

h/static/scripts/directives/annotation.coffee

@@ -29,12 +29,13 @@ validate = (value) ->
 # {@link annotationMapper AnnotationMapper service} for persistence.
 ###
 AnnotationController = [
-  '$scope', '$timeout', '$rootScope', '$document',
-  'auth', 'drafts', 'flash', 'permissions',
+  '$scope', '$timeout', '$q', '$rootScope', '$document',
+  'auth', 'drafts', 'flash', 'permissions', 'tagHelpers',
   'timeHelpers', 'annotationUI', 'annotationMapper'
-  ($scope,   $timeout,   $rootScope,   $document,
-   auth,   drafts,   flash,   permissions,
+  ($scope,   $timeout,   $q,   $rootScope,   $document,
+   auth,   drafts,   flash,   permissions,   tagHelpers,
    timeHelpers, annotationUI, annotationMapper) ->
+
     @annotation = {}
     @action = 'view'
     @document = null
@@ -50,6 +51,15 @@ AnnotationController = [
     original = null
     vm = this
 
+    ###*
+    # @ngdoc method
+    # @name annotation.AnnotationController#tagsAutoComplete.
+    # @returns {Promise} immediately resolved to {string[]} -
+    # the tags to show in autocomplete.
+    ###
+    this.tagsAutoComplete = (query) ->
+      $q.when(tagHelpers.filterTags(query))
+
     ###*
     # @ngdoc method
     # @name annotation.AnnotationController#isComment.
@@ -145,6 +155,11 @@ AnnotationController = [
       unless validate(@annotation)
         return flash 'info', 'Please add text or a tag before publishing.'
 
+      # Update stored tags with the new tags of this annotation
+      tags = @annotation.tags.filter (tag) ->
+        tag.text not in (model.tags or [])
+      tagHelpers.storeTags(tags)
+
       angular.extend model, @annotation,
         tags: (tag.text for tag in @annotation.tags)
 

h/static/scripts/directives/privacy.coffee

-privacy = ['$window', 'permissions', ($window, permissions) ->
+privacy = ['localstorage', 'permissions', (localstorage, permissions) ->
   VISIBILITY_KEY ='hypothesis.visibility'
   VISIBILITY_PUBLIC = 'public'
   VISIBILITY_PRIVATE = 'private'
@@ -16,24 +16,6 @@ privacy = ['$window', 'permissions', ($window, permissions) ->
 
   isPublic  = (level) -> level == VISIBILITY_PUBLIC
 
-  # Detection is needed because we run often as a third party widget and
-  # third party storage blocking often blocks cookies and local storage
-  # https://github.com/Modernizr/Modernizr/blob/master/feature-detects/storage/localstorage.js
-  storage = do ->
-    key = 'hypothesis.testKey'
-    try
-      $window.localStorage.setItem  key, key
-      $window.localStorage.removeItem key
-      $window.localStorage
-    catch
-      memoryStorage = {}
-      getItem: (key) ->
-        if key of memoryStorage then memoryStorage[key] else null
-      setItem: (key, value) ->
-        memoryStorage[key] = value
-      removeItem: (key) ->
-        delete memoryStorage[key]
-
   link: (scope, elem, attrs, controller) ->
     return unless controller?
 
@@ -62,7 +44,7 @@ privacy = ['$window', 'permissions', ($window, permissions) ->
 
     controller.$render = ->
       unless controller.$modelValue.read?.length
-        name = storage.getItem VISIBILITY_KEY
+        name = localstorage.getItem VISIBILITY_KEY
         name ?= VISIBILITY_PUBLIC
         level = getLevel(name)
         controller.$setViewValue level
@@ -71,7 +53,7 @@ privacy = ['$window', 'permissions', ($window, permissions) ->
 
     scope.levels = levels
     scope.setLevel = (level) ->
-      storage.setItem VISIBILITY_KEY, level.name
+      localstorage.setItem VISIBILITY_KEY, level.name
       controller.$setViewValue level
       controller.$render()
     scope.isPublic = isPublic

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

@@ -19,6 +19,7 @@ describe 'h.directives.annotation', ->
   fakePermissions = null
   fakePersonaFilter = null
   fakeStore = null
+  fakeTagHelpers = null
   fakeTimeHelpers = null
   fakeUrlEncodeFilter = null
   sandbox = null
@@ -48,6 +49,7 @@ describe 'h.directives.annotation', ->
       remove: sandbox.stub()
     }
     fakeFlash = sandbox.stub()
+
     fakeMomentFilter = sandbox.stub().returns('ages ago')
     fakePermissions = {
       isPublic: sandbox.stub().returns(true)
@@ -57,6 +59,10 @@ describe 'h.directives.annotation', ->
       private: sandbox.stub().returns({read: ['justme']})
     }
     fakePersonaFilter = sandbox.stub().returnsArg(0)
+    fakeTagsHelpers = {
+      filterTags: sandbox.stub().returns('a while ago')
+      refreshTags: sandbox.stub().returns(30)
+    }
     fakeTimeHelpers = {
       toFuzzyString: sandbox.stub().returns('a while ago')
       nextFuzzyUpdate: sandbox.stub().returns(30)
@@ -72,6 +78,7 @@ describe 'h.directives.annotation', ->
     $provide.value 'permissions', fakePermissions
     $provide.value 'personaFilter', fakePersonaFilter
     $provide.value 'store', fakeStore
+    $provide.value 'tagHelpers', fakeTagHelpers
     $provide.value 'timeHelpers', fakeTimeHelpers
     $provide.value 'urlencodeFilter', fakeUrlEncodeFilter
     return

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

@@ -12,6 +12,7 @@ describe 'h.directives.privacy', ->
   $window = null
   fakeAuth = null
   fakePermissions = null
+  fakeLocalStorage = null
   sandbox = null
 
   before ->
@@ -28,6 +29,13 @@ describe 'h.directives.privacy', ->
       user: 'acct:angry.joe@texas.com'
     }
 
+    storage = {}
+    fakeLocalStorage = {
+      getItem: sandbox.spy (key) -> storage[key]
+      setItem: sandbox.spy (key, value) -> storage[key] = value
+      removeItem: sandbox.spy (key) -> delete storage[key]
+    }
+
     fakePermissions = {
       isPublic: sandbox.stub().returns(true)
       isPrivate: sandbox.stub().returns(false)
@@ -37,6 +45,7 @@ describe 'h.directives.privacy', ->
     }
 
     $provide.value 'auth', fakeAuth
+    $provide.value 'localstorage', fakeLocalStorage
     $provide.value 'permissions', fakePermissions
     return
 
@@ -48,31 +57,7 @@ describe 'h.directives.privacy', ->
   afterEach ->
     sandbox.restore()
 
-  describe 'memory fallback', ->
-    $scope2 = null
-
-    beforeEach inject (_$rootScope_) ->
-      $scope2 = _$rootScope_.$new()
-
-      $window.localStorage = null
-
-    it 'stores the default visibility level when it changes', ->
-      $scope.permissions = {read: ['acct:user@example.com']}
-      $element = $compile('<privacy ng-model="permissions">')($scope)
-      $scope.$digest()
-      $isolateScope = $element.isolateScope()
-      $isolateScope.setLevel(name: VISIBILITY_PUBLIC)
-
-      $scope2.permissions = {read: []}
-      $element = $compile('<privacy ng-model="permissions">')($scope2)
-      $scope2.$digest()
-
-      # Roundabout way: the storage works because the directive
-      # could read out the privacy level
-      readPermissions = $scope2.permissions.read[0]
-      assert.equal readPermissions, 'everybody'
-
-  describe 'has localStorage', ->
+  describe 'saves visibility level', ->
 
     it 'stores the default visibility level when it changes', ->
       $scope.permissions = {read: ['acct:user@example.com']}
@@ -82,19 +67,15 @@ describe 'h.directives.privacy', ->
       $isolateScope.setLevel(name: VISIBILITY_PUBLIC)
 
       expected = VISIBILITY_PUBLIC
-      stored = $window.localStorage.getItem VISIBILITY_KEY
+      stored = fakeLocalStorage.getItem VISIBILITY_KEY
       assert.equal stored, expected
 
     describe 'setting permissions', ->
       $element = null
-      store = null
-
-      beforeEach ->
-        store = $window.localStorage
 
       describe 'when no setting is stored', ->
         beforeEach ->
-          store.removeItem VISIBILITY_KEY
+          fakeLocalStorage.removeItem VISIBILITY_KEY
 
         it 'defaults to public', ->
           $scope.permissions = {read: []}
@@ -105,7 +86,7 @@ describe 'h.directives.privacy', ->
 
       describe 'when permissions.read is empty', ->
         beforeEach ->
-          store.setItem VISIBILITY_KEY, VISIBILITY_PUBLIC
+          fakeLocalStorage.setItem VISIBILITY_KEY, VISIBILITY_PUBLIC
 
           $scope.permissions = {read: []}
           $element = $compile('<privacy ng-model="permissions">')($scope)
@@ -115,14 +96,14 @@ describe 'h.directives.privacy', ->
           assert.equal $element.isolateScope().level.name, VISIBILITY_PUBLIC
 
         it 'does not alter the level on subsequent renderings', ->
-          store.setItem VISIBILITY_KEY, VISIBILITY_PRIVATE
+          fakeLocalStorage.setItem VISIBILITY_KEY, VISIBILITY_PRIVATE
           $scope.permissions.read = ['acct:user@example.com']
           $scope.$digest()
           assert.equal $element.isolateScope().level.name, VISIBILITY_PUBLIC
 
       describe 'when permissions.read is filled', ->
         it 'does not alter the level', ->
-          store.setItem VISIBILITY_KEY, VISIBILITY_PRIVATE
+          fakeLocalStorage.setItem VISIBILITY_KEY, VISIBILITY_PRIVATE
 
           $scope.permissions = {read: ['group:__world__']}
           $element = $compile('<privacy ng-model="permissions">')($scope)
@@ -135,14 +116,14 @@ describe 'h.directives.privacy', ->
           $scope.permissions = {read: []}
 
         it 'fills the permissions fields with the auth.user name', ->
-          store.setItem VISIBILITY_KEY, VISIBILITY_PRIVATE
+          fakeLocalStorage.setItem VISIBILITY_KEY, VISIBILITY_PRIVATE
           $element = $compile('<privacy ng-model="permissions">')($scope)
           $scope.$digest()
 
           assert.deepEqual $scope.permissions, fakePermissions.private()
 
         it 'puts group_world into the read permissions for public visibility', ->
-          store.setItem VISIBILITY_KEY, VISIBILITY_PUBLIC
+          fakeLocalStorage.setItem VISIBILITY_KEY, VISIBILITY_PUBLIC
           $element = $compile('<privacy ng-model="permissions">')($scope)
           $scope.$digest()
 

h/static/scripts/helpers/helpers.coffee

@@ -4,6 +4,7 @@ angular.module('h.helpers', ['bootstrap'])
 
 require('./form-helpers')
 require('./string-helpers')
+require('./tag-helpers')
 require('./time-helpers')
 require('./ui-helpers')
 require('./xsrf-service')

h/static/scripts/helpers/tag-helpers.coffee

+createTagHelpers = ['localstorage', (localstorage) ->
+  TAGS_LIST_KEY = 'hypothesis.user.tags.list'
+  TAGS_MAP_KEY = 'hypothesis.user.tags.map'
+
+  filterTags: (query) ->
+    savedTags = localstorage.getObject TAGS_LIST_KEY
+    savedTags ?= []
+
+    # Only show tags having query as a substring
+    filterFn = (e) ->
+      e.toLowerCase().indexOf(query.toLowerCase()) > -1
+
+    savedTags.filter(filterFn)
+
+  # Add newly added tags from an annotation to the stored ones and refresh
+  # timestamp for every tags used.
+  storeTags: (tags) ->
+    savedTags = localstorage.getObject TAGS_MAP_KEY
+    savedTags ?= {}
+
+    for tag in tags
+      if savedTags[tag.text]?
+        # Update counter and timestamp
+        savedTags[tag.text].count += 1
+        savedTags[tag.text].updated = Date.now()
+      else
+        # Brand new tag, create an entry for it
+        savedTags[tag.text] = {
+          text: tag.text
+          count: 1
+          updated: Date.now()
+        }
+
+    localstorage.setObject TAGS_MAP_KEY, savedTags
+
+    tagsList = []
+    for tag of savedTags
+      tagsList[tagsList.length] = tag
+
+    # Now produce TAGS_LIST, ordered by (count desc, lexical asc)
+    compareFn = (t1, t2) ->
+      if savedTags[t1].count != savedTags[t2].count
+        return savedTags[t2].count - savedTags[t1].count
+      else
+        return -1 if t1 < t2
+        return 1 if t1 > t2
+        return 0
+
+    tagsList = tagsList.sort(compareFn)
+    localstorage.setObject TAGS_LIST_KEY, tagsList
+]
+
+angular.module('h.helpers')
+.service('tagHelpers', createTagHelpers)

h/static/scripts/helpers/test/tag-helpers-test.coffee

+{module, inject} = require('angular-mock')
+
+assert = chai.assert
+
+describe 'h.helpers:tag-helpers', ->
+  TAGS_LIST_KEY = 'hypothesis.user.tags.list'
+  TAGS_MAP_KEY = 'hypothesis.user.tags.map'
+
+  fakeLocalStorage = null
+  sandbox = null
+  savedTagsMap = null
+  savedTagsList = null
+  tagHelpers = null
+
+  before ->
+    angular.module('h.helpers', [])
+    require('../tag-helpers')
+
+  beforeEach module('h.helpers')
+
+  beforeEach module ($provide) ->
+    sandbox = sinon.sandbox.create()
+
+    fakeStorage = {}
+    fakeLocalStorage = {
+      getObject: sandbox.spy (key) -> fakeStorage[key]
+      setObject: sandbox.spy (key, value) -> fakeStorage[key] = value
+      wipe: -> fakeStorage = {}
+    }
+    $provide.value 'localstorage', fakeLocalStorage
+    return
+
+  beforeEach inject (_tagHelpers_) ->
+    tagHelpers = _tagHelpers_
+
+  afterEach ->
+    sandbox.restore()
+
+  beforeEach ->
+    fakeLocalStorage.wipe()
+
+    stamp = Date.now()
+    savedTagsMap =
+      foo:
+        text: 'foo'
+        count: 1
+        updated: stamp
+      bar:
+        text: 'bar'
+        count: 5
+        updated: stamp
+      future:
+        text: 'future'
+        count: 2
+        updated: stamp
+      argon:
+        text: 'argon'
+        count: 1
+        updated: stamp
+
+    savedTagsList = ['bar', 'future', 'argon', 'foo']
+
+    fakeLocalStorage.setObject TAGS_MAP_KEY, savedTagsMap
+    fakeLocalStorage.setObject TAGS_LIST_KEY, savedTagsList
+
+  describe 'filterTags()', ->
+    it 'returns tags having the query as a substring', ->
+      tags = tagHelpers.filterTags('a')
+      assert.deepEqual(tags, ['bar', 'argon'])
+
+    it 'is case insensitive', ->
+      tags = tagHelpers.filterTags('Ar')
+      assert.deepEqual(tags, ['bar', 'argon'])
+
+  describe 'storeTags()', ->
+    it 'saves new tags to storage', ->
+      tags = [{text: 'new'}]
+      tagHelpers.storeTags(tags)
+
+      storedTagsList = fakeLocalStorage.getObject TAGS_LIST_KEY
+      assert.deepEqual(storedTagsList, ['bar', 'future', 'argon', 'foo', 'new'])
+
+      storedTagsMap = fakeLocalStorage.getObject TAGS_MAP_KEY
+      assert.isTrue(storedTagsMap.new?)
+      assert.equal(storedTagsMap.new.count, 1)
+      assert.equal(storedTagsMap.new.text, 'new')
+
+    it 'increases the count for a tag already stored', ->
+      tags = [{text: 'bar'}]
+      tagHelpers.storeTags(tags)
+      storedTagsMap = fakeLocalStorage.getObject TAGS_MAP_KEY
+      assert.equal(storedTagsMap.bar.count, 6)
+
+    it 'list is ordered by count desc, lexical asc', ->
+      # Will increase from 1 to 6 (as future)
+      tags = [{text: 'foo'}]
+
+      tagHelpers.storeTags(tags)
+      tagHelpers.storeTags(tags)
+      tagHelpers.storeTags(tags)
+      tagHelpers.storeTags(tags)
+      tagHelpers.storeTags(tags)
+
+      storedTagsList = fakeLocalStorage.getObject TAGS_LIST_KEY
+      assert.deepEqual(storedTagsList, ['foo', 'bar', 'future', 'argon'])
+
+    it 'gets/sets its objects from the localstore', ->
+      tags = [{text: 'foo'}]
+      tagHelpers.storeTags(tags)
+
+      assert.called(fakeLocalStorage.getObject)
+      assert.called(fakeLocalStorage.setObject)

h/static/scripts/local-storage-service.coffee

+localstorage = ['$window', ($window) ->
+  # Detection is needed because we run often as a third party widget and
+  # third party storage blocking often blocks cookies and local storage
+  # https://github.com/Modernizr/Modernizr/blob/master/feature-detects/storage/localstorage.js
+  storage = do ->
+    key = 'hypothesis.testKey'
+    try
+      $window.localStorage.setItem  key, key
+      $window.localStorage.removeItem key
+      $window.localStorage
+    catch
+      memoryStorage = {}
+      getItem: (key) ->
+        if key of memoryStorage then memoryStorage[key] else null
+      setItem: (key, value) ->
+        memoryStorage[key] = value
+      removeItem: (key) ->
+        delete memoryStorage[key]
+
+  return {
+    getItem: (key) ->
+      storage.getItem key
+    getObject: (key) ->
+      json = storage.getItem key
+      return JSON.parse json if json
+      null
+    setItem: (key, value) ->
+      storage.setItem key, value
+    setObject: (key, value) ->
+      repr = JSON.stringify value
+      storage.setItem key, repr
+    removeItem: (key) ->
+      storage.removeItem key
+  }
+]
+
+angular.module('h')
+.service('localstorage', localstorage)

h/static/scripts/test/local-storage-service-test.coffee

+{module, inject} = require('angular-mock')
+
+assert = chai.assert
+sinon.assert.expose assert, prefix: null
+
+
+describe 'h:localstorage', ->
+  fakeWindow = null
+  sandbox = null
+
+  before ->
+    angular.module('h', [])
+    require('../local-storage-service')
+
+  beforeEach module('h')
+
+  describe 'memory fallback', ->
+    localstorage = null
+    key = null
+
+    beforeEach module ($provide) ->
+      sandbox = sinon.sandbox.create()
+      fakeWindow = {
+        localStorage: {}
+      }
+
+      $provide.value '$window', fakeWindow
+      return
+
+    afterEach ->
+      sandbox.restore()
+
+    beforeEach inject (_localstorage_) ->
+      localstorage = _localstorage_
+      key = 'test.memory.key'
+
+    it 'sets/gets Item', ->
+      value = 'What shall we do with a drunken sailor?'
+      localstorage.setItem key, value
+      actual = localstorage.getItem key
+      assert.equal value, actual
+
+    it 'removes item', ->
+      localstorage.setItem key, ''
+      localstorage.removeItem key
+      result = localstorage.getItem key
+      assert.isNull result
+
+    it 'sets/gets Object', ->
+      data = {'foo': 'bar'}
+      localstorage.setObject key, data
+      stringified = localstorage.getItem key
+      assert.equal stringified, JSON.stringify data
+
+      actual = localstorage.getObject key
+      assert.deepEqual actual, data
+
+  describe 'browser localStorage', ->
+    localstorage = null
+
+    beforeEach module ($provide) ->
+      sandbox = sinon.sandbox.create()
+      fakeWindow = {
+        localStorage: {
+          getItem: sandbox.stub()
+          setItem: sandbox.stub()
+          removeItem: sandbox.stub()
+        }
+      }
+
+      $provide.value '$window', fakeWindow
+      return
+
+    afterEach ->
+      sandbox.restore()
+
+    beforeEach inject (_localstorage_) ->
+      localstorage = _localstorage_
+
+    it 'uses window.localStorage functions to handle data', ->
+      key = 'test.storage.key'
+      data = 'test data'
+
+      localstorage.setItem key, data
+      assert.calledWith fakeWindow.localStorage.setItem, key, data

h/static/styles/tags-input.scss

@@ -3,6 +3,7 @@
 
 @import "mixins/forms";
 @import "compass/css3/user-interface";
+@import "variables";
 
 tags-input {
   .host {
@@ -101,3 +102,49 @@ tags-input {
   }
 }
 
+tags-input .autocomplete {
+  margin-top: .3em;
+  position: absolute;
+  padding: .3em 0;
+  z-index: 999;
+  width: 100%;
+  background-color: white;
+  border: thin solid rgba(0, 0, 0, 0.2);
+  -webkit-box-shadow: 0 .3em .6em rgba(0, 0, 0, 0.2);
+  -moz-box-shadow: 0 .3em .6em rgba(0, 0, 0, 0.2);
+  box-shadow: 0 .3em .6em rgba(0, 0, 0, 0.2);
+
+  .suggestion-list {
+    margin: 0;
+    padding: 0;
+    list-style-type: none;
+  }
+
+  .suggestion-item {
+    padding: .3em .6em;
+    cursor: pointer;
+    white-space: nowrap;
+    overflow: hidden;
+    text-overflow: ellipsis;
+    font-family: $sans-font-family;
+    color: black;
+    background-color: white;
+
+     em {
+       font-family: $sans-font-family;
+       font-weight: bold;
+       font-style: normal;
+       color: black;
+       background-color: white;
+    }
+
+    &.selected {
+      color: white;
+      background-color: #0097cf;
+      em {
+        color: white;
+        background-color: #0097cf;
+      }
+    }
+  }
+}

h/templates/client/annotation.html

@@ -80,7 +80,11 @@
               placeholder="Add tags…"
               min-length="1"
               replace-spaces-with-dashes="false"
-              enable-editing-last-tag="true"></tags-input>
+              enable-editing-last-tag="true">
+      <auto-complete source="vm.tagsAutoComplete($query)"
+                     min-length="1"
+                     max-results-to-show="10"></auto-complete>
+  </tags-input>
 </div>
 
 <div class="annotation-section tags tags-read-only"