Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#38499 closed defect (bug) (fixed)

Customize: newly-added pages should show that they were added to the menu

Reported by: celloexpressions's profile celloexpressions Owned by: westonruter's profile westonruter
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: has-screenshots good-first-bug has-patch commit
Focuses: ui, javascript Cc:

Description

When adding new pages in menus, the pages should show as "checked" or added within the available nav menu items panel.

Attachments (3)

38499.PNG (37.6 KB) - added by celloexpressions 8 years ago.
Currently, the new items are added to the menu and the top of the available items list, but not shown as checked.
186.diff (1.3 KB) - added by ryankienstra 8 years ago.
alt-38499.diff (558 bytes) - added by ryankienstra 8 years ago.

Download all attachments as: .zip

Change History (12)

@celloexpressions
8 years ago

Currently, the new items are added to the menu and the top of the available items list, but not shown as checked.

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


8 years ago

#2 @celloexpressions
8 years ago

  • Focuses javascript added
  • Keywords good-first-bug added

We need to add the item-added class the the menu item handle in the newly-added item's template in the available nav menu items panel. Should be a one-line addition to customize-nav-menus.js.

#3 @ryankienstra
8 years ago

Working On A Patch

Hi @celloexpressions,
If it's alright, I'm working on this now. I'll have a patch ready soon.

#4 @celloexpressions
8 years ago

Awesome, thanks @ryankienstra!

@ryankienstra
8 years ago

#5 @ryankienstra
8 years ago

Request For Review

Hi @celloexpressions,
Could you please review the attached patch, or the identical GitHub pull request?

This passes the available-menu-item template a value of true for is_new_item. The template will then output the class item-added, as you suggested.

Background:
Adding a new page to a menu in the Customizer uses the template available-menu-item. Existing pages also use this template, so this can't simply output the class for all of them.

#6 @ryankienstra
8 years ago

  • Keywords has-patch added; needs-patch removed

#7 @ryankienstra
8 years ago

An Approach With Only JavaScript

Hi @celloexpressions,
Thanks for reviewing this PR. Is this patch what you had in mind in your comment?

#8 @celloexpressions
8 years ago

  • Keywords commit added
  • Owner set to westonruter
  • Status changed from new to reviewing

Thanks @ryankienstra, alt-38499.diff looks ready for commit. It's a bit simpler with that approach I think.

#9 @westonruter
8 years ago

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

In 39002:

Customize: Mark newly-added page/post stubs as added (with checkmark) when they are inserted into the list of available items.

Props ryankienstra, celloexpressions.
See #34923.
Fixes #38499.

Note: See TracTickets for help on using tickets.