Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TinyMCE per block: Adding the block inserter menu #196

Merged
merged 1 commit into from
Mar 7, 2017

Conversation

youknowriad
Copy link
Contributor

No categories or keyboard navigation for now but it's good enough for a V1

@youknowriad youknowriad added the [Type] Enhancement A suggestion for improvement. label Mar 7, 2017
@youknowriad youknowriad self-assigned this Mar 7, 2017
@@ -72,5 +72,13 @@ registerBlock( 'image', {
attrs: { /* caption: block.caption ,*/ align: block.align },
rawContent
};
},
create: () => {
return {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I've toyed in my other recent prototypes. Would it be worthwhile to try to reuse logic from parse, and force the implementation to accommodate for an "empty" block to parse? I think there's a clearer need in my case where I have to ensure a common markup structure between creating and preparing an existing node for edit, though I quite like that it avoids breaking the saved <-> edited mode duality when adding a third case for creation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure yet. I thought about it, but there's also the transform case, which I think is a better candidate to be merged with the create. We can see the create as a special case of transform with an empty content.

By transform, I mean switching block types, And I was thinking about like:

const rawContent = oldBlock.content; // (some way to extract content from a block)
const newBlock = newBlockDefinition.create( rawContent )

What do you think?

Copy link
Member

@aduth aduth Mar 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, transforming is tricky 😄 I'd put some thought into it, but didn't make a whole lot of headway. One problem is trying to find a common "base content" type (rawContent in your snippet). Is it a string? How do blocks declare whether they can support transforming?

I'd also wondered since we'll need to preserve backwards compatibility, and perhaps to avoid treating paragraphs as formal blocks, a block could register patterns that it can identify and be instantiated from.

Some rough pseudo-code:

match: [
	{
		pattern: wp.shortcode.regexp( 'gallery' ),
		// RegExp -> Element
		initialize: function( match ) {}
	},
	{
		selector: 'p',
		// NodeList -> Element
		initialize: function(  ) {}
	},
	{
		block: 'foo',
		// WPBlock -> Element
		initialize: function() {}
	}
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm, nice idea I was hoping to be able to use only the WPBlock -> WPBlock initialization flow but this introduces some coupling between the source and the destination block.


Back to the create problem above. As you said, this is probably mergeable with parse. I have a personal preference over having it separate instead of handling an empty block as input. We could reconsider later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a personal preference over having it separate instead of handling an empty block as input. We could reconsider later.

That's fine, let's see how it works out 👍

@youknowriad youknowriad merged commit e60a2e8 into master Mar 7, 2017
@youknowriad youknowriad deleted the add/per-block/inserter branch March 7, 2017 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants