Rich text formatting filter

[ellatrix]

Description

See also #4658.

Related PRs: #9192.

This small change makes the formatting controls for all rich text instances extensible. E.g. a plugin wants to add an extra button to add a "code" tag:

addFilter( 'editor.richText.formattingControls', 'core/test', ( controls ) => {
	return [
		...controls,
		{
			icon: 'editor-code',
			title: __( 'Code' ),
			shortcut: displayShortcut.primary( 'x' ),
			format: 'code',
		},
	];
} );

For the moment it's limited to adding formats, but I could imagine an option to insert custom HTML as well. Let's start simple. :)

Questions:

• What's the right place to call applyFilters? At the top level, plugins will need to add the filter before the Gutenberg scripts run. At render it will be run too many times. Should it be run on constructing FormatToolbar?
• If we keep DEFAULT_CONTROLS, we this might need a filter too, but it does not make sense without context. RichText is not aware of the block using it. It could also be the tag, but not sure if this makes any sense. Alternatively we could get rid of DEFAULT_CONTROLS or invert the behaviour, e.g. a button would declare controls to omit rather then the ones to display.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[gziolo]

• What's the right place to call applyFilters? At the top level, plugins will need to add the filter before the Gutenberg scripts run. At render it will be run too many times. Should it be run on constructing FormatToolbar?

Ideally, at any time to make sure it also works in the case when plugin code is executed after an editor has intialized :) Initial implementation can take a simpler assumption and apply filters when formatting toolbar renders for the first time. The easier way to implement it is to convert FORMATTING_CONTROLS into a function and wrap it with once utility from lodash. This way you ensure all the filters are computed only once.

• If we keep DEFAULT_CONTROLS, we this might need a filter too, but it does not make sense without context. RichText is not aware of the block using it. It could also be the tag, but not sure if this makes any sense. Alternatively we could get rid of DEFAULT_CONTROLS or invert the behaviour, e.g. a button would declare controls to omit rather then the ones to display.

Can we assume that when enabledControls are not provided, all controls are allowed? It seems to be the case at the moment, anyway.

[gziolo]

For consistency, we should name this filter editor.RichText.formattingControls.

[youknowriad]

How can we make this tweakable per block? I want to add a button to the Paragraph and Heading but not the captions of images etc...

[gziolo]

You can also check how autocompleters were made extensible by @brandonpayton using a higher-order component in here: https://github.com/WordPress/gutenberg/blob/master/editor/components/autocomplete/index.js. It is much more complex use case, because every autocompleter needs to keep its state independently.

How can we make this tweakable per block? I want to add a button to the Paragraph and Heading but not the captions of images etc...

BlockEditContext is your friend, blocks.Autocomplete.completers filter takes blockName as 2nd param. See https://github.com/WordPress/gutenberg/blob/master/docs/extensibility/autocomplete.md#example :)

function appendAcronymCompleter( completers, blockName ) {
	return blockName === 'my-plugin/foo' ?
		completers.concat( acronymCompleter ) :
		completers;
}

[grappler]

Where are the formats defined that could be used?

[ellatrix]

How can we make this tweakable per block? I want to add a button to the Paragraph and Heading but not the captions of images etc...

I'm not sure about this... There will be blocks with multiple RichText fields, so what if you only want them enabled on one field? Maybe we should allow two ways of registering control?

• You can register controls globally for RichText with the filter. This will add them to all RichText instances.
• You can pass additional controls to the RichText instance within the block registration.

Alternatively we could pass the block name and the tag name as arguments on the filter.

[gziolo]

Yes, the more params we provide the better control plugin developers will get. I like the idea of passing a tag name 👍

[ellatrix]

Where are the formats defined that could be used?

They are defined here: https://github.com/tinymce/tinymce/blob/10534f7dead4bb79bf8a98d91c2465e1ee652b66/src/core/main/ts/fmt/DefaultFormats.ts

But this is very limited. Eventually I think it should be possible to pass a function to call on the selection, and return a React element.

[gziolo]

Rebased with master. I think we will need two filters:

• editor.RichText.formattingControls - which will allow to customize the list of the existing formatting controls (add/update/remove)
• editor.RichtText.enabledFormattingControls - which will allow to customize set of controls per block type, the idea is that to render control it would have to be registered as formattingControl and present in this array.

I'm AFK for the rest of the week so won't have chance to work on it until Monday.

[noisysocks]

While working on #8807 I was thinking how nice it would be if our Link button was entirely implemented using the formattingControls API that we expose to third parties.

I would especially love if the API were flexible enough that we could remove our complex link formatting logic out of RichText and into the link formattingControls object.

Rough and not-very-thought-out pseudocode of what I'm getting at:

{
	icon: 'admin-links',
	title: __( 'Link' ),
	shortcut: displayShortcut.primary( 'k' ),
	format: 'link',
	attributes: {
		href: { type: 'string', source: 'attribute', attribute: 'href', default: null },
	},
	renderAdditionalUI( format, setFormat ) {
		return <LinkContainer format={ format } setFormat={ setFormat } />;
	},
	beforeApply( attributes, editor ) {
		if ( isURL( editor.getSelection.getText() ) ) {
			return { href: editor.getSelection().getText() };
		}
		return attributes;
	},
	apply( attributes, editor ) {
		if ( attributes.href ) {
			if ( editor.getSelection().isCollapsed() ) {
				editor.insertContent( editor.dom.createHTML(
					'a',
					{ href: attributes.href },
					editor.dom.encode( attributes.href )
				) );
			} else {
				editor.execCommand( 'mceInsertLink', {
					href: attributes.href,
					'data-wp-placeholder': null,
					'data-mce-bogus': null,
				} );
			}
		} else {
			editor.formatter.apply( 'link', {
				href: '#',
				'data-wp-placeholder': true,
				'data-mce-bogus': true,
			} );
		}
	},
	remove( attributes, editor ) {
		editor.execCommand( 'Unlink' );
	},
},

[jcklpe]

ooooh, sorry to chime in. I'm just following along through all the github issues on this subject, reading how you guys are solving the issue and this is really clever. I mean I'm still a novice at code but this solution sounds really nice and flexible. Keep up the good work guys!

[ellatrix]

I'm going to close this as I think #10068 is the way forward. Feel free to comment with thoughts on it. This may address some concerns @noisysocks has regarding the flexibility of the API.