Parse ul and ols correctly

fixes #183

src/js/models/list-item.js

@@ -1,5 +1,12 @@
 import Markerable from './_markerable';
 import { LIST_ITEM_TYPE } from './types';
+import {
+  normalizeTagName
+} from 'content-kit-editor/utils/dom-utils';
+
+export const VALID_LIST_ITEM_TAGNAMES = [
+  'li'
+].map(normalizeTagName);
 
 export default class ListItem extends Markerable {
   constructor(tagName, markers=[]) {

src/js/models/list-section.js

@@ -2,8 +2,15 @@ import LinkedList from '../utils/linked-list';
 import { forEach } from '../utils/array-utils';
 import { LIST_SECTION_TYPE } from './types';
 import Section from './_section';
+import {
+  normalizeTagName
+} from 'content-kit-editor/utils/dom-utils';
 
-export const DEFAULT_TAG_NAME = 'ul';
+export const VALID_LIST_SECTION_TAGNAMES = [
+  'ul', 'ol'
+].map(normalizeTagName);
+
+export const DEFAULT_TAG_NAME = VALID_LIST_SECTION_TAGNAMES[0];
 
 export default class ListSection extends Section {
   constructor(tagName=DEFAULT_TAG_NAME, items=[]) {

src/js/models/markup-section.js

@@ -4,14 +4,14 @@ import { MARKUP_SECTION_TYPE } from './types';
 
 // valid values of `tagName` for a MarkupSection
 export const VALID_MARKUP_SECTION_TAGNAMES = [
-  'p', 'h3', 'h2', 'h1', 'blockquote', 'ul', 'ol', 'pull-quote'
+  'p', 'h3', 'h2', 'h1', 'blockquote', 'pull-quote'
 ].map(normalizeTagName);
 
 // valid element names for a MarkupSection. A MarkupSection with a tagName
-// not in this should be rendered as a div with a className matching the
-// tagName, instead
+// not in this will be rendered as a div with a className matching the
+// tagName
 export const MARKUP_SECTION_ELEMENT_NAMES = [
-  'p', 'h3', 'h2', 'h1', 'blockquote', 'ul', 'ol'
+  'p', 'h3', 'h2', 'h1', 'blockquote'
 ].map(normalizeTagName);
 export const DEFAULT_TAG_NAME = VALID_MARKUP_SECTION_TAGNAMES[0];
 

src/js/models/markup.js

@@ -8,8 +8,7 @@ export const VALID_MARKUP_TAGNAMES = [
   'i',
   'strong',
   'em',
-  'a',
-  'li'
+  'a'
 ].map(normalizeTagName);
 
 export const VALID_ATTRIBUTES = [

src/js/parsers/section.js

@@ -6,6 +6,20 @@ import {
   VALID_MARKUP_SECTION_TAGNAMES
 } from 'content-kit-editor/models/markup-section';
 
+import {
+  VALID_LIST_SECTION_TAGNAMES
+} from 'content-kit-editor/models/list-section';
+
+import {
+  VALID_LIST_ITEM_TAGNAMES
+} from 'content-kit-editor/models/list-item';
+
+import {
+  LIST_SECTION_TYPE,
+  LIST_ITEM_TYPE,
+  MARKUP_SECTION_TYPE
+} from 'content-kit-editor/models/types';
+
 import {
   VALID_MARKUP_TAGNAMES
 } from 'content-kit-editor/models/markup';
@@ -17,9 +31,18 @@ import {
 } from 'content-kit-editor/utils/dom-utils';
 
 import {
-  forEach
+  forEach,
+  contains
 } from 'content-kit-editor/utils/array-utils';
 
+function isListSection(section) {
+  return section.type === LIST_SECTION_TYPE;
+}
+
+function isListItem(section) {
+  return section.type === LIST_ITEM_TYPE;
+}
+
 /**
  * parses an element into a section, ignoring any non-markup
  * elements contained within
@@ -37,17 +60,30 @@ export default class SectionParser {
 
     let childNodes = isTextNode(element) ? [element] : element.childNodes;
 
-    forEach(childNodes, el => {
-      this.parseNode(el, state);
-    });
-
-    // close a trailing text nodes if it exists
-    if (state.text.length) {
-      let marker = this.builder.createMarker(state.text, state.markups);
-      state.section.markers.append(marker);
+    if (isListSection(state.section)) {
+      this.parseListItems(childNodes, state);
+    } else {
+      forEach(childNodes, el => {
+        this.parseNode(el, state);
+      });
+
+      // close a trailing text node if it exists
+      if (state.text.length) {
+        let marker = this.builder.createMarker(state.text, state.markups);
+        state.section.markers.append(marker);
+      }
     }
 
-    return section;
+    return state.section;
+  }
+
+  parseListItems(childNodes, state) {
+    forEach(childNodes, el => {
+      let li = new this.constructor(this.builder).parse(el);
+      if (isListItem(li)) {
+        state.section.items.append(li);
+      }
+    });
   }
 
   parseNode(node, state) {
@@ -141,39 +177,55 @@ export default class SectionParser {
     state.text = '';
   }
 
-  _sectionTagNameFromElement(element) {
+  _getSectionDetails(element) {
+    let sectionType,
+        tagName,
+        inferredTagName = false;
     if (isTextNode(element)) {
-      return null;
-    }
-    let tagName;
-
-    let elementTagName = normalizeTagName(element.tagName);
-
-    if (VALID_MARKUP_SECTION_TAGNAMES.indexOf(elementTagName) !== -1) {
-      tagName = elementTagName;
+      tagName = DEFAULT_TAG_NAME;
+      sectionType = MARKUP_SECTION_TYPE;
+      inferredTagName = true;
+    } else {
+      tagName = normalizeTagName(element.tagName);
+
+      if (contains(VALID_LIST_SECTION_TAGNAMES, tagName)) {
+        sectionType = LIST_SECTION_TYPE;
+      } else if (contains(VALID_LIST_ITEM_TAGNAMES, tagName)) {
+        sectionType = LIST_ITEM_TYPE;
+      } else if (contains(VALID_MARKUP_SECTION_TAGNAMES, tagName)) {
+        sectionType = MARKUP_SECTION_TYPE;
+      } else {
+        sectionType = MARKUP_SECTION_TYPE;
+        tagName = DEFAULT_TAG_NAME;
+        inferredTagName = true;
+      }
     }
 
-    return tagName;
-  }
-
-  inferSectionTagNameFromElement(/* element */) {
-    return DEFAULT_TAG_NAME;
+    return {sectionType, tagName, inferredTagName};
   }
 
   createSectionFromElement(element) {
     let { builder } = this;
 
-    let inferredTagName = false;
-    let tagName = this._sectionTagNameFromElement(element);
-    if (!tagName) {
-      inferredTagName = true;
-      tagName = this.inferSectionTagNameFromElement(element);
-    }
-    let section = builder.createMarkupSection(tagName);
+    let section;
+    let {tagName, sectionType, inferredTagName} =
+      this._getSectionDetails(element);
 
-    if (inferredTagName) {
-      section._inferredTagName = true;
+    switch (sectionType) {
+      case LIST_SECTION_TYPE:
+        section = builder.createListSection(tagName);
+        break;
+      case LIST_ITEM_TYPE:
+        section = builder.createListItem();
+        break;
+      case MARKUP_SECTION_TYPE:
+        section = builder.createMarkupSection(tagName);
+        section._inferredTagName = inferredTagName;
+        break;
+      default:
+        throw new Error('Cannot parse section from element');
     }
+
     return section;
   }
 

src/js/utils/array-utils.js

@@ -130,6 +130,10 @@ function filterObject(object, validKeys=[]) {
   return result;
 }
 
+function contains(array, item) {
+  return array.indexOf(item) !== -1;
+}
+
 export {
   detect,
   forEach,
@@ -143,5 +147,6 @@ export {
   kvArrayToObject,
   isArrayEqual,
   toArray,
-  filterObject
+  filterObject,
+  contains
 };

tests/unit/parsers/dom-test.js

@@ -323,4 +323,67 @@ test('unrecognized attributes are ignored', (assert) => {
   assert.ok(!markup.getAttribute('style'), 'style attribute not included');
 });
 
-// FIXME TODO ul, ol, li, img parsing
+test('singly-nested ul lis are parsed correctly', (assert) => {
+  let element= buildDOM(`
+    <ul><li>first element</li><li>second element</li></ul>
+  `);
+  const post = parser.parse(element);
+
+  assert.equal(post.sections.length, 1, '1 section');
+  let section = post.sections.objectAt(0);
+  assert.equal(section.tagName, 'ul');
+  assert.equal(section.items.length, 2, '2 items');
+  assert.equal(section.items.objectAt(0).text, 'first element');
+  assert.equal(section.items.objectAt(1).text, 'second element');
+});
+
+test('singly-nested ol lis are parsed correctly', (assert) => {
+  let element= buildDOM(`
+    <ol><li>first element</li><li>second element</li></ol>
+  `);
+  const post = parser.parse(element);
+
+  assert.equal(post.sections.length, 1, '1 section');
+  let section = post.sections.objectAt(0);
+  assert.equal(section.tagName, 'ol');
+  assert.equal(section.items.length, 2, '2 items');
+  assert.equal(section.items.objectAt(0).text, 'first element');
+  assert.equal(section.items.objectAt(1).text, 'second element');
+});
+
+test('lis in nested uls are flattened (when ul is child of li)', (assert) => {
+  let element= buildDOM(`
+    <ul>
+      <li>first element</li>
+      <li><ul><li>nested element</li></ul></li>
+    </ul>
+  `);
+  const post = parser.parse(element);
+
+  assert.equal(post.sections.length, 1, '1 section');
+  let section = post.sections.objectAt(0);
+  assert.equal(section.tagName, 'ul');
+  assert.equal(section.items.length, 2, '2 items');
+  assert.equal(section.items.objectAt(0).text, 'first element');
+  assert.equal(section.items.objectAt(1).text, 'nested element');
+});
+
+/*
+ * FIXME: Google docs nests uls like this
+test('lis in nested uls are flattened (when ul is child of ul)', (assert) => {
+  let element= buildDOM(`
+    <ul>
+      <li>outer</li>
+      <ul><li>inner</li></ul>
+    </ul>
+  `);
+  const post = parser.parse(element);
+
+  assert.equal(post.sections.length, 1, '1 section');
+  let section = post.sections.objectAt(0);
+  assert.equal(section.tagName, 'ul');
+  assert.equal(section.items.length, 2, '2 items');
+  assert.equal(section.items.objectAt(0).text, 'outer');
+  assert.equal(section.items.objectAt(1).text, 'inner');
+});
+ */