Skip to content

Conversation

@kean
Copy link
Contributor

@kean kean commented Oct 22, 2025

What?

Closes CMM-867: Add support for inserting selected blocks at the selection point.

How?

(see comments)

Testing Instructions

  • Verify that the selected block is inserted after the currently selected block. If the selected block is an empty paragraph, replace it with a newly created block.
  • Verify that when you open and close the inserter, it restores the focus

Note: there may be other scenarios I'm not thinking of, including some of the composite blocks like "Gallery" or "Columns". For "Columns", I already opened CMM-880.

Screenshots or screencast

Screen.Recording.2025-10-22.at.9.12.53.AM.mov

@kean kean force-pushed the task/insert-block branch from f5a1435 to a971b45 Compare October 22, 2025 18:20
// Capture the selected block ID before showing the inserter
// (the editor will lose focus when the modal appears)
Task { @MainActor in
let selectedBlockID = await getSelectedBlockID()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative was to memorize the selectedBlockID inside the editor, but I thought it would be more flexible to provide the native side with a capability to get the ID of the currently selected block and a way to insert block at any position.

if ( isInserterOpened ) {
setIsInserterOpened( false );
}
onTouchEnd={ ( e ) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Button with onTouchEnd is what works on mobile without losing focus.

@kean kean requested a review from dcalhoun October 22, 2025 18:29
@kean kean force-pushed the task/insert-block branch from d332b04 to e714b40 Compare October 22, 2025 18:30
Base automatically changed from task/show-blocks-in-grid to trunk October 23, 2025 16:53
@kean kean force-pushed the task/insert-block branch from 848d34d to 932ffc4 Compare October 23, 2025 16:58
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting this together. It tested relatively well for me. I left a few comments for us to consider. Curious of your thoughts.

if ( isInserterOpened ) {
setIsInserterOpened( false );
}
onTouchEnd={ ( e ) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usage of onTouchEnd does introduce one downside (there may be others I yet to notice): you cannot cancel the interaction by dragging your finger away from the button.

My perception is that is simply the nature of onTouchEnd, its callback is triggered regardless of the position of the user's finger when it is triggered. Contrastingly, the onClick callback is only triggered when a full "click" occurs, which requires the end of the interaction to be atop the target button element.

We may be able to add logic to track and compare the touch start and end positions, or the element beneath the latter. Or Claude mentioned we could potentially use onClick and pair it with onMouseDown invoking preventDefault() to prevent focus changes.

While the onTouchEnd approach may work for a GutenbergKit-specific button, I'm concerned it may not be an avenue we could use for other buttons rendered within Gutenberg components—e.g., dropdown menu toggles and items.

Copy link
Contributor Author

@kean kean Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot cancel the interaction by dragging your finger away from the button.

I noticed that too yesterday. I'll try different option. ideally, it should behave like native buttons and cancel action.

or Claude mentioned we could potentially use onClick and pair it with onMouseDown invoking preventDefault() to prevent focus changes.

I tried that yesterday, but was not satisfied with the result for some reason – I'll give it another go.

if ( targetBlockClientId ) {
const selectedBlock = getBlock( targetBlockClientId );

// If the selected block is an empty paragraph, replace it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's functionality like this that make the native inserter concerning. How many are there that we may need to reimplement and maintain?

I noticed one missing that relates to the current block (similar to the noted composite blocks). For example, when inside a List (technically focused on a List Item), the inserter should display List Item as a top item. Additionally, selecting other block types that cannot be placed in a List block results in a new block inserted after the currently selected List.

Simulator.Screen.Recording.-.iPhone.17.-.2025-10-23.at.12.31.26.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many are there that we may need to reimplement and maintain?

This is probably the main risk. The inserter, if you remove the media and tests, is ~2500 lines of JS. Most of it is related to UI and search – not business logic. I'll spend some time reading through it tomorrow to see what else we might be missing. I'm concerned there are some unknowns, but, in principle, it seems to be just isolated enough from the rest of the editor, and we could always expose the APIs that we need.

I'm happy to consider alternatives, but so far, it still seems like the one thing worth doing to push the envelope on mobile. If we are not providing first-class integration for media, we are not doing enough, and this seems like the only technical solution.

For example, when inside a List (technically focused on a List Item), the inserter should display List Item as a top item.

I'm tracking it in the existing ticket CMM-866: Add support for dynamic blocks – depending on selection.

Additionally, selecting other block types that cannot be placed in a List block results in a new block inserted after the currently selected List.

The web does the same. I've also tested it with core/table previously and thought it was a bug, but it was the standard Gutenberg behavior.

Copy link
Member

@dcalhoun dcalhoun Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, selecting other block types that cannot be placed in a List block results in a new block inserted after the currently selected List.

The web does the same. I've also tested it with core/table previously and thought it was a bug, but it was the standard Gutenberg behavior.

Sorry, I meant to convey that standard functionality does not work with the native inserter. The video in my previous comment shows the working functionality with the web inserter.

Copy link
Contributor Author

@kean kean Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I got what you mean now. It does work with core/table and the native inserter – the selected incompatible block is appended below the table. It does not work with core/list – nothing happens. I'll look into it in the scope of this PR without creating a ticket.

@kean
Copy link
Contributor Author

kean commented Oct 24, 2025

I'm putting it back into draft. I've research the existing inserters and they all use the reusable useInsertionPoint and useBlockTypesState methods to determine what blocks to show and how to insert them. We should tap into it. There is no point in merging the intermediate solution.

I will also look into a way to better architect the interaction between web and native. On a high level, I want to create all the requires context by the inserter, including the callbacks, and pass it a a separate JS object to the native side that it could invoke (as opposed to defining these methods on window.editor).

cc @dcalhoun – let me know if it sounds like a plan.

@dcalhoun
Copy link
Member

@kean overall, that makes sense to me.

On a high level, I want to create all the requires context by the inserter, including the callbacks, and pass it a a separate JS object to the native side that it could invoke (as opposed to defining these methods on window.editor).

Avoiding window.editor is attractive. I do not follow what the replacement would resemble. Could you provide pseudo code to help me understand please?

@kean
Copy link
Contributor Author

kean commented Oct 24, 2025

I made a significant progress by using useInsertionPoint and useBlockTypesState. With these hooks, the native side doesn't need to know and implement very little.

Scenarios

Top-Level Insertion

  • Insert a block in empty core/paragraph – the paragraph gets replaced with the selected block ✅
  • Insert a block in an arbitrary “empty” position in the editor – gets inserted at the selected position ✅
  • Insert a block when the title is selected - shows an error “Block X can’t be inserted” ⛔️

Composite Blocks

  • Insert a supported block (core/list-item) in core/list – the list item is added ✅
  • Insert an unsupported block (core/image) in core/list – gets inserted after the block ✅
  • Insert a supported block (core/list) in core/quote – gets inserted in the block ✅

Contextual Blocks

  • Insert a block that supports a subset of child blocks (core/list) and open the inserter – blocks that support this parent (core/list-item) are displayed in a dedicated section at the top ✅
  • Insert a block that supports a subset of child blocks (core/list) in a composite block (core/quote) and open the inserter – blocks that support this parent (core/list-item) are displayed in a dedicated section at the top ✅

Known Issues

Recording

Screen.Recording.2025-10-24.at.4.24.04.PM.mov

Current Limitations

The main challenges I encountered:

  • Both useInsertionPoint and useBlockTypesState are private functions that are not exposed even via @wordpress/private-apis. It looks like we'll need to update the core to mitigate it.
  • Both are hooks, so I had to create an empty React component to use them. I'm not sure if there are any performance implications to it. It seems to re-compute often.

I'll might need some help with it. I'm going to wrap up for today.

Could you provide pseudo code to help me understand please?

I decided to keep to simple. If there is more than one scenario where we need a generalized solution, then maybe it would be a good time to consider it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants