idea: tree block structure

ohanar commented 8 years ago

So I've been fumbling around with trying to figure out a clean solution to something more or less equivalent to the following problem:

Suppose that I want to create a rich text editor with the ability to create tables in a WYSIWYG fashion. Moreover, I want to have the full power of the rich text editor within each of the cells of my tables, i.e. I might want to bold text, make lists, or even include tables within one of my cells. I'm envisioning being able to richly edit something like this:

This is a paragraph

This is another paragraph

This cell contains a table:

A	B
C	D

This cell contains a list

apples
bananas
pineapples

HELLO

One way in which I could "solve" this problem is to create a custom block component to create the outer table and then nest new instances of the editor within each cell. Beyond being annoying, tedious, and incredibly error prone to write, it also would likely suffer from other strange issues (like trying to select across cells).

I couldn't really think of any other ways I might solve this, without making some fundamental changes to draft's data model, although maybe I'm missing something obvious. Regardless, I still thing the following would be a good idea.

I believe the fundamental issue lies with draft's organization of (content) blocks. In particular, it forces a flat structure to them, when in reality, there are certain instances where you really want a nested structure, namely that of the structure of a tree -- i.e. the same basic model as the DOM.

In the above example, I would envision some tree that roughly looks like

             A leaf block for some text before the table
           /
root block                1st cell block - 1st paragraph block
           \            /                \ 
             table block                   2nd paragraph block
                        \
                          2nd cell block ...

Additionally, it should be possible in this with this model to compose the currently builtin block styles (e.g. you could have a bulleted list inside of a block quote). Also, the whole hacked implementation of lists using depth should also be replaced with nested blocks.

(I have purposely avoided using the exact variable and constant names from draft's source because if a change like the rough one proposed above would be quite significant, and it would probably be an appropriate time to also think about how to make blocks more extensible [e.g. #129], so parts of the api will need changing.)

I would be happy to do large parts of the grunt work to see something like this happen -- I was tasked with starting to prototype a controlled contentEditable a few weeks ago and then draft came along. However, our needs require heavy use of this sort of nested structure.

I would love to get feedback, and if there is interest, I could try to work out a more technical proposal. (I did't really see any part of the contributing guidelines for how I should go about with proposing major changes like this.)

noizex commented 8 years ago

Can't you make the table a rich text object itself with some dedicated editor that allows to create tables as you want? I think flat block hierarchy is the thing that keeps draft.js clean and easier to use, otherwise you're going more into mimicking HTML DOM structure.

noizex commented 8 years ago

Thought that may work for simple data tables - if you want to use content types inside cells that are not pure data but also a list, a paragraph or even another table this may be a bit more complex :/

ohanar commented 8 years ago

To be clear, making tables like this is not what I need, it is just a much simpler example exhibiting the same issues. In my use case, it would terrible UX to have a separate editor for my custom block components.

Also, as you mentioned, having a separate editor makes the composable business annoying (and again, that is also vital to my use case).

ohanar commented 8 years ago

Also, as for making it more complicated -- I don't think that is necessarily the case. Any custom block renderers that you think about designing right now, could still be done -- they just would correspond to leaves in the tree. You should be more or less able to implement them the exact same way as before (there just might be some [hopefully minor] api changes).

noizex commented 8 years ago

I was actually pursuing the same thing (non-HTML rich text editor using React) when Draft.js came out. Looks like this project saved us all a lot of work. It just needs few more features of typical editors, so I'm glad someone started topic about tables, as this is bothering me too. And not only the basic idea how to make similar structure work in Draft, but whole editing process - tables were always the worst thing to work with in WYSIWYG editors, and there is no good intermediate format to store the data too. I'll be happy to see this discussion progress towards some nice and clean solution.

hellendag commented 8 years ago

Certainly open to discussion about this! Thanks for starting the conversation.

These kinds of nested editors are a known shortcoming. They are not something I've tried to solve for because it's not a problem we've needed to solve at Facebook -- all of our use cases involve flat structures, and there was no need to over-model. For features like depth, for instance, it may be a bit of a hack, but it's also completely fine for our current needs.

Now that the project is open source, though, I am happy to talk about re-evaulating the model.

Since blocks are currently tracked within an OrderedMap and contain only plaintext (and characterList) to operate on, it is typically straightforward to add, remove, or otherwise modify blocks within the editor. I think it's a pretty simple model for humans to follow. Trees make this structure a bit more complicated. So, a couple of questions to move the conversation: How do you envision the structure of the block records, and what would an operation like removeRangeFromContentState (for instance) look like under the hood?

mitermayer commented 8 years ago

I also believe that would be nice to have a way to represent a tree structure model, tables are still a largely used feature on rich text editors. Also during the model design we should also keep in mind on how can we have a model that would be compatible with OT operations envisioning a possible future collaborative editing compatibility, since that could be a challenge on itself.

ohanar commented 8 years ago

I'm not really familiar with the issues of collaborative editing, so please feel free to chime in if something will be an obstructor.

I'm going to refer to the proposed block structures as Blocks rather than ContentBlocks to avoid confusion with the current code (in the future, I would probably call these TextBlocks or more likely RichTextBlocks).

For the Block structure, I think that I would want to keep it very simple, something along the lines of a record of the following form

{
  // serves the same purpose as the current key
  key: string;

  // similar to the current types, however I would allow this to be user extensible
  type: string;

  // children of the Block
  children: ?OrderedMap<string, Block>

  // extra data that the Block might need
  data: ?Object; // some type depending on the property type
}

A few of examples of what records might look like for some builtin Block types

// the current 'unstyled' type
{
  key: string;
  type: 'richtext'; // I find 'unstyled' a bit misleading
  children: null;
  data: {
    text: string;
    characterList: List<CharacterMetadata>;
  }   
}

// the current 'blockquote' type would have a single 'richtext' child
{
  key: string;
  type: 'blockquote';
  children: OrderedMap<string, Block>;
  data: null;
}

// the current contents of a ContentState would now be contained in this type of Block
{
  key: string;
  type: 'plain-container';
  children: OrderedMap<string, Block>;
  data: null;
}

This is based redux's action types (although they use payload instead of data), basically I would like to allow a developer to pass along whatever extra info they would like, although we do need to keep track of the tree structure, so by necessity we have a children property.

Because I'm using OrderedMaps to hold the children of a node, we get a total ordering on the Blocks in the tree structure using a pre-order depth first traversal. This ordering should agree with order the ordering of the DOM so that we aren't fighting with native selections.

For the removeRangeFromContentState function, I would introduce a new function removeRangeFromBlock, and which removeRangeFromContentState would just call on the root Block. The default behavior of this new function would be the following:

For any child Block that is completely contained within the range, delete it.
Apply this function recursively to any children that still intersect the range with appropriately restricted ranges.
If there are two children for which you applied step 2 (there were at most two), try to merge these two children. If unable to, don't loose any sleep over it.

This should mimic the behavior of the current function, however, it isn't exactly what you want with a table (there I would expect that the children that are completely contained within the range are simply cleared, rather than deleted -- as otherwise you would get a shifting behavior with the cells). So I would introduce an api to allow a developer to specify how a range should be removed for a given block type (and I would try to use this api as much as possible internally -- namely, I would implement removing ranges in RichTextBlocks using this api).

My guess is that many, if not most, modifiers that deal with selections and ranges should be exposed in such an api, but I'm not sure what the best method for doing so would be (I'm not particularly fond of the blockRendererFn as an api, but I don't really have a better proposal at the moment).

hellendag commented 8 years ago

I think this is a pretty reasonable approach. Would you be interested in creating a fork to begin experimenting with it? I'd be happy to take part.

Regarding table behavior with respect to deletion, it might make sense to find a way to define consistency rules. Then invalid structures could be rejected without being propagated, with messaging in DEV to explain why.

(I'm not particularly fond of the blockRendererFn as an api, but I don't really have a better proposal at the moment).

Yeah, it's hard to determine how to expose the ability to inject custom components. It doesn't seem better to define a map from type to custom component definitions, since type may be an insufficient value to branch on -- the function gives us more flexibility there.

ohanar commented 8 years ago

Sure thing. I'll put up a link here once I have something (probably won't be for a few days).

ohanar commented 8 years ago

I've started experimenting with this here: https://github.com/ohanar/draft-js/tree/block-tree. Right now there is not much there, I've only done the lowest hanging fruit -- added a basic Block class with some convenience methods. I'll next be seeing about refactoring the content components to take advantage of the tree structure.

thesunny commented 8 years ago

When developing this feature, one suggestion is to ensure that the inner blocks can have a different configuration than the top level ones.

The reason is that often, you don't want a full set of editing capabilities within the children blocks.

For example, in a Github Flavored Markdown compatible editor that allows tables, the table cells themselves cannot have inner tables. It would be nice if the editor supported this limitation as well.

Another example is multi-column layouts. In most cases, you wouldn't want to support a two-column layout with a two-column layout inside (nested columns). Once you start allowing infinite recursion of editors, you come closer to ending up with a mess of dealing with the DOM.

In case you haven't seen it already, there has been an attempt at using the editor as it currently is to support child editors:

https://getpocket.com/a/read/1214871942

With a nice demo here:

https://draft-wysiwyg.herokuapp.com/

Might provide some ideas for a native implementation.

Finally, I think in an ideal implementation of children blocks, you could even provide different configuration within a block. This is a bit contrived but consider an advanced image gallery block. Each image has a description which has its own editor but is very limited because it's supposed to just be like a caption (but perhaps with bold, italic and link support). The description on the gallery itself has the ability to contain a full description though with all the formatting including tables, lists, etc.

My suggestion would be to make the child blocks have String keys instead of using a list. I think it would support more useful scenarios.

mitermayer commented 8 years ago

@ohanar, What's the current state of this issue? I believe in order to properly being able to add support to html tables we would need tree block structures

ohanar commented 8 years ago

@mitermayer , sorry something unexpected came of for me. I'm hoping to get back to it later this week or early next week. You are welcome to help out if you are up for it.

SamyPesse commented 8 years ago

@ohanar I'll be happy to contribute to such a PR.

mitermayer commented 8 years ago

Hi @ohanar being thinking a lot about this issue, and I agree with all the proposed changes by you.

I think this issue is of extremely importance for the future of Draft-js however in order to get it sooner rather then later into production we need to come up with a solution that involve a reduced scope of changes and aiming to provide backwards compatibility.

In a very high level we could try something like:

1 - Update the current data model to include the minimum necessary functionality required to enable the block tree data model, also making sure we do not modify existing ones. 2 - Assuming we now have the tree model, we should treat it as flatten by default (treating all blocks as leafs) unless explicit enabled via user configuration, this way applications already using DraftJS will maintain it's current behaviour 3 - We also need to make sure the new data model is backwards compatible with already saved raw data (this shouldn't be too hard).

Assuming we can come up of a way to introduce tree data structure, make it being an opt-in experience (by default we serve the flatten model by not allowing block level children and adding them as leafs) and without breaking existing experience I believe would make a lot easier to get it merged.

what are your thoughts ? @ohanar @hellendag

SamyPesse commented 8 years ago

@mitermayer I've started some experimentation the other day (https://github.com/facebook/draft-js/compare/master...SamyPesse:feature/tree). The goal was to keep the current data modelling and just add an optional blockList property to ContentBlock, to not be a breaking change:

var baseContent = convertFromRaw({
  blocks: [
    {
      type: 'unstyled',
      text: 'Hello World',
      blockList: [
        {
          type: 'unstyled',
          text: 'Cool'
        },
        {
          type: 'unstyled',
          text: 'Awesome'
        }
      ]
    }
  ],
  entityMap: {}
});

My branch is far from working. I'll maybe keep experimenting on this.

mitermayer commented 8 years ago

@SamyPesse I am happy to work together with you on that branch if you don't mind. I really want to get this issue resolved. :)

SamyPesse commented 8 years ago

@mitermayer Awesome, I've added you as a collaborator, it might be a good idea to open a PR and move the discussion there.

Concerning the implementations, I think the high level steps should be:

[x] Add blockList to the ContentBlock model, and supports it in decode/encode with RawContentState
[ ] Modify the react components to display the tree (no support for editing)
[ ] Modify the modifier to work with the tree of blocks instead of just a flat list Currently blocks have a unique identifier key used in SelectionState, etc. Maybe we should consider a new ID for blocks: path in the tree (like <id root block>/<id sub block 1>/<id sub block 2>), so that modifier can edit a deep block using only its ID (the identifier will remain compatible with the current implementation).
[ ] Adapt the handlers to support deep blocks

Let me know what are your thoughts, I'm still new to draft and its codebase, maybe I'm missing something important.

mitermayer commented 8 years ago

me and @SamyPesse are working on this issue, we already have a working prototype, hopefuly by next week we will be able to wrap the PR https://github.com/facebook/draft-js/pull/388, in the meantime we are tracking issues and remaining tasks on https://github.com/SamyPesse/draft-js/issues, if anyone is keen on contributing please let us know.

mitermayer commented 8 years ago

For everyone that is interested this PR is almost ready https://github.com/facebook/draft-js/pull/388

Remainder of the tasks are:

[ ] - Refactoring / Adding coverage ( https://github.com/SamyPesse/draft-js/issues/16 )
[ ] - Adding documentation ( https://github.com/SamyPesse/draft-js/issues/9 )

Please guys feel free to check this out and beta test it. All feedback is welcome!

The demo for it is under examples/tree/tree.html

This PR is backwards compatible with the previous data structure model and the nesting is configurable allowing the application layer to decide if want to support nesting or not. We also moved it to experimental so that would allow us to do tweaks and move this a bit faster.

Please let us know what you guys think about it

mitermayer commented 8 years ago

@hellendag @ohanar please have a look into it and share your thoughts !

mitermayer commented 8 years ago

I am back on working on this issue, i am taking over the PR (https://github.com/facebook/draft-js/pull/388) from @SamyPesse and implementing the feedbacks from @hellendag on (https://github.com/facebook/draft-js/pull/388#issuecomment-230168901). PR is a bit outdated so will now start by rebasing. Sorry for the long wait guys have been unable to work on it for the past few months

kevinguard commented 7 years ago

@mitermayer Thanks for the great news. Do you have any ETA for completion? I was going to patch the PR to a local copy to unblock myself on a personal project but if I know this will come out soon, I will hold off a little bit and wait to have a well-tested and clean code for it. Appreciate that!

DanKottke commented 7 years ago

@mitermayer I'm also be curious how this is going. The flatness of the content block scheme is blocking me as well at the moment, and it looks like I'm going to have to run with this PR instead.

chrisportela commented 7 years ago

An update on #388 for those coming along: https://github.com/facebook/draft-js/pull/388#issuecomment-274204865

They are still experimenting with this internally, but discussion will be continuing here.

kevinguard commented 7 years ago

@0xCMP Appreciate for the note. @DanKottke Yes, I am going to do this next week. I recall I tried patching this PR several months ago and there were quite a few difficulties I was facing, though, please let us know if you have seen something challenging enough to kill a day or so from you when patching this PR so we can share our observations here for other who want to try this out too. I will do too! Thanks!

againksy commented 6 years ago

Where can i find the example of table editor, whre i can edit each table ? so each table contain draft.js editor

almaron commented 6 years ago

So any doc I can follow on how to use this tree-structure?

crispiestsquid commented 3 years ago

Here I am in 2021 just finding this, and sad to see that we still cannot use tables in draft-js... Anyone have a way to work around this?

VivekNeel commented 2 years ago

I would love to have this table support. Anyone figured out this yet? Please let me know

facebookarchive / draft-js

idea: tree block structure #143