Block Bindings: Fix focus behavior when pressing Enter on connected block #58958
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { store as blocksStore } from '@wordpress/blocks'; | ||
| import { useSelect } from '@wordpress/data'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { BLOCK_BINDINGS_ALLOWED_BLOCKS } from '../../hooks/use-bindings-attributes'; | ||
| import { useBlockEditContext } from '../block-edit'; | ||
| import { store as blockEditorStore } from '../../store'; | ||
| import { unlock } from '../../lock-unlock'; | ||
|
|
||
| export function useShouldDisableEditing( attributeName ) { | ||
| const { clientId, name: blockName } = useBlockEditContext(); | ||
| return useSelect( | ||
| ( select ) => { | ||
| if ( | ||
| ! attributeName || | ||
| ! BLOCK_BINDINGS_ALLOWED_BLOCKS[ blockName ]?.includes( | ||
| attributeName | ||
| ) | ||
| ) { | ||
| return false; | ||
| } | ||
| const blockBindings = | ||
| select( blockEditorStore ).getBlockAttributes( clientId ) | ||
|
There was a problem hiding this comment. This is a performance problem because we're adding another subscription to block attributes to every block (at least paragraph and all other blocks with binding support in this case). I used context in #59320. |
||
| ?.metadata?.bindings; | ||
| if ( ! blockBindings?.[ attributeName ]?.source ) { | ||
| return false; | ||
| } | ||
| const blockBindingsSource = unlock( | ||
| select( blocksStore ) | ||
| ).getBlockBindingsSource( blockBindings[ attributeName ].source ); | ||
| return blockBindingsSource?.lockAttributesEditing !== false; | ||
| }, | ||
| [ clientId, blockName, attributeName ] | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -293,12 +293,23 @@ function ButtonEdit( props ) { | |
| ...spacingProps.style, | ||
| ...shadowProps.style, | ||
| } } | ||
| onSplit={ ( value, isOriginal ) => { | ||
| let newAttributes; | ||
|
There was a problem hiding this comment. I would be also curious to better understand the changes applied here. There was a problem hiding this comment. Okay, I'll try to do my best from what I understood from my testings 😄 When you use
With the previous logic, we were passing the attributes even if it wasn't the original block and its value it's empty. This happens when you click enter at the end of the block, with the cursor in the last character. You only want to preserve some value and the attributes when you actually split the text. Actually, I just realized that we are preserving the metadata in the new block, which I believe it is not correct. For example, if I have a button and rename it to "Custom button" with text "My button", I put the cursor in the middle and I click Enter, the new block has the "Custom button" name as well. It should be easy to fix just checking if it is the original block or not. I can work on that. Anyway, I believe this last part is not related to this PR because it happens with not bound blocks using rich text: paragraph, heading, button. It could be fixed changing the conditional to something like this: if ( isOriginal ) {
newAttributes = {
...attributes,
text: value,
};
} else if ( value ) {
newAttributes = {
...attributes,
metadata: undefined,
text: value,
};
}But we can work on that in a follow-up if we really want to get rid of the metadata There was a problem hiding this comment. Would it work this way? newAttributes = {
...attributes,
metadata: isOriginal ? attributes.metadata : undefined,
text: value,
}; There was a problem hiding this comment. I guess 🙂 Much better! There was a problem hiding this comment. This seems unrelated to the main problem? Maybe split this out in a separate PR? |
||
| if ( isOriginal || value ) { | ||
| newAttributes = { | ||
| ...attributes, | ||
| text: value, | ||
| }; | ||
| } | ||
| const block = createBlock( | ||
| 'core/button', | ||
| newAttributes | ||
| ); | ||
| if ( isOriginal ) { | ||
| block.clientId = clientId; | ||
|
There was a problem hiding this comment. I assumed it is needed because when we split a paragraph or a button we are actually creating two new blocks, and I guess that createBlock generates a new |
||
| } | ||
| return block; | ||
| } } | ||
| onReplace={ onReplace } | ||
| onMerge={ mergeBlocks } | ||
| identifier="text" | ||
|
|
||
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.
Can you explain what that branch of the code is doing?
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 believe this explanation about how
splitValueworks should help to understand this as well: link.Another option here could be to change
_value.end = _value.text?.length.From my testing, it seems that when the rich text is not editable, it acts as if the cursor was in the initial character. This causes that when you click on enter, it doesn't work as expected. For that reason, I believe we could assume that when
shouldDisableEditingis true, we don't have to add any text to the new block. Or we want to simulate placing the cursor at the end.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 see. I was totally lost, so that made it clear. I think simulating the case that the cursor is at the end should do the trick for most of the blocks 💯
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.
Just changed that part in this commit.