fix grouping of nested lists into single top-level list

src/js/parsers/section.js

@@ -80,31 +80,16 @@ class SectionParser {
     if (!finished) {
       let childNodes = isTextNode(element) ? [element] : element.childNodes;
 
-      if (this.state.section.isListSection) {
-        this.parseListItems(childNodes);
-      } else {
-        forEach(childNodes, el => {
-          this.parseNode(el);
-        });
-      }
+      forEach(childNodes, el => {
+        this.parseNode(el);
+      });
     }
 
     this._closeCurrentSection();
 
     return this.sections;
   }
 
-  parseListItems(childNodes) {
-    let { state } = this;
-    forEach(childNodes, el => {
-      let parsed = new this.constructor(this.builder).parse(el);
-      let li = parsed[0];
-      if (li && li.isListItem) {
-        state.section.items.append(li);
-      }
-    });
-  }
-
   runPlugins(node) {
     let isNodeFinished = false;
     let env = {
@@ -155,29 +140,44 @@ class SectionParser {
     }
 
     // handle closing the current section and starting a new one if we hit a
-    // new-section-creating element
-    if (this.state.section && !this.state.section.isListItem && !isTextNode(node) && node.tagName) {
-      // handle lists nested inside wrappers
-      if (this.state.section.isListSection) {
-        this.parseListItems(node.childNodes);
-        return;
-      }
-
+    // new-section-creating element.
+    if (this.state.section && !isTextNode(node) && node.tagName) {
       let tagName = normalizeTagName(node.tagName);
-      if (contains(VALID_MARKUP_SECTION_TAGNAMES, tagName) || contains(VALID_LIST_SECTION_TAGNAMES, tagName)) {
-        // avoid creating empty paragraphs due to wrappers elements around
+      let isNestedListSection = contains(VALID_LIST_SECTION_TAGNAMES, tagName)
+        && this.state.section.isListItem;
+
+      if (
+        contains(VALID_MARKUP_SECTION_TAGNAMES, tagName) ||
+        (contains(VALID_LIST_SECTION_TAGNAMES, tagName) && !isNestedListSection) ||
+        contains(VALID_LIST_ITEM_TAGNAMES, tagName)
+      ) {
+        // don't break out of the list for list items that contain a single <p>.
+        // deals with typical case of <li><p>Text</p></li><li><p>Text</p></li>
+        if (this.state.section.isListItem && tagName === 'p' && !node.nextSibling) {
+          this.parseElementNode(node);
+          return;
+        }
+
+        // avoid creating empty paragraphs due to wrapper elements around
         // section-creating elements
         if (this.state.section.isMarkerable && !this.state.text) {
           this.state.section = null;
         } else {
           this._closeCurrentSection();
         }
+
         this._updateStateFromElement(node);
+      }
 
-        if (this.state.section.isListSection) {
-          this.parseListItems(node.childNodes);
-          return;
-        }
+      if (this.state.section.isListSection) {
+        // ensure the list section is closed and added to the sections list.
+        // _closeCurrentSection handles pushing list items onto the list section
+        this._closeCurrentSection();
+
+        forEach(node.childNodes, (node) => {
+          this.parseNode(node);
+        });
+        return;
       }
     }
 
@@ -227,6 +227,7 @@ class SectionParser {
 
   _closeCurrentSection() {
     let { sections, state } = this;
+    let lastSection = sections[sections.length - 1];
 
     if (!state.section) {
       return;
@@ -237,7 +238,18 @@ class SectionParser {
       this._createMarker();
     }
 
-    sections.push(state.section);
+    // push listItems onto the listSection or add a new section
+    if (state.section.isListItem && lastSection && lastSection.isListSection) {
+      lastSection.items.append(state.section);
+    } else {
+      // remove empty list sections before creating a new section
+      if (lastSection && lastSection.isListSection && lastSection.items.length === 0) {
+        sections.pop();
+      }
+
+      sections.push(state.section);
+    }
+
     state.section = null;
   }
 

tests/unit/parsers/section-test.js

@@ -199,12 +199,14 @@ test("#parse handles multiple paragraphs in list item", assert => {
   parser = new SectionParser(builder);
   let sections = parser.parse(element);
 
-  assert.equal(sections.length, 1, "single list section");
+  assert.equal(sections.length, 2, '2 sections');
 
-  let list = sections[0];
-  assert.equal(list.type, "list-section");
-  assert.equal(list.items.length, 1, "1 list item");
-  assert.equal(list.items.objectAt(0).text, "OneTwo");
+  let p1 = sections[0];
+  assert.equal(p1.type, 'markup-section', 'first section type');
+  assert.equal(p1.text, 'One');
+  let p2 = sections[1];
+  assert.equal(p2.type, "markup-section", "second section type");
+  assert.equal(p2.text, "Two");
 });
 
 test("#parse handles multiple headers in list item", assert => {
@@ -216,12 +218,16 @@ test("#parse handles multiple headers in list item", assert => {
   parser = new SectionParser(builder);
   let sections = parser.parse(element);
 
-  assert.equal(sections.length, 1, "single list section");
+  assert.equal(sections.length, 2, '2 sections');
 
-  let list = sections[0];
-  assert.equal(list.type, "list-section");
-  assert.equal(list.items.length, 1, "1 list item");
-  assert.equal(list.items.objectAt(0).text, "OneTwo");
+  let h1 = sections[0];
+  assert.equal(h1.type, 'markup-section', 'first section type');
+  assert.equal(h1.text, 'One');
+  assert.equal(h1.tagName, 'h1');
+  let h2 = sections[1];
+  assert.equal(h2.type, 'markup-section', 'second section type');
+  assert.equal(h2.text, 'Two');
+  assert.equal(h2.tagName, 'h2');
 });
 
 // see https://github.com/bustle/mobiledoc-kit/issues/656