Skip to content

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

Closed
aduth wants to merge 2 commits intomasterfrom
add/bind-attribute
Closed

Block API: Add new bindAttribute helper for editor inputs#7352
aduth wants to merge 2 commits intomasterfrom
add/bind-attribute

Conversation

@aduth
Copy link
Copy Markdown
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 mtias and nb June 18, 2018 16:58
@nb
Copy link
Copy Markdown
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
Copy Markdown
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
@aduth aduth mentioned this pull request Aug 10, 2018
Closed
@aduth aduth mentioned this pull request Sep 13, 2018
Closed
@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

Reviewers

@nb nb Awaiting requested review from nb

@mtias mtias Awaiting requested review from mtias

Assignees

No one assigned

Labels

[Feature] Block API API that allows to express the block paradigm.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aduth @nb