Replace BlockPath with external clientIds for selection persistence#75193
Replace BlockPath with external clientIds for selection persistence#75193youknowriad wants to merge 9 commits intotrunkfrom
Conversation
|
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 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. |
|
Size Change: -34 B (0%) Total Size: 2.99 MB
ℹ️ View Unchanged
|
Move getInternalClientId and getExternalClientId from public selectors to private-selectors.js since they are internal implementation details for entity navigation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@tellthemachines This is close to what I had in mind, might still need some iterations. |
|
Flaky tests detected in 432381c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/21667093594
|
ntsekouras
left a comment
There was a problem hiding this comment.
I like how this simplifies the code. The only issue I noticed is that in post editor the block is selected when we go back from the isolated editing, but there is no scroll to that block.
I'm not sure how it would be best to achieve that without a timeout (like before) or requestAnimationFrame. I've tried a few things updating useScrollIntoView, but without success.
|
|
||
| - _clientId_ `string`: The block's clientId. | ||
| - _hasControlledInnerBlocks_ `boolean`: True if the block's inner blocks are controlled. | ||
| - _mappings_ `Object[]`: Optional array of { external, internal } clientId mappings. |
There was a problem hiding this comment.
I think we could elaborate a bit more here. What do these mappings do in practice.
There was a problem hiding this comment.
The mapping we were doing in use-block-sync need to be accessible outside use-block-sync (for selection restoration), this is why I'm putting this information into the store.
If the template view is enabled in the post editor, it doesn't even select the block. Likewise, when trying to "View" a page from Navigation in a template:
it also doesn't reselect the Nav item block when going back. Those were the exact scenarios that led me to use block paths instead of clientIds in #73737, so we need to make sure any alternative solution works for them. |
|
yes, this is not finished, but I wanted to show the idea, the remaining issues are likely small issues. I'm also looking at removing |
This commit addresses several issues with block selection when navigating between entities (e.g., from template to page and back): 1. Read selection from entity edits using post.id prop directly, bypassing getCurrentPostId() which lags in useEffect timing. 2. Use entity identity (kind:name:id) as cache key for parsed blocks with content validation, instead of object reference that changes on re-render. 3. For edit-site, restore selection synchronously before EditorProvider renders by dispatching editEntityRecord during the render phase. 4. Store only selectedBlock (external clientId) in URL - postType/postId are already resolved from the URL path. 5. Clean up: remove debug logs, simplify hook return signatures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| // Use requestAnimationFrame to ensure nested controlled blocks | ||
| // (like synced patterns within templates) have finished setting up | ||
| // their clientId mappings before we try to convert selection IDs. | ||
| window.requestAnimationFrame( () => { |
There was a problem hiding this comment.
This is the only thing that I don't like and that I wish I could improve somehow in this approach.
Consolidates the selection restoration logic into useResolveEditedEntity hook since it's already responsible for resolving entity state from the URL. Also removes the URL param clearing as the ref-based guard is sufficient. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| @@ -13,7 +13,7 @@ import useEntityId from './use-entity-id'; | |||
| import { updateFootnotesFromMeta } from '../footnotes'; | |||
There was a problem hiding this comment.
The changes on this file ensure that when we navigate to an entity, don't make any edits, move away and go back to that entity, the same blocks (and clientIds) are used instead of reparsing the blocks a second time.
|
This should be working properly now, let me know if you noticed anything problematic. I wished we had e2e tests for this. (might add some) |
|
I gave this a quick test and it's working as expected in the post editor now - both with and without visible template. It also works for templates in the site editor (both with sync patterns and when clicking "view" on a nav link). But I can't get it to work when editing pages in the site editor, whether the template is visible or not. It's not even selecting the block for me. |
When editing a page with a template, EditorProvider reads selection from the page entity (context.postType/context.postId), not the template entity. Set selection on the correct entity so it is picked up by EditorProvider. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Good catch @tellthemachines looks like I was not setting the selection on the right post type / post id. It should be fixed now. |
Selection restoration on entity navigation should not create undo levels, as it's an internal mechanism, not a user edit. Also moves selection restoration logic into useResolveEditedEntity for cleaner separation of concerns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
For root-level blocks (the common undo case), external and internal clientIds are identical, so resetSelection can be applied synchronously after resetBlocks. Only defer with requestAnimationFrame when the target block is inside a controlled inner block whose clientId mappings haven't been set up yet. This reduces timing variability that could cause flaky undo tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When blocks are reset (e.g. during template revert), the entity edits may still contain a stale selection pointing to a block that no longer exists. The rAF fallback was restoring this stale selection, causing useAutoSwitchEditorSidebars to switch the sidebar to the Block tab. Re-check if the block exists after the rAF. If it still doesn't, the selection is stale rather than a controlled inner block waiting for setup — skip the restoration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 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. |
This reverts commit 4030ab5.
|
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 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. |

What?
Replaces the BlockPath system with direct external clientIds for block selection persistence across entity navigation. This is a follow-up to the stable clientId mapping work in useBlockSync.
Fixes the block selection restoration when navigating between entities (e.g., editing a synced pattern and returning to the post).
Why?
The previous BlockPath approach used
{blockName, index, contentHash}paths which were fragile and required walking the block tree to find matches. Since blocks from entities now have stable external clientIds (mapped to internal IDs when cloned), we can use these directly for more reliable selection persistence.How?
Store Changes
controlledInnerBlocksstate to store clientId mappings alongside the controlled stategetInternalClientIdandgetExternalClientIdselectors to convert between external and internal clientIdssetHasControlledInnerBlocksaction to accept mappings parameteruseBlockSync Changes
setHasControlledInnerBlockswhen blocks are clonedinitialSelectionprop to apply selection after blocks mountNavigation Hook Changes
selectionobject instead ofselectedBlockClientIdfor extensibilitygetExternalClientIdto convert internal selection to external before storinginitialSelectionobject directlyCleanup
useGenerateBlockPathanduseRestoreBlockFromPathutilitiesTesting Instructions
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com