WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 10 months ago

#24146 accepted defect (bug)

Menu items with blank labels are removed on saving

Reported by: rodrigo@… Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5.1
Component: Menus Keywords: dev-feedback has-patch needs-testing
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Hello,

When edit an item of menu leaving the label in blank, the item is auto deleted.
There are two problems in that:

  1. I could need an item with blank title to add some class with an background image
  2. When this item has subitems with two or more depth, all subitems are moved to first depth loosing submenus hierarchy. Moreover if i try to drag the subitems to make the hierarchy again, after save, all subitems come back to first depth. While i not change the depth of first item this issue occurs again.

Best regards

Attachments (5)

24146.patch (2.5 KB) - added by vinod dalvi 7 years ago.
Patch Added.
24146-2.patch (1.8 KB) - added by LeoPeo 6 years ago.
revision of patch 2
24146.diff (1.8 KB) - added by MikeHansenMe 6 years ago.
Refresh plus code standard formatting updates
24146.2.diff (1.9 KB) - added by SergeyBiryukov 6 years ago.
24146.3.diff (2.0 KB) - added by vinod dalvi 3 years ago.

Download all attachments as: .zip

Change History (28)

#1 @rodrigo@…
8 years ago

  • Cc rodrigo@… added

#2 @johnbillion
8 years ago

  • Component changed from General to Menus

#4 @SergeyBiryukov
7 years ago

Related: #23051, #25214.

I had the impression that it's a duplicate of #23051, but it's not. Menu items with blank titles are still deleted on saving.

#5 @SergeyBiryukov
7 years ago

  • Description modified (diff)
  • Summary changed from Auto delete of blank labels menu itens to Menu items with blank labels are removed on saving

#6 @SergeyBiryukov
7 years ago

#25218 was marked as a duplicate.

#7 @vinod dalvi
7 years ago

  • Cc mozillavvd@… added
  • Keywords has-patch needs-testing 2nd-opinion added

Patch Added.

@vinod dalvi
7 years ago

Patch Added.

@LeoPeo
6 years ago

revision of patch 2

#8 @LeoPeo
6 years ago

Hello,
I've tested the patch 24146 and it works!

Right now, this is what I see:

https://www.dropbox.com/s/bzd5j6i5g0yprml/24146.png?dl=0

WP: 4.0
Language:Italian

As you can see elements has no label and also element heigh is affected.

I've patched in this way:

1) If $title is not set, $title: (no label)
2) If $title is not set, I add to .menu-item-title the following class so it's styled like you delete the title in preview.

Version 0, edited 6 years ago by LeoPeo (next)

#9 @SergeyBiryukov
6 years ago

#29729 was marked as a duplicate.

#10 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 4.1

#11 @SergeyBiryukov
6 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#12 @johnbillion
6 years ago

  • Keywords 4.2-early added; needs-testing 2nd-opinion removed
  • Milestone changed from 4.1 to Future Release

#13 @iseulde
6 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

@MikeHansenMe
6 years ago

Refresh plus code standard formatting updates

This ticket was mentioned in Slack in #core by drew. View the logs.


6 years ago

#15 @DrewAPicture
6 years ago

  • Keywords needs-patch added; has-patch 4.2-early removed

In testing 24146.diff, I noticed a couple of standout issues:

  • With the patch applied, the menus screen is tossing a notice:
    PHP Notice:  Undefined variable: no_title_class in /srv/www/core/mutrunk/src/wp-admin/includes/nav-menu.php on line 109
    
  • Creating and saving ordinary menu links with no label works as expected, though it looks like the same is not true for other types of menu items. Removing labels on category items, for instance, the label is simply restored on save.
Last edited 6 years ago by DrewAPicture (previous) (diff)

#16 @DrewAPicture
6 years ago

The latest patch gets us part-way toward a fix here, just need to new patch addressing the feedback in comment:15. We'll need the new patch pretty soon to remain in consideration for 4.2 inclusion.

#17 @SergeyBiryukov
6 years ago

24146.2.diff fixes the notice and uses an existing string, but still doesn't look like a complete solution.

As noted in comment:15, if you remove the label for a post type or category item, the change is not saved and the label is restored, making the first part of the patch irrelevant.

Looks like 24146.patch attempted to solve this, but it no longer applies cleanly and seems to break the labels of existing menu items when those changes are recreated manually. Also need to take [24560] into account.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#18 @alexgarces
6 years ago

  • Keywords dev-feedback added

You can make a blank label adding the string " " at the label field. Doing it this way, the item is not removed from the menu.

#19 @SergeyBiryukov
6 years ago

  • Milestone changed from 4.2 to Future Release

#20 @SergeyBiryukov
5 years ago

#22070 was marked as a duplicate.

#21 @ocean90
5 years ago

In 32895:

Customizer: Fix live preview for menu item titles.

Show also a default label for menu items without a label which are assigned to a menu. This is currently only supported in the Customizer, see #24146 for nav menus screen.

see #32576.

@vinod dalvi
3 years ago

#22 @vinod dalvi
3 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Updated patch to make it work with latest code changes.

The patch doesn't handle customizer menu as it is handled in another ticket.

#23 @ElectricFeet
10 months ago

Reminder to self: check Trac exhaustiveley before writing out a bug report.

I'll add my text here anyway, as it elaborates on some of the reasoning behind this issue -- specifically giving a scenario where this happens. Reading through the various comments above and noting that it's taking so long to fix, you might want to consider at least taking approach B (below) to the problem. That is, at least warn the user you're about to delete their work before deleting it.

Original aim:

To create menu items that shows icons with no text.

Problem:

Menu items can be deleted with no warning.

Steps to reproduce problem:

  1. Create new menu
  2. Create two new custom links (for example to https://example.com), with Link Text test1 and test2, adding each to the menu
  3. Save the menu.
  4. On the Manage Locations tab, add this menu to a theme location you will be able to see and save your changes.
  5. View site and check that the two menu items are there.
  6. Go back to Edit Menus. If the CSS Classes box is not available, enable it from the Screen Options pull down (top right of screen)
  7. Open the test1 menu item and add fa fa-envelope to the CSS classes.
  8. Delete the navigation label and save the menu.

The menu item test1 disappears and there is no way to get it back. The site does not display it. There was no warning or other indication to the user that the item was to be deleted. If this has been done for a lot of menu items, a lot of time/work is lost.

I should point out that I know that I can add <span class="fa fa-envelope"></span> to the navigation label to get the result I need; my issue is with no warning of impending deletion if a user tries to do this in the way described above.

Recommendation:

  1. I recommend that the menu item should remain, with no navigation label (similar to what happens if you add &nbsp; to the menu). This allows the user to continue to modify it -- including to remove it, if that was the original intention. (It's very unlikely that users will want to delete menu items by removing the navigation label, when they have a red "Remove" link at the bottom.)
  1. If it isn't possible to implement A, then -- in the case that there are menu items without Navigation Labels when the user clicks Save Menu -- another approach would be to warn the user that menu items without Navigation Labels will be deleted, and allow them to cancel the save. That way, they will be able to re-instate the navigation labels and not lose all their menu items.

Personally, I much prefer A. It gives the user more flexibility, behaviour is not unexpected, and allows more creative use of pseudo elements / classes etc.

Note: See TracTickets for help on using tickets.