Closed gziolo closed 4 years ago
Related comment from @youknowriad left in https://github.com/WordPress/gutenberg/issues/6023#issuecomment-485372463:
Ultimately I agree that we need a good API to toggle features in the editor. We have a precedent here with
disable-custom-font-sizes
anddisable-custom-font-sizes
but I don't think is going to scale properly if we keep adding these options per "feature".I think a good way to think about this might be to try to be more "generic" and "semantic" instead of thinking specifically about given features of particular block types or thinking about specific UI elements (inspector/block controls)
For example, the editor settings could say "colors are disabled" and block authors would have the possibility to understand this setting and implement it properly. I think a path forward would be to try to find these "generic/global configs". A rough list could be:
- Colors
- Font Sizes
- Image manipulation
- Advanced typography (dropcap)
- ...
Based on this the
editor settings
and the existing editor settings filter https://developer.wordpress.org/reference/hooks/block_editor_settings/ could be a good solution here
Prompted by the suggestion of exploring React hooks as a possible implementation here, it should be considered that most of these behaviors share some common characteristics as augmenting the underlying block:
alignment
edit
implementation
save
implementation
alignleft
, etc. alignment classesNotably, the need to define a custom attribute for these behaviors poses a challenge to use React hooks in that hooks are specific to the client-side JavaScript implementation, whereas attributes definitions are being explored as defined in a context-agnostic fashion #13693.
In the related #8171 (maybe ought to be included in original issue comment), I proposed some idea of an "abstract block type" at https://github.com/WordPress/gutenberg/issues/8171#issuecomment-480942157 , including some very rough pseudo-code. The idea of an abstract block type being that the above characteristics are common to any block, but an abstract block type presumes that (a) it cannot be used standalone and (b) is inherited into the implementation of an extending block type.
registerBlockType( 'core/colorable', {
abstract: true,
attributes: {
color: {
type: 'string',
},
},
edit: () => (
<InspectorControls>
<PanelColorSettings /* ... */ />
</InspectorControls
),
save( { children, attributes } ) {
const child = Children.only( children );
return cloneElement( child, {
style: {
...child.props.style,
color: attributes.color,
},
} );
},
} );
registerBlockType( 'core/paragraph', {
uses: [ 'core/colorable' ],
// ...
} );
A more involved example is also the media placeholder/media upload functionality present on a number of blocks (audio, image, video, file ...). Most of these blocks roughly do the same thing, but with slight variations (accepted file types, attribute names), but the implementations are mostly separate and have diverged.
As seen in https://github.com/WordPress/gutenberg/pull/14918, when there's an attempt to bring a consistent improvement to these blocks it involves a lot of work.
I've been thinking about this recently and I wanted to summarize my current thoughts.
If we take a look at how colors or alignments are applied in some blocks, you'd see that there are big differences in terms of markup and which elements gets updated using these attributes. This makes it clear for me that we need to distinguish two things.
1- Common Functionality we can apply in a generic way without knowledge of the internals of the block: This applies to some of our features like "custom classNames", "anchor" and even "align" in some blocks.
2- Common Functionality that doesn't apply seamlessly across blocks due to markup differences... I'd refer to this as Common Code
I think both of these two problems need solving but I think each one needs a specific solution:
1- Common Functionality (aka block extensions)
Right now, this is achieved using the support property and extensibility is allowed using the registerBlockType filter. The current approach suffers from three main issues for me:
1- Timing issues: It's very hard for third-party developers to register their extensions with the correct dependencies, at the correct moment.
2- Validation: We all know that updating the save
function leads to invalidation if the "extension" is removed.
3- Consistency between these extensions.
To solve this, here's a rough proposal:
const blockExtension = options => ( {
attributes: ( attributes ) => ({
...attributes,
anchor: {
type: 'string',
source: 'attribute',
attribute: 'id',
selector: '*',
},
}),
edit: ( BlockEdit ) => props => {
// Extend BlockEdit
return <BlockEdit {...props} />
}
} );
wp.data.dispatch( 'core/blocks' ).addBlockExtension( 'anchor', blockExtension );
wp.data.dispatch( 'core/blocks' ).attachExtensionToBlocks( 'anchor', [ 'core/paragraph' ], options );
anchor
in the example aboveNow the remaining issue is the "block validation". The reality is that some extensions/plugins don't care about invalidating blocks if disabled. Which means we can support save
in the block extension but also raise a flag if used.
Block Extensions can have a server-side version to allow defining a render_block
callback as an alternative to the save
to avoid the validation issues.
2- Common Code (aka block extensions)
This is an area that seems easier to solve, at the moment we used to have Higher-order components (withColors) and also components. I think we could invest a little bit more in the React Hooks as a way to encapsulate functionality but staying flexible. The current way we apply colors requires a lot of boilerplate, and I'd love to see this explored. (We could provide two hooks for each pattern: one for edit
and one for save
)
I have an idea around the validation problem, but it's not well formed and potentially complicated.
The essence is that a plugin could leave something behind in the markup to indicate the effect it's had. Perhaps like a 'patch' that can be reversed during a validation attempt. With an API like the extension idea mentioned above, a paragraph with an anchor extension could output something like:
<!-- wp:paragraph -->
<!-- extension:anchor -->
- <p>test</p>
+ <p id="test">test</p>
<-- /extension:anchor -->
<p id="test">test</p>
<!-- /wp:paragraph -->
As mentioned, if the 'anchor' extension isn't active, the reverse of the patch indicated between the extension
comments is applied to the markup and a validation attempt is made on that version of the markup. If it passes, the extension
comments are removed the next time the block is saved.
Raised on WordPress Slack by @phpbits (link requires registration):
https://wordpress.slack.com/archives/C02QB2JS7/p1559297836048800:
Just following this one up : https://core.trac.wordpress.org/ticket/45882 I’m having the same issue with custom attributes registered via
blocks.registerBlockType
JS filter. Thanks!
The issue exists because of those custom attributes are registered only with JS but not PHP. It's something that should be considered in the final proposal.
The issue exists because of those custom attributes are registered only with JS but not PHP. It's something that should be considered in the final proposal.
Custom attributes registered via blocks.registerBlockType
JS filter are not working. Probably add custom PHP filter too? Reregistering attributes isn't that much of a hassle I think.
Now the remaining issue is the "block validation".
Another thought on the validation problem: If we can know that the difference in output causing an invalidation is purely the difference of an extension, we could safely "ignore" (or warn) these invalidations.
For extensions which are purely modifying by style, we could always use data-
attributes, since they're ignored by the validator anyways. This also gets into overlap with the existing styles
registration property.
The bigger challenge is in identifying meaningful markup differences. There's still some options here, especially if we limit or have knowledge about how extensions source (map) from attributes.
I could imagine a separate comment blob which embeds both its own comment-serialized attributes and (redundant, but necessarily so) information about the attributes sourcing.
In the given example of anchor, then:
<!-- wp:paragraph -->
<!-- extension:anchor {"attributes":["*[id]"]} /-->
<p id="test">test</p>
<!-- /wp:paragraph -->
If the extension becomes disabled, we can still have awareness to know that the block invalidation is a result of the extension, since the embedded markup includes enough information to derive that it had claimed responsibility for this attribute.
(Aside: I'm not attached to naming here, and it's difficult to disambiguate "attributes" in the DOM sense vs. the block sense)
@gziolo @aduth Following up this issue if there's a branch available that I can check and test it out. Thanks!
I think this will look something like @youknowriad 's proposal with the diff for rolling back invalidations as proposed by @talldan. We also need a way to remove extensions so that issues like #6023 are resolved.
I will start to explore the Common Code part of this in a PR, starting with the colors code.
Early exploration:
/* use-attribute-picker.js */
/**
* External dependencies
*/
import { upperFirst, kebabCase } from 'lodash';
/**
* WordPress dependencies
*/
import { useMemo } from '@wordpress/element';
/**
* Internal dependencies
*/
import { useBlockEditContext } from './context';
export default function useAttributePicker(
names,
valuesEnum,
{
findValue = ( value, newValue ) => value === newValue,
mapValue = ( value ) => value,
} = {},
setterDeps = [],
mapAttribute = ( value ) => value,
mapDeps = [],
) {
const { setAttributes, attributes } = useBlockEditContext();
const setters = useMemo(
() => names.map( ( name ) => ( newValue ) => {
const foundValue = valuesEnum.find( ( value ) => findValue( value, newValue ) );
const mappedValue = foundValue && mapValue( foundValue );
setAttributes( {
[ name ]: mappedValue ? mappedValue : undefined,
[ `custom${ upperFirst( name ) }` ]: mappedValue ? undefined : newValue,
} );
} ),
setterDeps
);
const mappedAttributes = useMemo(
() => names.map( ( name ) => mapAttribute( attributes[ name ] || attributes[ `custom${ upperFirst( name ) }` ], name ) ),
[ ...names.map( ( name ) => attributes[ name ] || attributes[ `custom${ upperFirst( name ) }` ] ), ...mapDeps ]
);
return useMemo(
() => names.reduce( ( attributeObjectsAccumulator, name, i ) => {
attributeObjectsAccumulator[ `set${ upperFirst( name ) }` ] = setters[ i ];
attributeObjectsAccumulator[ name ] = mappedAttributes[ i ];
return attributeObjectsAccumulator;
}, {} ),
[ setters, mappedAttributes ]
);
}
/* use-colors.js */
export default function useColors( colorTypes ) {
const colorPalette = useSelect( ( select ) => select( 'core/block-editor' ).getSettings() || [], [] );
return useAttributePicker(
colorTypes.map( ( colorType ) => typeof colorType === 'string' ? colorType : Object.keys( colorType )[ 0 ] ),
colorPalette,
{
findValue: ( colorObject, colorValue ) => colorObject.color === colorValue,
mapValue: ( colorObject ) => colorObject.slug,
},
[ colorPalette ],
( colorValue, name ) => {
const colorObject = colorPalette.find( ( colorPaletteObject ) => colorPaletteObject.slug === colorValue ) || { color: colorValue };
const foundColorType = colorTypes.find( ( colorType ) => typeof colorType === 'string' ? colorType === name : Object.keys( colorType )[ 0 ] === name );
const colorContextName = typeof colorType === 'string' ? foundColorType : Object.keys( foundColorType )[ 0 ];
return {
...colorObject,
class: colorObject.slug && colorContextName && `has-${ kebabCase( colorObject.slug ) }-${ colorContextName }`,
};
},
[ colorPalette ]
);
}
Here is a huge need to remove block features with the help of hooks. Gutenberg is turning more and more in a full pagebuilder, but I have to restrict common features like resizable cover blocks for my clients, so they won't mess with it.
Yes, agreed. Here a strong need to limit eg the Heading levels of the Heading block, or remove DropCaps switch on paragraph. And limit (or control) the general editing options of the blocks.
As i understand correctly, we need to solve this for now via css / jquery?
My findings so far are, if I want to modify the sidebar (i.e. remove elements from it), use:
add_theme_support
.. remove color palette and fontsizesremoveEditorPanel
unregisterBlockStyle
I written more about it at https://soderlind.no/hide-block-styles-in-gutenberg/
Additional issues that contain requests to customize block functionality across different blocks:
https://github.com/WordPress/gutenberg/issues/21910 (turning off default wide alignment) https://github.com/WordPress/gutenberg/issues/16478 (color settings) https://github.com/WordPress/gutenberg/issues/13341 (color palette)
I also noticed a request in bringing in font size to the List block. https://wordpress.org/support/topic/change-size-text-in-list-block-in-gutenberg/
I think this issue served its purpose. We have now a clear direction about how we want to proceed.
Thanks all.
Part of the Roadmap:
This issue is created as a central point for the discussion for the proposed solution and to allow cross-linking between several existing issues which would benefit from the unified approach. Some examples: