Fix #689

Author: YoranBrondsema

src/js/editor/post.js

@@ -790,7 +790,6 @@ class PostEditor {
 
     sectionTagName = normalizeTagName(sectionTagName);
     let { post } = this.editor;
-    let nextRange = range;
 
     let everySectionHasTagName = true;
     post.walkMarkerableSections(range, section => {
@@ -800,18 +799,47 @@ class PostEditor {
     });
 
     let tagName = everySectionHasTagName ? 'p' : sectionTagName;
-    let firstChanged;
+    let sectionTransformations = [];
     post.walkMarkerableSections(range, section => {
       let changedSection = this.changeSectionTagName(section, tagName);
-      firstChanged = firstChanged || changedSection;
+      sectionTransformations.push({
+        from: section,
+        to: changedSection
+      });
     });
 
-    if (firstChanged) {
-      nextRange = firstChanged.headPosition().toRange();
-    }
+    let nextRange = this._determineNextRangeAfterToggleSection(range, sectionTransformations);
     this.setRange(nextRange);
   }
 
+  _determineNextRangeAfterToggleSection(range, sectionTransformations) {
+    if (sectionTransformations.length) {
+      let changedHeadSection = sectionTransformations
+        .find(({ from }) => { return from === range.headSection; })
+        .to;
+      let changedTailSection = sectionTransformations
+        .find(({ from }) => { return from === range.tailSection; })
+        .to;
+
+      if (changedHeadSection.isListSection || changedTailSection.isListSection) {
+        // We don't know to which ListItem's the original sections point at, so
+        // we don't have enough information to reconstruct the range when
+        // dealing with lists.
+        return sectionTransformations[0].to.headPosition().toRange();
+      } else {
+        return Range.create(
+          changedHeadSection,
+          range.headSectionOffset,
+          changedTailSection,
+          range.tailSectionOffset,
+          range.direction
+        );
+      }
+    } else {
+      return range;
+    }
+  }
+
   setAttribute(key, value, range=this._range) {
     range = toRange(range);
     let { post } = this.editor;

tests/unit/editor/post-test.js

@@ -815,7 +815,16 @@ test('#toggleSection changes multiple sections to and from tag name', (assert) =
   assert.equal(post.sections.head.tagName, 'p');
   assert.equal(post.sections.tail.tagName, 'p');
 
-  assert.positionIsEqual(mockEditor._renderedRange.head, post.sections.head.headPosition());
+  assert.positionIsEqual(
+    mockEditor._renderedRange.head,
+    post.sections.head.toPosition(2),
+    'Maintains the selection'
+  );
+  assert.positionIsEqual(
+    mockEditor._renderedRange.tail,
+    post.sections.tail.toPosition(2),
+    'Maintains the selection'
+  );
 });
 
 test('#toggleSection skips over non-markerable sections', (assert) => {
@@ -1301,6 +1310,33 @@ test('#toggleSection joins contiguous list items', (assert) => {
                    ['abc', '123', 'def']);
 });
 
+test('#toggleSection maintains the selection when the sections in the selected range are still there', (assert) => {
+  let post = Helpers.postAbstract.build(({post, markupSection, marker}) => {
+    return post([
+      markupSection('p', [marker('abc')])
+    ]);
+  });
+
+  mockEditor = renderBuiltAbstract(post, mockEditor);
+  const range = Range.create(post.sections.head, 1,
+                             post.sections.head, 2);
+
+  postEditor = new PostEditor(mockEditor);
+  postEditor.toggleSection('h1', range);
+  postEditor.complete();
+
+  assert.positionIsEqual(
+    mockEditor._renderedRange.head,
+    post.sections.head.toPosition(1),
+    'Maintains the selection'
+  );
+  assert.positionIsEqual(
+    mockEditor._renderedRange.tail,
+    post.sections.tail.toPosition(2),
+    'Maintains the selection'
+  );
+});
+
 test('#toggleMarkup when cursor is in non-markerable does nothing', (assert) => {
   editor = buildEditorWithMobiledoc(
     ({post, markupSection, marker, cardSection}) => {