Fixes #121 list toggle - improve line break handling and reduce defer…

…red function calls
Voog · Mar 16, 2015 · bc28e08 · bc28e08
1 parent 994834e
commit bc28e08
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 11 deletions.
diff --git a/src/commands/insertList.js b/src/commands/insertList.js
@@ -58,8 +58,9 @@ wysihtml5.commands.insertList = (function(wysihtml5) {
     // <ul><li>foo</li><li>bar</li></ul>
     // becomes:
     // foo<br>bar<br>
-    composer.selection.executeAndRestore(function() {
-      var otherLists = getListsInSelection(otherNodeName, composer);
+
+    composer.selection.executeAndRestoreRangy(function() {
+      otherLists = getListsInSelection(otherNodeName, composer);
       if (otherLists.length) {
         for (var l = otherLists.length; l--;) {
           wysihtml5.dom.renameElement(otherLists[l], nodeName.toLowerCase());
@@ -81,7 +82,7 @@ wysihtml5.commands.insertList = (function(wysihtml5) {
     // becomes:
     // <ul><li>foo</li><li>bar</li></ul>
     // Also rename other lists in selection
-    composer.selection.executeAndRestore(function() {
+    composer.selection.executeAndRestoreRangy(function() {
       var renameLists = [el].concat(getListsInSelection(otherNodeName, composer));
 
       // All selection inner lists get renamed too

diff --git a/src/dom/resolve_list.js b/src/dom/resolve_list.js
@@ -42,12 +42,15 @@
     var doc             = list.ownerDocument,
         fragment        = doc.createDocumentFragment(),
         previousSibling = wysihtml5.dom.domNode(list).prev({ignoreBlankTexts: true}),
+        nextSibling = wysihtml5.dom.domNode(list).next({ignoreBlankTexts: true}),
         firstChild,
         lastChild,
         isLastChild,
         shouldAppendLineBreak,
         paragraph,
-        listItem;
+        listItem,
+        lastListItem = list.lastElementChild || list.lastChild,
+        isLastItem;
 
     if (useLineBreaks) {
       // Insert line break if list is after a non-block element
@@ -57,10 +60,11 @@
 
       while (listItem = (list.firstElementChild || list.firstChild)) {
         lastChild = listItem.lastChild;
+        isLastItem = listItem === lastListItem;
         while (firstChild = listItem.firstChild) {
           isLastChild           = firstChild === lastChild;
           // This needs to be done before appending it to the fragment, as it otherwise will lose style information
-          shouldAppendLineBreak = isLastChild && !_isBlockElement(firstChild) && !_isLineBreak(firstChild);
+          shouldAppendLineBreak = (!isLastItem || (nextSibling && !_isBlockElement(nextSibling))) && isLastChild && !_isBlockElement(firstChild) && !_isLineBreak(firstChild);
           fragment.appendChild(firstChild);
           if (shouldAppendLineBreak) {
             _appendLineBreak(fragment);

diff --git a/test/dom/resolve_list_test.js b/test/dom/resolve_list_test.js
@@ -16,32 +16,41 @@ module("wysihtml5.dom.resolveList", {
 test("Basic tests (useLineBreaks = true)", function() {
   this.equal(
     this.resolveList("<ul><li>foo</li></ul>", true),
-    "foo<br>"
+    "foo",
+    "List with one li element resolved"
+  );
+
+  this.equal(
+    this.resolveList("<ul><li>foo</li></ul>Test", true),
+    "foo<br>Test",
+    "List with test after adds line break"
   );
 
   this.equal(
     this.resolveList("<ul><li>foo</li><li>bar</li></ul>", true),
-    "foo<br>bar<br>"
+    "foo<br>bar",
+    "List with two li elements resolved"
   );
 
   this.equal(
     this.resolveList("<ol><li>foo</li><li>bar</li></ol>", true),
-    "foo<br>bar<br>"
+    "foo<br>bar",
+    "Numbered list with two li elements resolved"
   );
 
   this.equal(
     this.resolveList("<ol><li></li><li>bar</li></ol>", true),
-    "bar<br>"
+    "bar"
   );
 
   this.equal(
     this.resolveList("<ol><li>foo<br></li><li>bar</li></ol>", true),
-    "foo<br>bar<br>"
+    "foo<br>bar"
   );
 
   this.equal(
     this.resolveList("<ul><li><h1>foo</h1></li><li><div>bar</div></li><li>baz</li></ul>", true),
-    "<h1>foo</h1><div>bar</div>baz<br>"
+    "<h1>foo</h1><div>bar</div>baz"
   );
 });