Repository navigation
Blocks: Adjust local state during render instead of in effects - #85814
Merged
Merged
Conversation
Ten effects in core blocks only fixed the component's own state after a render: close the link editor or clear the selected table cell on deselect, show or hide the caption, hide the group placeholder, stop the navigation menu update, clear a temporary image URL, and fall back to the first playlist track. They now do the same check during render, which React supports for state that depends on props, and skips the extra render. Adds e2e tests for the button link editor and the caption. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in its handler Playlist computes the current track from the selected one, so it no longer sets state during render. The navigation menu selector resets its "updating" flag in a `finally` after creating a menu, which also covers a failed request, so it no longer needs the creation status props. Adds an e2e test that "Create new Menu" works again after a failed attempt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ellatrix
added
[Type] Code Quality
🤖 PR meta 🤖🎉 PropsIf 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. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: -174 B (0%) Total Size: 8.56 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information. As a user I want to be able to add multiple close buttons so that users can close the overlay from different positions in
|
ellatrix
marked this pull request as ready for review
October 28, 2026 15:47
ellatrix
enabled auto-merge (squash)
October 28, 2026 15:57
This was referenced Oct 28, 2026
This was referenced Nov 12, 2026
Open
This file contains hidden or 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
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?
Follow up to #85810. Removes 10 more
useEffectcalls in core blocks: the ones that only fix the component's own state after a render.finally, so thecreateNavigationMenuIsSuccess/createNavigationMenuIsErrorprops passed through four files only for that effect are removedAdds e2e tests for the Button link editor, the Caption and "Create new Menu" after a failed request.
Why?
Step towards not using effects in core blocks. An effect that fixes state renders once with the stale state, commits it, then renders again. For example, the Navigation Submenu link popover is not hidden by
isSelected, so it is rendered for one more commit after the block is deselected.React's "You Might Not Need an Effect" recommends computing the value, or failing that, adjusting the state during render. React then throws away that render and renders again before anything is committed. Each check here only sets the component's own state and turns false once set, so it can't loop.
Post Featured Image and Site Logo keep the state update during render: the real URL arrives when the media record loads, after the handler that sets the id, so clearing the temporary URL in the handler would flash the placeholder.
How?
if ( ! isSelected && isEditingURL ) { setIsEditingURL( false ); }in place of an effect onisSelected, and the same shape for the other state updates.usePrevious.selectedTrackClientIdis the state;currentTrackClientIdis computed from it.try { await onCreateNew(); } finally { setIsUpdatingMenuRef( false ); }covers both cases.Testing Instructions
New e2e tests: each passes on trunk, since behaviour doesn't change, and fails when the code it guards is removed:
Test output: guarded code removed, trunk
The Table change has no e2e test: selecting the table block again focuses a cell, which selects it again, so there is no keyboard path to a selected table without a selected cell.
Testing Instructions for Keyboard
Steps 1 to 3 work with the keyboard: Ctrl/Cmd+K, Tab and arrow keys to move between blocks, Ctrl/Cmd+Z to undo.
Screenshots or screencast
None: the stale state was on screen for at most one commit, so nothing visible changes.
Use of AI Tools
Written with Claude Code (Claude Opus 5.5), directed by @ellatrix.
🤖 Generated with Claude Code