Navigation Link: Preserve custom label when adding URL via LinkPicker#74013
Merged
Navigation Link: Preserve custom label when adding URL via LinkPicker#74013
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: +322 B (+0.01%) Total Size: 2.58 MB
ℹ️ View Unchanged
|
getdave
requested review from
MaggieCabrera,
getdave,
mikachan and
scruffian
getdave
added
[Block] Navigation Link
Contributor
|
@copilot can you work out why the e2e tests are failing and propose a solution? |
Contributor
getdave
reviewed
Dec 16, 2025
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
This works when I test it but the e2e tests are certainly not happy. Let's see what happens.
packages/block-library/src/navigation-link/shared/use-handle-link-change.js
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where selecting a page from the LinkPicker in the Navigation block would overwrite an existing custom link label. The fix ensures that when a link already has a label, it is preserved even when adding or changing the URL.
Key changes:
- Simplified the condition for including title from checking both URL and label to only checking if a label exists
- Added comprehensive unit test coverage for the specific scenario of adding a URL to a link with an existing label but no URL
- Updated and enabled E2E test to verify the fix works end-to-end
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/block-library/src/navigation-link/shared/use-handle-link-change.js |
Changed the condition from ! attributes.url || ! attributes.label to ! attributes.label to preserve custom labels when updating links |
packages/block-library/src/navigation-link/shared/test/use-handle-link-change.test.js |
Updated mock implementation to be more realistic, updated test description for clarity, and added new test case for preserving label when adding URL to link with existing label |
test/e2e/specs/editor/blocks/navigation.spec.js |
Simplified test by selecting the Empty Link directly via list view, and uncommented the assertion to verify that the custom label is preserved after adding a URL |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Thanks for the review - the new test was failing because of a block selection issue, not because of the code. I fixed that and added a check to also replace empty string titles like you suggested. The rest of the e2e test failures look unrelated, so I'm rerunning tests and seeing if they pass. |
getdave
approved these changes
Dec 16, 2025
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
Works for me. Let's bring it in!
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?
Closes #74005
Fix an issue where selecting a page from the LinkPicker would overwrite a custom link label.
Why?
LinkPicker should respect the exiting label/title if present, not overwrite it.
How?
Changes the condition in useHandleLinkChange from checking both url and label to only checking if a label exists, ensuring custom labels are always preserved when present.
Testing Instructions
<!-- wp:navigation-link {"label":"Empty Link"} /-->Testing Instructions for Keyboard
Screenshots or screencast