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

Block API: Add new bindAttribute helper for editor inputs #7352

Closed
wants to merge 2 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 18, 2018

Related: #5739

This pull request seeks to explore a developer experience optimization for implementations of blocks, reducing the effort necessary to support value changing of an attribute via a new optional bindAttribute prop for use in editor input components.

Before:

function MyBlockEdit( props ) {
	var value = props.attributes.content;

	function onChangeContent( nextContent ) {
		props.setAttributes( { content: nextContent } );
	}

	return wp.element.createElement( wp.editor.RichText, {
		value: value,
		onChange: onChangeContent,
	} );
}

After:

function MyBlockEdit( props ) {
	return wp.element.createElement( wp.editor.RichText, {
		bindAttribute: 'content',
	} );
}

This is intended to be very similar to React's now-deprecated LinkedStateMixin, a pattern for two-way data binding. While it sacrifices explicitness, it does so in a way intentionally for promoting a simplified developer experience for block implementers, in a consistent fashion to the automatic handling of RichText focus and isSelected conditional rendering.

Implementation notes:

Attribute binding is achieved with a new withBindAttribute higher-order component, which can be applied to any editor input component to optionally accept the bindAttribute prop as an alternative to the value and onChange pairing.

Currently, this is only implemented for RichText. It has proven to be difficult to implement for other components because context is lost in Slot/Fill for non-bubblesVirtually slots (including toolbars and inspector controls). This will need to be addressed separately as a general issue, either updating all existing slots to use bubblesVirtually or a way for context to be preserved in non-virtual-bubbling slots rendering.

Testing instructions:

Verify that there are no regressions in the display and editing of a paragraph block's value.

@aduth aduth added the [Feature] Block API API that allows to express the block paradigm. label Jun 18, 2018
@aduth aduth requested review from nb and mtias June 18, 2018 16:58
@nb
Copy link
Member

nb commented Jun 20, 2018

Thanks for the exploration, @aduth.

The <MyEnhancedInput bindAttribute="content" /> example is not working for me, because for built--in elements the onChange event handler is passed an event object and not just the value, unlike RichText.

As a block owner, I would expect this to work on all Gutenberg-supplied input components, not only RichText. The extra cognitive load of the shortcut would only pay off learning only if it lets me avoid using setAttributes in 80-90% of the cases.

After looking at the core blocks, I am surprised how often we can't use bindAttributeseven if it was supported in all components – we often need to pre-process the data (default value, opposite value, fromRichTextValue, etc.) or have even more complicated logic, like different event handlers or setting two attributes at once. If I still have to use setAttributes in a file, I’d prefer to use an explicit method for clarity and consistency in the same file, instead of bindAttributes.

Overall bindAttribute doesn't look as attractive as I thought it would be – it doesn't save us a ton of space, it saves us only the method, which is often inline, probably won't be enough to get rid if the props and attributes destructuring lines. I'd rather we don't include it.

@aduth
Copy link
Member Author

aduth commented Jun 25, 2018

Yes, the MyEnhancedInput is an interesting counter-example. I think we could make this work consistently, but only across those components we promote as a "preferred" variant of an input: i.e. we'd probably need an entire set of block/inspector controls (which we had at one point) that the developer would be expected to use.

There's a bit of context in the original comment to why this was only introduced for RichText currently, due to challenges around Slot/Fill and inheriting context.

I'll close this for now as an interesting exploration that could be picked up later if desired.

@aduth aduth closed this Jun 25, 2018
@sirreal sirreal deleted the add/bind-attribute branch June 18, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants