Conversation
youknowriad
left a comment
There was a problem hiding this comment.
Aside the default as h2, this LGTM 👍 for a v1 of the heading block.
Now, we have some work to do on the Editable side to ensure we can not create paragraphs etc... (but this is transversal to different blocks, same for text for example)
editor/blocks/heading-block/index.js
Outdated
There was a problem hiding this comment.
I would use h2 by default to keep h1 for the post title.
editor/blocks/index.js
Outdated
editor/blocks/heading-block/index.js
Outdated
There was a problem hiding this comment.
Why not type={ headingType } as the prop?
There was a problem hiding this comment.
Yeah, I went back and forth on that. Technically the value for the property is the nodeName and not the tag value. So I left it as nodeName. I basically just copied what aduth wrote because it looked good lol.
62a40be to
570b744
Compare
|
We will need some way to change the |
|
I think we should try to have |
|
@iseulde Sounds like a potentially better solution. From my understanding, that would mean not using TinyMCE for the heading block? What will we lose out on if we use
instead? |
|
Hey, Have you considered making tinymce use the heading itself as the target container, rather than embedding a div inside the heading? I see at the moment that the Editable react component always creates a div and uses that as the rendered element, but you could make its node name configurable. Then you wouldn't have to worry about If you want an example of using a heading tag for an inline tinymce instance, see the examples here: https://www.tinymce.com/docs/demo/inline/ |
|
@ephox-mogran What are the pros of this approach compared to always using a |
|
It gives tinymce more context for the various operations. With differently structured blocks (like ol and ul), it prevents the insertion of a div tag from causing problems with the HTML structure. Primarily, it means that tinymce is given more information to make decisions, and more of the finer formatting details can be delegated to tiny. When a tinymce instance is created on a target node, it's able to use that node to govern how it should act (with regards to Enter key behaviour etc). If you are embedding tinymce inside something and then trying to give tinymce restrictions based on information that you know but it doesn't, it's probably going to get a lot more complicated. |
|
@ephox-mogran Thanks for the explanation, Good points here. I guess the more we build blocks (heading, quotes, lists, text ...), the more we'll have context on whether we should do this or not. (Especially when we'll introduce keyboard interactions and HTML restrictions) |
|
See #327 (comment) for relevant changes to |
|
Closing in favor of #423 |
Accidentally made a mess of git here is a clean version adding in JSX and making use of t he Editable component.