-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
RichText: Extend the AlignmentToolbar #14824
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { dispatch, select } from '@wordpress/data'; | ||
|
||
/** | ||
* Registers a new custom alignment type provided a unique name and an object defining its | ||
* behavior. | ||
* | ||
* @param {string} name Custom alignment name. | ||
* @param {Object} settings Custom alignment settings. | ||
* @param {string} settings.blockName The block name this custom alignment will be added to the Rich Text alignment toolbar. | ||
* @param {string} settings.align The alignment setting. | ||
* @param {string} settings.title Name of the custom alignment. | ||
* @param {string} settings.icon The icon to be displayed in the alignment toolbar. | ||
* | ||
* @return {Object|undefined} The Custom alignment, if it has been successfully registered; | ||
* otherwise `undefined`. | ||
*/ | ||
export function registerCustomAlignmentType( name, settings ) { | ||
settings = { | ||
name, | ||
...settings, | ||
}; | ||
|
||
if ( typeof settings.name !== 'string' ) { | ||
window.console.error( | ||
'Custom alignment names must be strings.' | ||
); | ||
return; | ||
} | ||
|
||
if ( ! /^[a-z][a-z0-9-]*\/[a-z][a-z0-9-]*$/.test( settings.name ) ) { | ||
window.console.error( | ||
'Custom alignment names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-alignment' | ||
); | ||
return; | ||
} | ||
|
||
if ( | ||
typeof settings.blockName !== 'string' || | ||
settings.blockName === '' | ||
) { | ||
window.console.error( | ||
`Custom alignment block name must be a string.` | ||
); | ||
return; | ||
} | ||
|
||
if ( ! select( 'core/blocks' ).getBlockType( settings.blockName ) ) { | ||
window.console.error( | ||
'Custom alignment block "' + settings.blockName + '" must be registered.' | ||
); | ||
return; | ||
} | ||
|
||
if ( select( 'core/rich-text' ).getCustomAlignmentType( settings.name, settings.blockName ) ) { | ||
window.console.error( | ||
'Custom alignment "' + settings.name + '" is already registered.' | ||
); | ||
return; | ||
} | ||
|
||
if ( ! ( 'title' in settings ) || settings.title === '' ) { | ||
window.console.error( | ||
'The custom alignment "' + settings.name + '" must have a title.' | ||
); | ||
return; | ||
} | ||
|
||
if ( ! ( 'icon' in settings ) || settings.icon === '' ) { | ||
window.console.error( | ||
'The custom alignment "' + settings.name + '" must have an icon.' | ||
); | ||
return; | ||
} | ||
|
||
if ( ! ( 'align' in settings ) || settings.align === '' ) { | ||
window.console.error( | ||
'The custom alignment "' + settings.name + '" must have an alignment setting.' | ||
); | ||
return; | ||
} | ||
|
||
dispatch( 'core/rich-text' ).addCustomAlignmentTypes( settings ); | ||
|
||
return settings; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd personally love us to move away from these Another downside of this approach is that if you disable the custom alignment, this might create invalid blocks in some cases (depending on how the alignment is applied to the block). And I think we should discourage the extensibility APIs that lead to this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The use-case would be to allow custom alignment options to be added to the alignment toolbar. Now, this is an exploratory PR opened by someone with close-to-zero familiarity with how extending Gutenberg works, so comments like yours about the risk of breaking a block are absolutely appreciated! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I guess, it means you're lucky to have early feedback. About the Also, you're mixing both block and text alignment in the comments while it seems that at the moment, this PR is addressing text alignments only. I wonder if this should be just another editor setting. The same way we have "colors" and "fontSizes" in the editor settings, we could include "blockAlignments" and "textAlignments". The block alignment option would be a little be redundunt with the "alignWide" setting but we can probably ensure BC. Thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that this is a draft PR but I'd love to stress how valuable some inline documentation right here can be. people will look here to try and find the answer "what information is necessary to define an alignment type?" (if only we had some way to designate types and have the computer answer the question for us…)
looking through the whole PR I'm able to discern that it needs a
name
, atype
, andcontrol
but I still don't know what those properties should be.we could consider adding a JSDoc typedef
even without the typedef though having an example use can clear up so much confusion, as can enumerating what properties of
settings
may exist and what they meanThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out @dmsnell!
That file is a very stripped down version of register-format-type.js.
It doesn't have any docs, validations, etc, because this exploration is more about extending the toolbar than handling this registration.
I will add everything (and more!) back as soon as I get the extension part right. 👍