WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 4 months ago

Last modified 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:

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 4 years ago.
Refreshed patch
19647.3.diff (15.1 KB) - added by morganestes 4 months ago.
Refreshed and added docs to include remove_menu
19647.4.diff (16.1 KB) - added by morganestes 4 months ago.
Add unit tests for refreshed patch.
19647.5.diff (2.6 KB) - added by whyisjake 4 months ago.

Download all attachments as: .zip

Change History (22)

@linuxologos
8 years ago

#1 @nacin
7 years ago

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

Ryan, you game?

#2 @ryan
6 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
4 years ago

Refreshed patch

#7 @swissspidy
3 years ago

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

@morganestes
4 months ago

Refreshed and added docs to include remove_menu

@morganestes
4 months ago

Add unit tests for refreshed patch.

#8 @whyisjake
4 months ago

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

Looks great Morgan, going to get this committed.

#9 @whyisjake
4 months 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 months 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 months ago

#11 @whyisjake
4 months ago

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

#12 @johnbillion
4 months 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 months 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

#14 @afercia
6 weeks ago

@johnbillion when you have a chance: both add_menu() and remove_menu() still have a @since notation mentioning the deprecation. Should those be removed as well?

* @since 5.4.0 Deprecated in favor of {@see WP_Admin_Bar::add_node()}.
* @since 5.4.0 Deprecated in favor of {@see WP_Admin_Bar::remove_node()}.

#15 @afercia
4 weeks ago

In 47108:

Toolbar: Remove leftover @since notations after [46678].

See #19647.

Note: See TracTickets for help on using tickets.