Use the same wrapper for both ordered and unordered list. Maintain bl…

[ericbiewener]

Summary

Adds support for nesting ordered & unordered lists within one another, preserving the block depth when toggling between the two. This fixes #443 in the process.

The enhancement is simple. Since DraftJS already uses classes in combination with the ::before pseudo element to custom render ordered list numbering rather than depending on default browser functionality, there isn't actually any reason to render an <ol> vs <ul>. By giving both block types the same wrapper, we automatically gain support for nesting one within the other.

To maintaining block depth with toggling between the two list types, we simply merge in the block's current depth rather than 0.

Test Plan

Open up the rich.html example and create an ordered list. Indent a list item and convert it to an unordered list. It will remain nested at the same depth. Play around further, adding ordered and unordered lists throughout the list tree. Ordered list numbering will remain correct, restarting at new levels of depth and continuing from the last ordered list item at the same depth.

[facebook-github-bot]

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired.

Before we can review or merge your code, we need you to email [email protected] with your details so we can update your status.

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[davidchang]

thanks for the article @ericbiewener - this actually looks all right to me.

can someone else take a look? @flarnie @stopachka @tylercraft @hellendag?

[mzbac]

any update for this? really looking forward this fix

[DawnWright]

@ericbiewener I just took this fix for a spin, and noticed that it breaks copy-paste of lists from one editor into another: all ordered lists become unordered upon paste. This is because Draft pastes from HTML, and with your changes <ul> is used to wrap both ordered and unordered lists. So when it's converted back to blocks, all the <li> elements are assumed to be unordered items.
https://github.com/facebook/draft-js/blob/a399ea9ce7f5a4f6aa123b5937a7815f4e4313d4/src/model/encoding/convertFromHTMLToContentBlocks.js#L146-L154

[flarnie]

Thanks for making this PR! I like the idea of fixing #443 but would prefer the approach Isaac suggested, of just not resetting the depth when the block type changes from one list to another.

It doesn't make any visual difference to use an ol vs. ul but it has the potential to create inconsistencies like the one @DawnWright pointed out, and I'd also prefer we use semantic HTML when possible. That might also benefit things like screen readers, or styles folks have added to their editors on top of Draft.js.

Please feel free to open a PR with a different approach.