Fix empty url value from unbinding entity from inspector sidebar #72447
Conversation
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
|
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: +33 B (0%) Total Size: 2.2 MB
ℹ️ View Unchanged
|
jeryj
added
[Type] Bug
|
Flaky tests detected in ce74a66. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18652675495
|
Contributor
Author
|
I'd rather wait to merge this until #72436 is merged. |
getdave
approved these changes
Oct 20, 2025
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
I tested this and the flow does feel less jarring and it's easier to avoid ending up in a bad state.
I agree, LinkControl is the goal post-6.9 but for now this is an improvement.
I note that it's now not possible to end up with an empty URL as the logic always restores the last value if the blur happens and the value is empty. I was thinking of this as a bug but actually it's a good safety catch. Worst case folks have to use at least # so we avoid possibility of true empty links.
packages/block-library/src/navigation-link/shared/test/controls.js
Outdated
Show resolved
Hide resolved
The unsync and edit button enables the disabled input and removes the button that was focused. This causes a focus loss. I added: - Ref for the input - Ref for knowing if we should focus on next render (we can't focus immediately, as the input is disabled and cannot receive focus) - useEffect to check if we should send focus to the input when hasUrlBinding changes
Unbinding the navigation link entity from the sidebar updates the url value to be '', which causes the canvas to go into its empty URL state, which removes the label. Then, when you start typing in the url link input, it would steal focus back to the canvas. This changes the flow when unbinding to set the url as the previously linked url, which avoids an empty link issue. However, you can still get to the messed up state by deleting the url value. To fix this, instead of updating the attribute onChange, update it onBlur so we can restore the last known url.
jeryj
force-pushed
the
fix/navigation-link-empty-inspector-link
branch
from
c4e2877 to
0b29a12
Compare
ellatrix
pushed a commit
that referenced
this pull request
Nov 11, 2025
) When unsyncing a navigation link url, we now set the url as the synced entity url to avoid empty url states which cause bugs in the canvas. We also defer updating the url value in the store until the blur event to avoid the canvas receiving a temporarily empty value (such as when a user clears the URL in order to start typing a new URL). ---- Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
Member
|
I've manually cherry-picked this in 6a7bf43. |
ellatrix
added
the
Backported to WP Core
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.
Fixes #72481
Unbinding the navigation link entity from the sidebar updates the url value to be '', which causes the canvas to go into its empty URL state, which removes the label. Then, when you start typing in the url link input, it would steal focus back to the canvas. This changes the flow when unbinding to set the url as the previously linked url, which avoids an empty link issue. However, you can still get to the messed up state by deleting the url value. To fix this, instead of updating the attribute onChange, update it onBlur so we can restore the last known url.
What?
When unsyncing the entity of a navigation link from the inspector sidebar:
This PR fixes the issue by filling the url field with the existing entity link url so it doesn't get an empty value, which means the placeholder on the canvas does not show.
Why?
The current flow is clearly not the ideal behavior. It's a bug.
How?
This does not need to be the final solution, as I think integrating the link control with this field will ultimately be better. However, the behavior on trunk is very buggy, and this PR does not feel buggy.
Testing Instructions
From an entity bound navigation link
Go to its inspector sidebar
Click the Unsync and edit button
The link text should be whatever the previously bound url is
The link text should be selected so you can easily delete it
Update the link
Move focus away from the link.
Check it works as expected
From an entity bound navigation link
Go to its inspector sidebar
Click the Unsync and edit button
Delete the url
The on canvas text label should remain
Tab away from the field
The url link input should revert to the value it was before editing began
Testing Instructions for Keyboard
Screenshots or screencast
Before
Screen.Recording.2025-10-17.at.2.09.09.PM.mov
After
Screen.Recording.2025-10-17.at.2.08.31.PM.mov