Menu link validation #6389
Menu link validation #6389
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
src/js/_enqueues/lib/nav-menu.js
Outdated
| @@ -589,8 +589,11 @@ | |||
| }, | |||
|
|
|||
| initPreviewing : function() { | |||
|
|
|||
| const menuToEdit = $( '#menu-to-edit' ); | |||
There was a problem hiding this comment.
Make it Var instead of Const. Also remove extra line from top and bottom.
There was a problem hiding this comment.
- Resolved in 24598ed 🚀 .
src/js/_enqueues/lib/nav-menu.js
Outdated
| // Show warning when url is empty/invalid in custom menu item type. | ||
| menuToEdit.on( 'change input', '.edit-menu-item-url', function( e ) { | ||
|
|
||
| const input = $( e.currentTarget ); |
There was a problem hiding this comment.
Same as before, use var instead of const. Also instead of declaring var multiple times, make it comma-separated. For Reference check codes from 597 to 599 above. Also no extra empty line.
There was a problem hiding this comment.
- Resolved in 24598ed 🚀 .
There was a problem hiding this comment.
@Pathan-Amaankhan Var code needs to be something like this:
src/js/_enqueues/lib/nav-menu.js
Outdated
| if ( '' === url || 'https://' === url || 'http://' === url ) { | ||
|
|
||
| urlEl.addClass( 'form-invalid' ); | ||
| urlEl.attr( 'placeholder', 'https://' ); |
There was a problem hiding this comment.
We don't need to change the placeholder value in any case. So remove this line from both the If and else conditions.
There was a problem hiding this comment.
- Resolved in 24598ed 🚀 .
src/js/_enqueues/lib/nav-menu.js
Outdated
| @@ -1063,12 +1086,35 @@ | |||
| }, | |||
|
|
|||
| attachMenuSaveSubmitListeners : function() { | |||
| const updateNavMenuEl = $( '#update-nav-menu' ); | |||
There was a problem hiding this comment.
Same var instead const, Remove empty line.
There was a problem hiding this comment.
- Resolved in 24598ed 🚀 .
| /* | ||
| * When a navigation menu is saved, store a JSON representation of all form data | ||
| * in a single input to avoid PHP `max_input_vars` limitations. See #14134. | ||
| */ | ||
| $( '#update-nav-menu' ).on( 'submit', function() { | ||
| var navMenuData = $( '#update-nav-menu' ).serializeArray(); | ||
| updateNavMenuEl.on( 'submit', function() { |
There was a problem hiding this comment.
- Resolved in 24598ed 🚀 .
|
@Pathan-Amaankhan Can you make the changes as per suggestions? That will be great. |
|
Hi @Rcreators 👋, |
Trac ticket: https://core.trac.wordpress.org/ticket/60916
Description
Custom Linkmenu items.Steps to reproduce Bug
If we try adding
Custom Linkwith an empty URL in the Menu, We get a validation error & are not able to addCustom Link.Screen.Recording.2024-04-14.at.8.22.54.PM.mov
But if we try the same after adding
Custom Linkto the Menu, we don't get the validation error & can addCustom Linkwith an empty URL. This breaches the consistency.Screen.Recording.2024-04-14.at.8.31.05.PM.mov
Expected behaviour
Custom Linkwith an empty URL.Screenshots/Screencasts
Before
Able to save
Custom Linkwith an empty URL.Screen.Recording.2024-04-14.at.8.31.05.PM.mov
After
A validation error is displayed if we try to save
Custom Linkwith an empty URL.Screen.Recording.2024-04-14.at.8.36.16.PM.online-video-cutter.com.mp4
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.