Move tag editor to separate component

As part of an effort to simplify the <annotation> directive and make
testing easier, split this out.

h/static/scripts/app.js

@@ -152,6 +152,7 @@ module.exports = angular.module('h', [
   .directive('sortDropdown', require('./directive/sort-dropdown'))
   .directive('spinner', require('./directive/spinner'))
   .directive('statusButton', require('./directive/status-button'))
+  .directive('tagEditor', require('./directive/tag-editor'))
   .directive('timestamp', require('./directive/timestamp'))
   .directive('topBar', require('./directive/top-bar'))
   .directive('windowScroll', require('./directive/window-scroll'))

h/static/scripts/directive/annotation.js

@@ -13,19 +13,6 @@ var isNew = annotationMetadata.isNew;
 var isReply = annotationMetadata.isReply;
 var extractDocumentMetadata = annotationMetadata.extractDocumentMetadata;
 
-/** Return a domainModel tags array from the given vm tags array.
- *
- * domainModel.tags and vm.form.tags use different formats.  This
- * function returns a domainModel.tags-formatted copy of the given
- * vm.form.tags-formatted array.
- *
- */
-function domainModelTagsFromViewModelTags(viewModelTags) {
-  return (viewModelTags || []).map(function(tag) {
-    return tag.text;
-  });
-}
-
 /** Return a human-readable error message for the given server error.
  *
  * @param {object} reason The error object from the server. Should have
@@ -92,16 +79,13 @@ function saveToDrafts(drafts, domainModel, vm) {
  * Copy any properties from vm that might have been modified by the user into
  * domainModel, overwriting any existing properties in domainModel.
  *
- * Additionally, the `tags` property of vm - an array of objects each with a
- * `text` string - will become a simple array of strings in domainModel.
- *
  * @param {object} domainModel The object to copy properties to
  * @param {object} vm The object to copy properties from
  *
  */
 function updateDomainModel(domainModel, vm, permissions) {
   domainModel.text = vm.form.text;
-  domainModel.tags = domainModelTagsFromViewModelTags(vm.form.tags);
+  domainModel.tags = vm.form.tags;
   if (vm.isPrivate) {
     domainModel.permissions = permissions.private();
   } else {
@@ -115,7 +99,7 @@ function updateViewModel($scope, domainModel,
 
   vm.form = {
     text: domainModel.text,
-    tags: viewModelTagsFromDomainModelTags(domainModel.tags),
+    tags: domainModel.tags,
   };
 
   if (domainModel.links) {
@@ -136,19 +120,6 @@ function updateViewModel($scope, domainModel,
   vm.documentDomain = documentDomain(documentMetadata);
 }
 
-/** Return a vm tags array from the given domainModel tags array.
- *
- * domainModel.tags and vm.form.tags use different formats.  This
- * function returns a vm.form.tags-formatted copy of the given
- * domainModel.tags-formatted array.
- *
- */
-function viewModelTagsFromDomainModelTags(domainModelTags) {
-  return (domainModelTags || []).map(function(tag) {
-    return {text: tag};
-  });
-}
-
 /**
   * @ngdoc type
   * @name annotation.AnnotationController
@@ -158,7 +129,7 @@ function viewModelTagsFromDomainModelTags(domainModelTags) {
 function AnnotationController(
   $document, $q, $rootScope, $scope, $timeout, $window, annotationUI,
   annotationMapper, drafts, flash, features, groups, permissions, session,
-  settings, tags) {
+  settings) {
 
   var vm = this;
   var domainModel;
@@ -554,13 +525,6 @@ function AnnotationController(
       return Promise.resolve();
     }
 
-    // Update stored tags with the new tags of this annotation.
-    var newTags = vm.form.tags.filter(function(tag) {
-      var tags = domainModel.tags || [];
-      return tags.indexOf(tag.text) === -1;
-    });
-    tags.store(newTags);
-
     var saved;
     switch (vm.action) {
       case 'create':
@@ -624,16 +588,6 @@ function AnnotationController(
     vm.isPrivate = (privacy === 'private');
   };
 
-  /**
-    * @ngdoc method
-    * @name annotation.AnnotationController#tagsAutoComplete.
-    * @returns {Promise} immediately resolved to {string[]} -
-    * the tags to show in autocomplete.
-    */
-  vm.tagsAutoComplete = function(query) {
-    return $q.when(tags.filter(query));
-  };
-
   vm.tagStreamURL = function(tag) {
     return vm.serviceUrl + 'stream?q=tag:' + encodeURIComponent(tag);
   };
@@ -677,6 +631,10 @@ function AnnotationController(
     vm.form.text = text;
   };
 
+  vm.setTags = function (tags) {
+    vm.form.tags = tags;
+  };
+
   init();
 }
 

h/static/scripts/directive/tag-editor.js

+'use strict';
+
+// @ngInject
+function TagEditorController(tags) {
+  this.onTagsChanged = function () {
+    tags.store(this.tagList);
+
+    var newTags = this.tagList.map(function (item) { return item.text; });
+    this.onEditTags({tags: newTags});
+  };
+
+  this.autocomplete = function (query) {
+    return Promise.resolve(tags.filter(query));
+  };
+
+  this.$onChanges = function (changes) {
+    if (changes.tags) {
+      this.tagList = changes.tags.currentValue.map(function (tag) {
+        return {text: tag};
+      });
+    }
+  };
+}
+
+module.exports = function () {
+  return {
+    bindToController: true,
+    controller: TagEditorController,
+    controllerAs: 'vm',
+    restrict: 'E',
+    scope: {
+      tags: '<',
+      onEditTags: '&',
+    },
+    template: require('../../../templates/client/tag_editor.html'),
+  };
+};

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

@@ -83,20 +83,14 @@ describe('annotation', function() {
       var domainModel = {};
       var viewModel = {
         form: {
-          tags: [
-            {text: 'foo'},
-            {text: 'bar'}
-          ]
+          tags: ['foo', 'bar'],
         }
       };
 
       updateDomainModel(domainModel, viewModel, fakePermissions(),
                         fakeGroups());
 
-      assert.deepEqual(
-        domainModel.tags, ['foo', 'bar'],
-        'The array of {tag: "text"} objects in  viewModel becomes an array ' +
-        'of "text" strings in domainModel');
+      assert.deepEqual(domainModel.tags, ['foo', 'bar']);
     });
 
     it('sets domainModel.permissions to private if vm.isPrivate', function() {
@@ -230,11 +224,6 @@ describe('annotation', function() {
         serviceUrl: 'https://test.hypothes.is/',
       };
 
-      var fakeTags = {
-        filter: sandbox.stub().returns('a while ago'),
-        store: sandbox.stub()
-      };
-
       fakeGroups = {
         focused: function() {
           return {};
@@ -250,7 +239,6 @@ describe('annotation', function() {
       $provide.value('permissions', fakePermissions);
       $provide.value('session', fakeSession);
       $provide.value('settings', fakeSettings);
-      $provide.value('tags', fakeTags);
       $provide.value('groups', fakeGroups);
     }));
 
@@ -700,11 +688,7 @@ describe('annotation', function() {
         controller.form.text = 'bar';
         assert.ok(controller.hasContent());
         controller.form.text = '';
-        controller.form.tags = [
-          {
-            text: 'foo'
-          }
-        ];
+        controller.form.tags = ['foo'];
         assert.ok(controller.hasContent());
       });
     });
@@ -995,11 +979,7 @@ describe('annotation', function() {
     describe('drafts', function() {
       it('starts editing immediately if there is a draft', function() {
         fakeDrafts.get.returns({
-          tags: [
-            {
-              text: 'unsaved'
-            }
-          ],
+          tags: ['unsaved'],
           text: 'unsaved-text'
         });
         var controller = createDirective().controller;
@@ -1008,15 +988,11 @@ describe('annotation', function() {
 
       it('uses the text and tags from the draft if present', function() {
         fakeDrafts.get.returns({
-          tags: [{text: 'unsaved-tag'}],
+          tags: ['unsaved-tag'],
           text: 'unsaved-text'
         });
         var controller = createDirective().controller;
-        assert.deepEqual(controller.form.tags, [
-          {
-            text: 'unsaved-tag'
-          }
-        ]);
+        assert.deepEqual(controller.form.tags, ['unsaved-tag']);
         assert.equal(controller.form.text, 'unsaved-text');
       });
 
@@ -1096,7 +1072,7 @@ describe('annotation', function() {
       it('does not remove the current annotation if it has tags', function () {
         var annotation = fixtures.newAnnotation();
         var parts = createDirective(annotation);
-        parts.controller.form.tags = [{text: 'a-tag'}];
+        parts.controller.form.tags = ['a-tag'];
         $rootScope.$emit(events.BEFORE_ANNOTATION_CREATED,
           fixtures.newAnnotation());
         assert.notCalled(fakeDrafts.remove);

h/static/scripts/directive/test/tag-editor-test.js

+'use strict';
+
+var angular = require('angular');
+
+var util = require('./util');
+
+describe('tagEditor', function () {
+  var fakeTags;
+
+  before(function () {
+    angular.module('app',[])
+      .directive('tagEditor', require('../tag-editor'));
+  });
+
+  beforeEach(function () {
+    fakeTags = {
+      filter: sinon.stub(),
+      store: sinon.stub(),
+    };
+
+    angular.mock.module('app', {
+      tags: fakeTags,
+    });
+  });
+
+  it('converts tags to the form expected by ng-tags-input', function () {
+    var element = util.createDirective(document, 'tag-editor', {
+      tags: ['foo', 'bar']
+    });
+    assert.deepEqual(element.ctrl.tagList, [{text: 'foo'}, {text: 'bar'}]);
+  });
+
+  describe('when tags are changed', function () {
+    var element;
+    var onEditTags;
+
+    beforeEach(function () {
+      onEditTags = sinon.stub();
+      element = util.createDirective(document, 'tag-editor', {
+        onEditTags: {args: ['tags'], callback: onEditTags},
+        tags: ['foo'],
+      });
+      element.ctrl.onTagsChanged();
+    });
+
+    it('calls onEditTags handler', function () {
+      assert.calledWith(onEditTags, sinon.match(['foo']));
+    });
+
+    it('saves tags to the store', function () {
+      assert.calledWith(fakeTags.store, sinon.match([{text: 'foo'}]));
+    });
+  });
+
+  describe('#autocomplete', function () {
+    it('suggests tags using the `tags` service', function () {
+      var element = util.createDirective(document, 'tag-editor', {tags: []});
+      element.ctrl.autocomplete('query');
+      assert.calledWith(fakeTags.filter, 'query');
+    });
+  });
+});

h/templates/client/annotation.html

@@ -84,24 +84,15 @@
 
   <!-- Tags -->
   <div class="annotation-body form-field" ng-if="vm.editing()">
-    <tags-input ng-model="vm.form.tags"
-      name="tags"
-      class="tags"
-      placeholder="Add tags…"
-      min-length="1"
-      replace-spaces-with-dashes="false"
-      enable-editing-last-tag="true">
-      <auto-complete source="vm.tagsAutoComplete($query)"
-        min-length="1"
-        max-results-to-show="10"></auto-complete>
-    </tags-input>
+    <tag-editor tags="vm.form.tags"
+                on-edit-tags="vm.setTags(tags)"></tag-editor>
   </div>
 
   <div class="annotation-body u-layout-row tags tags-read-only"
        ng-if="(vm.canCollapseBody || vm.form.tags.length) && !vm.editing()">
     <ul class="tag-list">
       <li class="tag-item" ng-repeat="tag in vm.form.tags">
-        <a ng-href="{{vm.tagStreamURL(tag.text)}}" target="_blank">{{tag.text}}</a>
+        <a ng-href="{{vm.tagStreamURL(tag)}}" target="_blank">{{tag}}</a>
       </li>
     </ul>
     <div class="u-stretch"></div>

h/templates/client/tag_editor.html

+<tags-input ng-model="vm.tagList"
+  name="tags"
+  class="tags"
+  placeholder="Add tags…"
+  min-length="1"
+  replace-spaces-with-dashes="false"
+  enable-editing-last-tag="true"
+  on-tag-added="vm.onTagsChanged()"
+  on-tag-removed="vm.onTagsChanged()">
+  <auto-complete source="vm.autocomplete($query)"
+    min-length="1"
+    max-results-to-show="10"></auto-complete>
+</tags-input>