WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 4 weeks ago

#19647 closed enhancement (fixed)

Use add_node() instead of add_menu() in core

Reported by: linuxologos Owned by: chriscct7
Milestone: 5.4 Priority: low
Severity: normal Version: 3.3
Component: Toolbar Keywords: has-patch commit
Focuses: Cc:
PR Number:

Description (last modified by chriscct7)

'It's clear through class-wp-admin-bar.php and Nacin has stated this too:

The API previously emphasized add_menu(), but this can be confusing, so add_node() is now being promoted a bit more.

Another step towards this would be that the core used add_node() instead of add_menu() when building menu items in admin-bar.php.

Proposed patch replaces all references to the add_menu() method with the add_node() one. (Also some code structure modifications for wp_admin_bar_appearance_menu().)

Attachments (7)

19647.patch (11.6 KB) - added by linuxologos 8 years ago.
19647.2.patch (12.3 KB) - added by paulschreiber 4 years ago.
add_node() changes only; no formatting changes
19647.diff (14.8 KB) - added by morganestes 4 years ago.
Adds a deprecated notice to add_menu() and directs use of add_node()
19647.2.diff (13.0 KB) - added by akibjorklund 3 years ago.
Refreshed patch
19647.3.diff (15.1 KB) - added by morganestes 5 weeks ago.
Refreshed and added docs to include remove_menu
19647.4.diff (16.1 KB) - added by morganestes 5 weeks ago.
Add unit tests for refreshed patch.
19647.5.diff (2.6 KB) - added by whyisjake 4 weeks ago.

Download all attachments as: .zip

Change History (20)

@linuxologos
8 years ago

#1 @nacin
7 years ago

  • Owner set to ryan
  • Status changed from new to assigned

Ryan, you game?

#2 @ryan
5 years ago

  • Owner ryan deleted

#3 @chriscct7
4 years ago

  • Description modified (diff)
  • Keywords needs-refresh added; 2nd-opinion removed
  • Severity changed from minor to normal

#4 @johnbillion
4 years ago

  • Keywords needs-patch added; needs-refresh has-patch removed
  • Priority changed from normal to low

Needs a patch minus the unnecessary formatting changes.

@paulschreiber
4 years ago

add_node() changes only; no formatting changes

#5 @paulschreiber
4 years ago

  • Keywords has-patch added; needs-patch removed

#6 @chriscct7
4 years ago

  • Owner set to chriscct7
  • Status changed from assigned to reviewing

@morganestes
4 years ago

Adds a deprecated notice to add_menu() and directs use of add_node()

@akibjorklund
3 years ago

Refreshed patch

#7 @swissspidy
3 years ago

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

@morganestes
5 weeks ago

Refreshed and added docs to include remove_menu

@morganestes
5 weeks ago

Add unit tests for refreshed patch.

#8 @whyisjake
5 weeks ago

  • Keywords needs-dev-note commit added; needs-testing removed

Looks great Morgan, going to get this committed.

#9 @whyisjake
5 weeks ago

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

In 46642:

Toolbar: Use add_node() instead of add_menu() in core.

This patch replaces all references to the add_menu() method with the add_node() one. (Also some code structure modifications for wp_admin_bar_appearance_menu().)

Fixes: #19647
Props: linuxologos, paulschreiber, morganestes, akibjorklund, nacin, whyisjake.

#10 @johnbillion
4 weeks ago

  • Keywords 2nd-opinion added; commit removed
  • Milestone changed from Future Release to 5.4
  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm not sure that this deprecated notice provides much value. Most plugins that add an admin toolbar menu are using the add_menu() method, as was core until this changeset.

A search for >add_menu( in the plugin directory returns over 1,200 results: https://wpdirectory.net/search/01DS0ZDS11A6JZEF4VH3SNVZAF

I'd like to propose removing the deprecated notice for this method. It might have made sense eight years ago but it now seems like an unnecessary deprecation given its widespread usage.

@whyisjake
4 weeks ago

#11 @whyisjake
4 weeks ago

@johnbillion, I removed the test and the deprecated warnings, thoughts?

#12 @johnbillion
4 weeks ago

  • Keywords commit added; needs-dev-note 2nd-opinion removed

Thanks Jake! I'm going to make an executive decision and go ahead with this as per my comment above.

#13 @johnbillion
4 weeks ago

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

In 46678:

Toolbar: Un-deprecate the WP_Admin_Bar::add_menu() method.

This is only a wrapper for the add_node() method, but it's in widespread use both in core until [46642] and in thousands of plugins and themes. Deprecating it would have made sense when #19647 was originally opened but that's no longer the case.

Props whyisjake

Fixes #19647

Note: See TracTickets for help on using tickets.