InnerBlocks: Support insert before/after block actions when using allowedBlocks #59162
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mamaduka
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @mapk, @gwwar. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Mamaduka
changed the title
|
Size Change: +132 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 9010ab0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7970075184
|
Mamaduka
changed the title
Mamaduka
requested review from
a team,
tellthemachines and
ntsekouras
and removed request for
a team
|
@Mamaduka I think it will also close this one - Make the 'Insert Before' / 'Insert After' options work for block lists that don't allow the default block type |
|
@talldan, the Social Icons block will need an update to display a placeholder state when no variation is defined, similar to the Groups block. That can be done in a follow-up, so it shouldn't be a blocker for this PR. |
|
Thanks @Mamaduka! I tested the examples @richtabor identified in #56228. With this PR I can now see an "Add before" and "Add after" button when selecting an item in these blocks:
But not these blocks:
Are we able to handle those as well? |
|
Working for button and navigation blocks 👍🏻
On top of this PR I tested adding default blocks to List, Social Links, Gallery and Columns and it also works pretty well. Not a huge change (see diff below) so in this PR or a follow up would be ace. diff - adding default blocks to userInnerBlockPropsdiff --git a/packages/block-library/src/columns/edit.js b/packages/block-library/src/columns/edit.js
index 95e0ba43ef..0a756b34c6 100644
--- a/packages/block-library/src/columns/edit.js
+++ b/packages/block-library/src/columns/edit.js
@@ -40,6 +40,10 @@ import {
toWidthPrecision,
} from './utils';
+const DEFAULT_BLOCK = {
+ name: 'core/column',
+};
+
function ColumnsEditContainer( { attributes, setAttributes, clientId } ) {
const { isStackedOnMobile, verticalAlignment, templateLock } = attributes;
const { count, canInsertColumnBlock, minCount } = useSelect(
@@ -90,6 +94,8 @@ function ColumnsEditContainer( { attributes, setAttributes, clientId } ) {
className: classes,
} );
const innerBlocksProps = useInnerBlocksProps( blockProps, {
+ defaultBlock: DEFAULT_BLOCK,
+ directInsert: true,
orientation: 'horizontal',
renderAppender: false,
templateLock,
diff --git a/packages/block-library/src/gallery/edit.js b/packages/block-library/src/gallery/edit.js
index 226e4da673..c62ebff570 100644
--- a/packages/block-library/src/gallery/edit.js
+++ b/packages/block-library/src/gallery/edit.js
@@ -54,6 +54,9 @@ import useGetNewImages from './use-get-new-images';
import useGetMedia from './use-get-media';
import GapStyles from './gap-styles';
+const DEFAULT_BLOCK = {
+ name: 'core/image',
+};
const MAX_COLUMNS = 8;
const linkOptions = [
{ value: LINK_DESTINATION_ATTACHMENT, label: __( 'Attachment Page' ) },
@@ -496,6 +499,8 @@ function GalleryEdit( props ) {
};
const innerBlocksProps = useInnerBlocksProps( blockProps, {
+ defaultBlock: DEFAULT_BLOCK,
+ directInsert: true,
orientation: 'horizontal',
renderAppender: false,
...nativeInnerBlockProps,
diff --git a/packages/block-library/src/list/edit.js b/packages/block-library/src/list/edit.js
index ad294de93e..5c01b6899b 100644
--- a/packages/block-library/src/list/edit.js
+++ b/packages/block-library/src/list/edit.js
@@ -31,6 +31,9 @@ import TagName from './tag-name';
const TEMPLATE = [ [ 'core/list-item' ] ];
const NATIVE_MARGIN_SPACING = 8;
+const DEFAULT_BLOCK = {
+ name: TEMPLATE[ 0 ][ 0 ],
+};
/**
* At the moment, deprecations don't handle create blocks from attributes
@@ -125,6 +128,8 @@ export default function Edit( { attributes, setAttributes, clientId, style } ) {
} );
const innerBlocksProps = useInnerBlocksProps( blockProps, {
+ defaultBlock: DEFAULT_BLOCK,
+ directInsert: true,
template: TEMPLATE,
templateLock: false,
templateInsertUpdatesSelection: true,
diff --git a/packages/block-library/src/social-links/edit.js b/packages/block-library/src/social-links/edit.js
index 38a68e7f2d..6c0641bab5 100644
--- a/packages/block-library/src/social-links/edit.js
+++ b/packages/block-library/src/social-links/edit.js
@@ -27,6 +27,10 @@ import {
import { __ } from '@wordpress/i18n';
import { check } from '@wordpress/icons';
+const DEFAULT_BLOCK = {
+ name: 'core/social-link',
+};
+
const sizeOptions = [
{ name: __( 'Small' ), value: 'has-small-icon-size' },
{ name: __( 'Normal' ), value: 'has-normal-icon-size' },
@@ -104,6 +108,8 @@ export function SocialLinksEdit( props ) {
const blockProps = useBlockProps( { className } );
const innerBlocksProps = useInnerBlocksProps( blockProps, {
+ defaultBlock: DEFAULT_BLOCK,
+ directInsert: true,
placeholder: isSelected ? SelectedSocialPlaceholder : SocialPlaceholder,
templateLock: false,
orientation: attributes.layout?.orientation ?? 'horizontal',
Oh, just saw this 👍🏻 |
Mamaduka
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
@noisysocks, yes, most of those blocks just need to update the inner block setting definitions. I've prepared a PR for the Gallery block - #59168 and will update others as well. The Social Icons block will need more adjustments on the block level. |
Mamaduka
force-pushed
the
try/inner-blocks-add-before-after
branch
from
bf7dee9 to
425e5e0
Compare
Mamaduka
force-pushed
the
try/inner-blocks-add-before-after
branch
from
425e5e0 to
9010ab0
Compare
noisysocks
reviewed
Feb 20, 2024
| const canInsertDefaultBlock = canInsertBlockType( | ||
| getDefaultBlockName(), | ||
| rootClientId | ||
| const selected = useSelect( |
There was a problem hiding this comment.
Why store in selected instead of destructuring right here?
| ); | ||
| const { getBlocksByClientId, getBlocks } = useSelect( blockEditorStore ); |
There was a problem hiding this comment.
This approach is much nicer 🙂
| ...directInsertBlock.attributes, | ||
| ...copiedAttributes, | ||
| } ); | ||
| return dispatch.insertBlock( block, blockIndex + 1, rootClientId ); |
There was a problem hiding this comment.
Can we make insertBeforeBlock and insertAfterBlock both call into e.g. insertDefaultBlock( clientId: string, position: 'before' | 'after' ) or insertDefaultBlock( clientId: string, offset: 1 | -1 ) to reduce duplication? There's quite a lot of repetition now that both actions have more logic.
There was a problem hiding this comment.
So, move most of the logic into the insertDefaultBlock action, correct?
The only issue I see here is that insertDefaultBlock is more widely used where consumers rely on it to insert an empty paragraph. Example:
gutenberg/packages/editor/src/components/post-title/index.js
Lines 77 to 79 in 09dc127
P.S. It's definitely doable, but I am unsure if removing 20+ lines is worth complicating the logic of the existing insertDefaultBlock action.
There was a problem hiding this comment.
Sorry I meant a new selector not insertDefaultBlock. Not sure what to to name it 😀
Up to you. I think it's worth it now that the logic isn't trivial, but respect your judgement.
| @@ -75,11 +88,9 @@ export default function BlockActions( { | |||
| return children( { | |||
| canCopyStyles, | |||
| canDuplicate, | |||
| canInsertDefaultBlock, | |||
| canInsertBlock, | |||
There was a problem hiding this comment.
Just confirming that BlockActions isn't a public API, yeah?
There was a problem hiding this comment.
Correct. It's not a public API.
noisysocks
approved these changes
Feb 21, 2024
|
Thanks everyone for the feedback. |
andrewserong
changed the title
|
It looks like some block bindings changes for 6.5 rely on the changes of this PR to work properly, so I'm going to include this in 6.5. Let me know if there's any concern with that. |
youknowriad
added
the
Backport to WP 6.7 Beta/RC
youknowriad
pushed a commit
that referenced
this pull request
Mar 4, 2024
* Don't call selectors during the render; subscribe to the store instead * Get blocks on demand * Update 'insertBeforeBlock' and 'insertAfterBlock' actions * A handler 'attributesToCopy' definitions Unlinked contributors: mapk, gwwar. Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: noisysocks <noisysocks@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: shaunandrews <shaunandrews@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org>
|
I just cherry-picked this PR to the update/packages-6.5-rc1 branch to get it included in the next release: ebdf83d |
youknowriad
added
Backported to WP Core
youknowriad
pushed a commit
that referenced
this pull request
Mar 4, 2024
* Don't call selectors during the render; subscribe to the store instead * Get blocks on demand * Update 'insertBeforeBlock' and 'insertAfterBlock' actions * A handler 'attributesToCopy' definitions Unlinked contributors: mapk, gwwar. Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: noisysocks <noisysocks@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: shaunandrews <shaunandrews@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Resolves #23603.
Resolves #56228.
PR enables "Add before/after" block actions for InnerBlocks with
allowedBlockswhen the default block is specified.Note
Currently, this doesn't handle the cases when the
allowedBlockscontains a single block type.Why?
See #56228.
How?
BlockActionsto use store subscription instead of calling selectors during the render to avoid stale values.blocksvalue inside the callbacks. Prevents block settings from re-rendering when the Top Toolbar is used.insertBeforeBlockandinsertAfterBlockactions to handle thedirectInsertBlocksetting.P.S. Happy to extract general refactoring comments into a new PR if that would make reviewing easier.
Todos
directInsertBlock.attributesToCopysetting. See bf7dee9.Testing Instructions
Testing Instructions for Keyboard
Same.
Screenshots or screencast
CleanShot.2024-02-19.at.09.09.17.mp4