Use data-in, events-out pattern for <markdown> component

This simplifies the implementation of the <markdown> component slightly,
and makes its inputs more consistent with other recently written components.

 - Replace use of 'ngModel' with 'text' input property and 'onEditText'
   output event.

 - Add tests for preview state.

 - Add tests for rendering and editing annotations that have no text.

 - Optimize editor initialization by not creating the toolbar's DOM
   elements until the user starts editing the annotation, via using
   ng-if rather than ng-show to hide the toolbar in view mode.

h/static/scripts/directive/annotation.js

@@ -713,6 +713,10 @@ function AnnotationController(
     $scope.$digest();
   };
 
+  vm.setText = function (text) {
+    vm.form.text = text;
+  };
+
   init();
 }
 

h/static/scripts/directive/markdown.js

@@ -15,11 +15,10 @@ var mediaEmbedder = require('../media-embedder');
  * the markdown editor.
  */
 // @ngInject
-module.exports = function($filter, $sanitize, $sce) {
+module.exports = function($filter, $sanitize) {
   return {
-    link: function(scope, elem, attr, ctrl) {
-      if (!(typeof ctrl !== "undefined" && ctrl !== null)) { return; }
-
+    controller: function () {},
+    link: function(scope, elem) {
       var input = elem[0].querySelector('.js-markdown-input');
       var inputEl = angular.element(input);
       var output = elem[0].querySelector('.js-markdown-preview');
@@ -128,17 +127,8 @@ module.exports = function($filter, $sanitize, $sce) {
       });
 
       scope.preview = false;
-      scope.togglePreview = function() {
-        if (!scope.readOnly) {
-          scope.preview = !scope.preview;
-          if (scope.preview) {
-            output.style.height = input.style.height;
-            return ctrl.$render();
-          } else {
-            input.style.height = output.style.height;
-            focusInput();
-          }
-        }
+      scope.togglePreview = function () {
+        scope.preview = !scope.preview;
       };
 
       var renderInlineMath = function(textToCheck) {
@@ -229,38 +219,39 @@ module.exports = function($filter, $sanitize, $sce) {
         return domElement.innerHTML;
       };
 
+      // React to the changes to the input
+      inputEl.bind('blur change keyup', function () {
+        scope.onEditText({text: input.value});
+      });
+
       // Re-render the markdown when the view needs updating.
-      ctrl.$render = function() {
-        if (!scope.readOnly && !scope.preview) {
-          input.value = ctrl.$viewValue || '';
-        }
-        var value = ctrl.$viewValue || '';
-        output.innerHTML = renderMathAndMarkdown(value);
+      scope.$watch('text', function () {
+        output.innerHTML = renderMathAndMarkdown(scope.text || '');
+      });
+
+      scope.showEditor = function () {
+        return !scope.readOnly && !scope.preview;
       };
 
-      // React to the changes to the input
-      inputEl.bind('blur change keyup', function() {
-        ctrl.$setViewValue(input.value);
+      scope.$watch('readOnly', function () {
+        // Exit preview mode when editor stops
+        scope.preview = false;
       });
 
-      // Reset height of output div in case it has been changed.
-      // Re-render when it becomes uneditable.
-      // Auto-focus the input box when the widget becomes editable.
-      scope.$watch('readOnly', function(readOnly) {
-        scope.preview = false;
-        output.style.height = "";
-        ctrl.$render();
-        if (!readOnly) {
+      scope.$watch('showEditor()', function (show) {
+        if (show) {
+          input.value = scope.text || '';
           focusInput();
         }
       });
     },
 
-    require: '?ngModel',
     restrict: 'E',
     scope: {
       readOnly: '<',
-      required: '@'
+      text: '<?',
+      onEditText: '&',
+      required: '@',
     },
     template: require('../../../templates/client/markdown.html'),
   };

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

@@ -66,7 +66,7 @@ describe('markdown', function () {
     it('should show the rendered view when readOnly is true', function () {
       var editor = util.createDirective(document, 'markdown', {
         readOnly: true,
-        ngModel: 'Hello World',
+        text: 'Hello World',
       });
       assert.isTrue(isHidden(inputElement(editor)));
       assert.isFalse(isHidden(viewElement(editor)));
@@ -75,7 +75,7 @@ describe('markdown', function () {
     it('should show the editor when readOnly is false', function () {
       var editor = util.createDirective(document, 'markdown', {
         readOnly: false,
-        ngModel: 'Hello World',
+        text: 'Hello World',
       });
       assert.isFalse(isHidden(inputElement(editor)));
       assert.isTrue(isHidden(viewElement(editor)));
@@ -86,17 +86,22 @@ describe('markdown', function () {
     it('should render input markdown', function () {
       var editor = util.createDirective(document, 'markdown', {
         readOnly: true,
-        ngModel: 'Hello World',
+        text: 'Hello World',
       });
       assert.equal(getRenderedHTML(editor), 'rendered:Hello World');
     });
+
+    it('should render nothing if no text is provided', function () {
+      var editor = util.createDirective(document, 'markdown', {readOnly: true});
+      assert.equal(getRenderedHTML(editor), 'rendered:');
+    });
   });
 
   describe('math rendering', function () {
     it('should render LaTeX', function () {
       var editor = util.createDirective(document, 'markdown', {
         readOnly: true,
-        ngModel: '$$x*2$$',
+        text: '$$x*2$$',
       });
       assert.equal(getRenderedHTML(editor),
         'rendered:math:\\displaystyle {x*2}rendered:');
@@ -107,7 +112,7 @@ describe('markdown', function () {
     it('should apply formatting when clicking toolbar buttons', function () {
       var editor = util.createDirective(document, 'markdown', {
         readOnly: false,
-        ngModel: 'Hello World',
+        text: 'Hello World',
       });
       var input = inputElement(editor);
       var buttons = editor[0].querySelectorAll('.markdown-tools-button');
@@ -120,15 +125,84 @@ describe('markdown', function () {
   });
 
   describe('editing', function () {
-    it('should update the input model', function () {
+    it('should populate the input with the current text', function () {
+      var editor = util.createDirective(document, 'markdown', {
+        readOnly: false,
+        text: 'initial comment',
+        onEditText: function () {},
+      });
+      var input = inputElement(editor);
+      assert.equal(input.value, 'initial comment');
+    });
+
+    it('should populate the input with empty text if no text is specified', function () {
+      var editor = util.createDirective(document, 'markdown', {
+        readOnly: false,
+        onEditText: function () {},
+      });
+      var input = inputElement(editor);
+      assert.equal(input.value, '');
+    });
+
+    it('should call onEditText() callback when text changes', function () {
+      var onEditText = sinon.stub();
       var editor = util.createDirective(document, 'markdown', {
         readOnly: false,
-        ngModel: 'Hello World',
+        text: 'Hello World',
+        onEditText: {
+          args: ['text'],
+          callback: onEditText,
+        },
       });
       var input = inputElement(editor);
       input.value = 'new text';
       util.sendEvent(input, 'change');
-      assert.equal(editor.scope.ngModel, 'new text');
+      assert.called(onEditText);
+      assert.calledWith(onEditText, 'new text');
+    });
+  });
+
+  describe('preview state', function () {
+    var editor;
+
+    function togglePreview() {
+      var toggle = editor[0].querySelector('.markdown-tools-toggle');
+      angular.element(toggle).click();
+      editor.scope.$digest();
+    }
+
+    function isPreviewing() {
+      return editor.isolateScope().preview;
+    }
+
+    beforeEach(function () {
+      // Create a new editor, initially in editing mode
+      editor = util.createDirective(document, 'markdown', {
+        readOnly: false,
+        text: 'Hello World',
+      });
+    });
+
+    it('enters preview mode when clicking the "Preview" toggle button', function () {
+      togglePreview();
+      assert.isTrue(isPreviewing());
+    });
+
+    it('should hide the input when previewing changes', function () {
+      togglePreview();
+      assert.isTrue(isHidden(inputElement(editor)));
+    });
+
+    it('should show the rendered markdown when previewing changes', function () {
+      togglePreview();
+      assert.isFalse(isHidden(viewElement(editor)));
+    });
+
+    it('exits preview mode when switching to read-only mode', function () {
+      togglePreview();
+      editor.scope.readOnly = true;
+      editor.scope.$digest();
+      assert.isFalse(isPreviewing());
     });
   });
 });

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

@@ -96,8 +96,8 @@ function createDirective(document, name, attrs, initialScope, initialHtml, opts)
   opts = opts || {};
   opts.parentElement = opts.parentElement || document.body;
 
-  // create a template consisting of a single element, the directive
-  // we want to create and compile it
+  // Create a template consisting of a single element, the directive
+  // we want to create and compile it.
   var $compile;
   var $scope;
   angular.mock.inject(function (_$compile_, _$rootScope_) {
@@ -109,15 +109,20 @@ function createDirective(document, name, attrs, initialScope, initialHtml, opts)
     var attrName = hyphenate(key);
     var attrKey = key;
     if (typeof attrs[key] === 'function') {
+      // If the input property is a function, generate a function expression,
+      // eg. `<my-component on-event="onEvent()">`
       attrKey += '()';
     } else if (attrs[key].callback) {
+      // If the input property is a function which accepts arguments,
+      // generate the argument list.
+      // eg. `<my-component on-change="onChange(newValue)">`
       attrKey += '(' + attrs[key].args.join(',') + ')';
     }
     templateElement.setAttribute(attrName, attrKey);
   });
   templateElement.innerHTML = initialHtml;
 
-  // add the element to the document's body so that
+  // Add the element to the document's body so that
   // it responds to events, becomes visible, reports correct
   // values for its dimensions etc.
   opts.parentElement.appendChild(templateElement);

h/templates/client/annotation.html

@@ -80,7 +80,8 @@
              collapsed-height="400"
              overflow-hysteresis="20"
              content-data="vm.form.text">
-      <markdown ng-model="vm.form.text"
+      <markdown text="vm.form.text"
+                on-edit-text="vm.setText(text)"
                 read-only="!vm.editing()">
       </markdown>
     </excerpt>

h/templates/client/markdown.html

-<div ng-hide="readOnly" class="markdown-tools" ng-class="preview && 'disable'">
+<div ng-if="!readOnly" class="markdown-tools" ng-class="preview && 'disable'">
   <span class="markdown-preview-toggle">
     <a class="markdown-tools-badge h-icon-markdown" href="https://help.github.com/articles/markdown-basics" title="Parsed as Markdown" target="_blank"></a>
     <a href="" class="markdown-tools-toggle" ng-click="togglePreview()" ng-show="!preview">Preview</a>
@@ -14,7 +14,10 @@
   <i class="h-icon-format-list-bulleted markdown-tools-button" ng-click="insertList()" title="Insert list"></i>
 </div>
 <textarea class="form-input form-textarea js-markdown-input"
-          ng-hide="readOnly || preview"
+          ng-show="showEditor()"
           ng-click="$event.stopPropagation()"
           ng-required="required"></textarea>
-<div class="styled-text js-markdown-preview" ng-class="preview && 'markdown-preview'" ng-dblclick="togglePreview()" ng-show="readOnly || preview"></div>
+<div class="styled-text js-markdown-preview"
+     ng-class="preview && 'markdown-preview'"
+     ng-dblclick="togglePreview()"
+     ng-show="!showEditor()"></div>