Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#23076 closed enhancement (fixed)

Update menu item title when editing menu item label

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

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 10 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 10 years ago.
23076.3.diff (642 bytes) - added by helen 10 years ago.
23076.4.diff (639 bytes) - added by SergeyBiryukov 10 years ago.

Download all attachments as: .zip

Change History (20)

#1 @wonderboymusic
10 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
10 years ago

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

#2 @celloexpressions
10 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
10 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
10 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
10 years ago

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

#6 @DrewAPicture
10 years ago

  • Focuses javascript added

hawt.

#7 @wonderboymusic
10 years ago

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

Fixed in [28789].

#8 @nacin
10 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 10 years ago by SergeyBiryukov (previous) (diff)

#9 @nacin
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#10 @celloexpressions
10 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
10 years ago

#11 @helen
10 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
10 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
10 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
10 years ago

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

Fine to be committed.

#15 @SergeyBiryukov
10 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
10 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.