WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 weeks ago

#38415 assigned defect (bug)

New Custom Link menu item has a wrong fallback label

Reported by: afercia Owned by: afercia
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.5
Component: Menus Keywords: has-screenshots good-first-bug has-patch
Focuses: ui Cc:

Description (last modified by desrosj)

On the menus screen, when adding a Custom Link to a menu and leaving the link text field empty, a default fallback Menu Item label was set:

https://cldup.com/xaDohbjzS9.png

On the latest versions though, the fallback label is (Pending), with a leading space:

https://cldup.com/zq1ti4qSnq.png

This was broken by [36379] my bad. Haven't looked in depth in the code but I think it could be fixed on the PHP side, I guess there's no need to restore the JS part that was removed in [36379].

Attachments (2)

38415.patch (692 bytes) - added by thakkarhardik 4 months ago.
As per the suggestion, I have changed the label. Please look over it and let me know if anything more is required.
38415.diff (608 bytes) - added by afercia 3 weeks ago.

Download all attachments as: .zip

Change History (11)

#1 follow-up: @christophherr
3 years ago

Would a feasible approach be to simply add Menu Item to
https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-walker-nav-menu-edit.php#L103?
e.g.:

$title = sprintf( __('%s Menu Item (Pending)'), $item->title );

https://cldup.com/exbrWmnc2t.png

Last edited 3 years ago by christophherr (previous) (diff)

#2 in reply to: ↑ 1 @Fencer04
3 years ago

I think having Menu Item (Pending) makes sense. I'm new to this but is there a reason we don't make the text field for Custom Links required?

#3 @desrosj
4 months ago

  • Description modified (diff)
  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

@thakkarhardik
4 months ago

As per the suggestion, I have changed the label. Please look over it and let me know if anything more is required.

#4 @thakkarhardik
4 months ago

  • Keywords has-patch added; needs-patch removed

#5 @backermann1978
4 weeks ago

Works fine with my local VVV-setup and fixes the issue (tested on WordCamp EU Contributor Day).

#6 @antpb
4 weeks ago

  • Keywords commit added

#7 @afercia
4 weeks ago

  • Milestone changed from Future Release to 5.3
  • Owner set to afercia
  • Status changed from new to assigned

#8 @afercia
3 weeks ago

  • Keywords commit removed

Thanks everyone! I tried to look a bit more in depth into this and there are a few things to consider. Testing with JavaScript support disabled and enabled:

JavaScript disabled:
When adding menu items, an additional string (Pending) gets appended to the item name. Also, a notice "Click Save Menu..." appears at the top:

http://cldup.com/dXLp0ChW_X.png

Looks like this part was never implemented in the JavaScript-based implementation.

JavaScript enabled:
The additional string (Pending) is intentionally not displayed. Also the notice doesn't appear.

To respect the original implementation, the string (Pending) shouldn't be displayed. I'd like to propose to just restore the original behavior: when a custom link text is empty, a default "Menu Item" string is used as fallback.

I've noticed a couple inconsistencies in the PHP part and I'm not sure the Custom Links are fully functional when JavaScript is disabled but seems to me those are pre-existing issues. Considering the Menu page will be refactored soon to use blocks, I'd tend to think the best option is to restore a mechanism to have the fallback to athe"Menu Item" string.

@afercia
3 weeks ago

#9 @afercia
3 weeks ago

38415.diff adds a fallback to Menu Item in the ajax action used for the JS implementation. This shoudl restore the original behavior as detailed above. Some testing would be nice :)

Note: See TracTickets for help on using tickets.