Add "inline" prop to Editable component

[aduth]

This pull request seeks to add a new inline prop to the Editable component, used in controlling the forced_root_block setting. With these changes, it's used to prevent the image's figcaption from including paragraphs, assuming instead only inline nodes are allowed.

Open questions:

What should happen when pressing Enter in an image's caption? Should it allow line breaks, or create a new text block after the image block? Is this specific to the Image block, or should this be a general behavior for any inline Editable?

@youknowriad I'm recalling a recent discussion where you'd mentioned accepting an inlineFormattingControls toolbar to the Editable component. Should that be automatically inferred from this prop perhaps?

Testing instructions:

Verify that pressing Enter in an image's caption results in line breaks, not paragraphs, at that there are no paragraphs in the serialized figcaption when changing to the Text view.

1. Click image block
2. Add caption, including at least one line break
3. Switch to Text mode
4. Search for the figcaption element of the image block you modified
5. Verify there are no <p> tags

[ellatrix]

Works great!

I wonder if the footer in block quote should be the same.

What should happen when pressing Enter in an image's caption?

#579

[ellatrix]

I'm getting the the key warning now as well for the caption. Do you think I should just assign keys in nodeListToReact?

[youknowriad]

I'm recalling a recent discussion where you'd mentioned accepting an inlineFormattingControls

Should we use the inline here for the heading block too? If so I think inlineFormattingControls should be a separate prop.

[aduth]

Should we use the inline here for the heading block too?

Hmm, I see what you're saying in the sense that the heading block shouldn't allow paragraph roots. In practice, this doesn't occur though; I'm assuming TinyMCE is preventing the paragraphs as descendants of the heading, but not here with a figcaption element.

Broadly speaking, I'd agree it probably makes sense that Heading's Editable be flagged as inline, but also that it's inline formatting controls be shown in the toolbar (your suggestion of a separate inlineFormattingControls prop).

[aduth]

I'm getting the the key warning now as well for the caption. Do you think I should just assign keys in nodeListToReact?

I'm going to take a closer look at these warnings today. I'm not sure yet exactly how we'll want to address.

[aduth]

Reflecting on the conversation here, I think considering <Editable /> agnostic its implementation, it's sensible that all of our current Editables should be treated as inline with exception of: text, quote's content, and list. This is now reflected in the rebased 76d62e7.

There are some implications now on blocks which previously wouldn't have had an Enter newline behavior now inserting a <br> line break. I think this is consistent with what we'd expect, but we also might consider adding more control to this in the future.

[ellatrix]

Hm, do we want line <br> in headings?

[aduth]

Hm, do we want line <br> in headings?

I alluded to this in my previous comment; probably not, but I think we'll want to expand on this separately by exposing more control to the block to define its newline behavior. Given this, I still think the Editable input itself makes most makes semantic sense as an inline field, child of the root heading element.

[youknowriad]

Just thinking out loud, maybe to better differentiate the different possibilities of the Editable component we should create Aliases: InlineEditable (with inline and inlineFormatting by default), MultiParagraphEditable ...)

[aduth]

I'm generally fine with props controlling the behavior, but I'd agree the distinction between inline and inlineFormattingControls is a little fuzzy, and could do for some developer experience improvements. Not positive this means separate components, vs. better consolidation of the two props.

[aduth]

I'm getting the the key warning now as well for the caption. Do you think I should just assign keys in nodeListToReact?

I've been looking at this some more, and I think assigning keys is a reasonable approach. html-to-react does the same, simply assigning the index of the child. Given that we have a DOM node, we could do something more extreme to try to help React with reconciliation, like assigning an internal property to the node with a unique ID that we prefer over just the child index.

[ellatrix]

I've been looking at this some more, and I think assigning keys is a reasonable approach.

Yeah. What about further down the tree? Should they all have keys? We're not taking advantage of reconciliation, so I guess not?

[aduth]

Should they all have keys?

Yes, but I don't think the keys need to be globally unique; it should be enough that they're assigned an index based only off their immediate parent.

I do think assigning a preferred property to the DOM node is worth considering, in case we call nodeListToReact with the same element references multiple time. I can put together an example of what I mean if it'd help clarify.