WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#23076 closed enhancement (fixed)

Update menu item title when editing menu item label

Reported by: tillkruess Owned by: helen
Milestone: 4.0 Priority: normal
Severity: minor Version: 3.5
Component: Menus Keywords: has-patch commit
Focuses: javascript Cc:
PR Number:

Description

I'd like to suggest that the menu item title gets updated when the user is editing the "Navigation Label" field. That results in a nicer preview, when the menu item is collapsed again before saving.

Attachments (4)

23076.diff (2.3 KB) - added by celloexpressions 5 years ago.
Live-update the menu item handle title when the corresponding navigation label field is changed.
23076.2.diff (2.3 KB) - added by wonderboymusic 5 years ago.
23076.3.diff (642 bytes) - added by helen 5 years ago.
23076.4.diff (639 bytes) - added by SergeyBiryukov 5 years ago.

Download all attachments as: .zip

Change History (20)

#1 @wonderboymusic
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

With a patch, this can get moved to 4.0

@celloexpressions
5 years ago

Live-update the menu item handle title when the corresponding navigation label field is changed.

#2 @celloexpressions
5 years ago

  • Keywords has-patch added; needs-patch removed

I implemented something similar to this for menu names in my Menu Customizer project. This is a nice improvement and helps reinforce the relationship between the navigation label field and the item.

#3 @wonderboymusic
5 years ago

  • Milestone changed from Future Release to 4.0

23076.2.diff deals with some JSHint and also uses delegation so the items don't have to already be on the page on load.

#4 @wonderboymusic
5 years ago

In 28707:

Live update the menu item title when the user is editing the "Navigation Label" field.

Props celloexpressions.
See #23076.

#5 @wonderboymusic
5 years ago

Leaving open in case someone has an opinion about the translation stuff.

#6 @DrewAPicture
5 years ago

  • Focuses javascript added

hawt.

#7 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Fixed in [28789].

#8 @nacin
5 years ago

I'm concerned about the performance of [28707]:

  • This is a huge DOM and we're letting every input event bubble all the way up to the body. Not good.
  • The 'input' event is buggy in IE9 and nonexistent before then.

Can we also double-check this doesn't allow for self-XSS?

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

#9 @nacin
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#10 @celloexpressions
5 years ago

We should be able to change $( 'body' ).on() to $( '#menu-to-edit' ).on() to help with performance (that's the direct parent of each menu item).

I wouldn't be worried if this doesn't work in old IE, since they'd just have the same situation as 3.9, but does it break things?

I have no idea about security implications here.

@helen
5 years ago

#11 @helen
5 years ago

23076.3.diff reels in the bubbling to #menu-to-edit and adds propertychange to the events - we do this elsewhere for IE < 9. Self-XSS is not an issue here, due to the use of .text().

#12 @helen
5 years ago

Further testing in IE8 and 9 indicates that propertychange and selectionchange, which I added for IE9's deletion issues in testing, are not firing at all. I haven't unraveled what's going on.

I'm comfortable just saying it doesn't work in IE8, but it's a little weird that it works except for deleting in IE9.

#13 @nacin
5 years ago

  • Keywords commit added

This was an enhancement and I'm most concerned about any performance regressions versus getting it to work in IE. 23076.3.diff is fine. We could remove 'propertychange' if it's confirmed it doesn't work in IE.

#14 @nacin
5 years ago

  • Owner set to helen
  • Status changed from reopened to assigned

Fine to be committed.

#15 @SergeyBiryukov
5 years ago

Adding propertychange seems to cause ~90 second delays for me when editing menu items in IE8.

I'd suggest adding change instead: 23076.4.diff.

#16 @ocean90
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 29672:

Menus: Improve performance for title live updating. Add change event for IE < 9.

props helen, SergeyBiryukov.
fixes #23076.

Note: See TracTickets for help on using tickets.