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

Common Block Functionality #15450

Open
gziolo opened this issue May 6, 2019 · 14 comments

Comments

@gziolo
Copy link
Member

@gziolo gziolo commented May 6, 2019

Part of the Roadmap:

coalesce into a preferred mechanism for creating and sharing chunks of functionality (block alignment, color tools, etc) across blocks with a simple and intuitive code interface. (Suggested exploration: React Hooks.)

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:

@gziolo

This comment has been minimized.

Copy link
Member Author

@gziolo gziolo commented May 6, 2019

Related comment from @youknowriad left in #6023 (comment):

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 and disable-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

@aduth

This comment has been minimized.

Copy link
Member

@aduth aduth commented May 6, 2019

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:

  • Attribute as value storage
    • Example: alignment
  • Custom controls in the edit implementation
    • Example: Alignment block toolbar controls
  • Altered markup or classes in the save implementation
    • Example: Including alignleft, etc. alignment classes

Notably, 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 #8171 (comment) , 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' ],

	// ...
} );
@talldan

This comment has been minimized.

Copy link
Contributor

@talldan talldan commented May 7, 2019

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 #14918, when there's an attempt to bring a consistent improvement to these blocks it involves a lot of work.

@youknowriad

This comment has been minimized.

Copy link
Contributor

@youknowriad youknowriad commented May 22, 2019

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 );
  • This introduces a concept of a block extension: There's some analogy with the block registration where each property is a function that receives the "previous value" and returns the "next value" of the property.
  • We would control which properties are supported (in the example above I have attributes and edit)
  • A block extension have a name: anchor in the example above
  • A block extension can have options.
  • A block extension can be attached to one or several blocks
  • The options used for the same extension can defer between blocks
  • A block extension can be registered and/or attached at any time before the parsing happens (unfortunately the parsing is still a hard limit in terms of timing) but it doesn't care when the block is registered.

Now 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)

@talldan

This comment has been minimized.

Copy link
Contributor

@talldan talldan commented May 23, 2019

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.

@gziolo

This comment has been minimized.

Copy link
Member Author

@gziolo gziolo commented May 31, 2019

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.

@phpbits

This comment has been minimized.

Copy link

@phpbits phpbits commented May 31, 2019

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.

@aduth

This comment has been minimized.

Copy link
Member

@aduth aduth commented Jun 3, 2019

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)

@phpbits

This comment has been minimized.

Copy link

@phpbits phpbits commented Jun 15, 2019

@gziolo @aduth Following up this issue if there's a branch available that I can check and test it out. Thanks!

@epiqueras

This comment has been minimized.

Copy link
Contributor

@epiqueras epiqueras commented Jun 18, 2019

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.

@epiqueras

This comment has been minimized.

Copy link
Contributor

@epiqueras epiqueras commented Jun 18, 2019

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 ]
	);
}
@Horttcore

This comment has been minimized.

Copy link

@Horttcore Horttcore commented Aug 30, 2019

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.

@sschat

This comment has been minimized.

Copy link

@sschat sschat commented Sep 25, 2019

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?

@soderlind

This comment has been minimized.

Copy link

@soderlind soderlind commented Nov 4, 2019

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 fontsizes
  • removeEditorPanel
  • unregisterBlockStyle

I written more about it at https://soderlind.no/hide-block-styles-in-gutenberg/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.