Merge pull request #3114 from hypothesis/editor-block-formatting

Fix list/quote toolbar command when selection is empty and cursor is at start of line

h/static/scripts/markdown-commands.js

@@ -41,14 +41,14 @@ function replaceText(state, pos, length, text) {
   var newSelectionStart = state.selectionStart;
   var newSelectionEnd = state.selectionEnd;
 
-  if (newSelectionEnd <= pos) {
-    // 1. Selection is before replaced text: Leave selection unchanged
-  } else if (newSelectionStart >= pos + length) {
-    // 2. Selection is after replaced text:
+  if (newSelectionStart >= pos + length) {
+    // 1. Selection is after replaced text:
     //    Increment (start, end) by difference in length between original and
     //    replaced text
     newSelectionStart += text.length - length;
     newSelectionEnd += text.length - length;
+  } else if (newSelectionEnd <= pos) {
+    // 2. Selection is before replaced text: Leave selection unchanged
   } else if (newSelectionStart <= pos &&
              newSelectionEnd >= pos + length) {
     // 3. Selection fully contains replaced text:
@@ -148,7 +148,7 @@ function toggleSpanStyle(state, prefix, suffix, placeholder) {
 
   if (state.selectionStart === state.selectionEnd && placeholder) {
     newState = replaceText(state, state.selectionStart, 0, placeholder);
-    newState.selectionEnd = newState.selectionStart + placeholder.length;
+    newState.selectionStart = newState.selectionEnd - placeholder.length;
   }
 
   if (selectionPrefix === prefix && selectionSuffix === suffix) {

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

@@ -72,23 +72,32 @@ describe('markdown commands', function () {
   });
 
   describe('block formatting', function () {
-    function toggle(state) {
-      return commands.toggleBlockStyle(state, '> ');
-    }
-
-    it('adds formatting to blocks', function () {
-      var output = toggle(parseState('one\n<sel>two\nthree</sel>\nfour'));
-      assert.equal(formatState(output), 'one\n> <sel>two\n> three</sel>\nfour');
-    });
-
-    it('removes formatting from blocks', function () {
-      var output = toggle(parseState('one \n<sel>> two\n> three</sel>\nfour'));
-      assert.equal(formatState(output), 'one \n<sel>two\nthree</sel>\nfour');
-    });
+    var CASES = {
+      'adds formatting to blocks': {
+        input: 'one\n<sel>two\nthree</sel>\nfour',
+        output: 'one\n> <sel>two\n> three</sel>\nfour',
+      },
+      'removes formatting from blocks': {
+        input: 'one \n<sel>> two\n> three</sel>\nfour',
+        output: 'one \n<sel>two\nthree</sel>\nfour',
+      },
+      'preserves the selection': {
+        input: 'one <sel>two\nthree </sel>four',
+        output: '> one <sel>two\n> three </sel>four',
+      },
+      'inserts the block prefix before an empty selection': {
+        input: '<sel></sel>',
+        output: '> <sel></sel>',
+      }
+    };
 
-    it('preserves the selection', function () {
-      var output = toggle(parseState('one <sel>two\nthree </sel>four'));
-      assert.equal(formatState(output), '> one <sel>two\n> three </sel>four');
+    Object.keys(CASES).forEach(function (case_) {
+      it(case_, function () {
+        var output = commands.toggleBlockStyle(
+          parseState(CASES[case_].input), '> '
+        );
+        assert.equal(formatState(output), CASES[case_].output);
+      });
     });
   });