WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#29463 closed enhancement (fixed)

Add filter in WP_Nav_Menu_Widget

Reported by: cyman Owned by: SergeyBiryukov
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.0
Component: Widgets Keywords: has-patch 4.2-early commit
Focuses: administration Cc:
PR Number:

Description

There is currently no way to change the arguments of function wp_nav_menu( array( 'fallback_cb' => '', 'menu' => $nav_menu ) ); in file wp-includes/default-widgets.php for class WP_Nav_Menu_Widget as there is no hook applied to the argument.

Suggested solution:

wp_nav_menu( apply_filters( 'nav_menu_widget_args', array( 
  'fallback_cb' => '', 
  'menu' => $nav_menu 
) ) );

Attachments (4)

filter-wp_nav_menu-widgets.diff (611 bytes) - added by cyman 5 years ago.
filter-wp_nav_menu-widgets.2.diff (818 bytes) - added by cyman 5 years ago.
Added the inline documentation.
29463.diff (1.0 KB) - added by DrewAPicture 5 years ago.
29463.2.diff (1.2 KB) - added by DrewAPicture 5 years ago.

Download all attachments as: .zip

Change History (24)

#1 @cyman
5 years ago

I am sure I could create a patch for this (I am theme and plugin developer), but it will be first time for me, so I will appreciate instructions/link how to do that.

#2 @johnbillion
5 years ago

  • Focuses template removed
  • Keywords reporter-feedback added
  • Version changed from trunk to 3.0

Thanks for the report cyman.

The wp_nav_menu() function contains a filter which gets applied to the arguments for all nav menus (see here).

$args = apply_filters ( 'wp_nav_menu_args', array $args );

Is this filter sufficient for your use case? You can target the menu using the menu parameter. Eg:

add_filter( 'wp_nav_menu_args', function( array $args ) {
    if ( 'my_menu' == $args['menu'] ) {
        // do something
    }
    return $args;
} );

If this isn't sufficient, and you think we might need a more targeted filter for the WP_Nav_Menu_Widget, then you can read up about Subversion and creating a patch on the SVN page of the WordPress handbook. Once you've created a patch file you can upload it to this ticket.

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

#3 follow-up: @cyman
5 years ago

Thank you for a quick answer @johnbillion!

Yes, I am aware of the wp_nav_menu_args filter and how to target specific menus with if clauses.

However, this feels to me more a hack than elegant solution, especially in some cases (and I found other questioning the same thing elsewhere) when one'd like a specific menu classes or walker applied to all custom menus that appear in the sidebars.

Example usecase: I use the Tw Bootstrap for creating the theme, so I would like to apply the Bootstrap Navwalker and some Bootstrap classes to all the sidebar menus. While I don't want to use the navwalker for the custom menu positions in my theme (for example main navigation in header and footer navigation).

What I have to do now to achieve that is to register a new widget (which extends the WP_Nav_Menu_Widget) and define new widget method just to change the arguments of the wp_nav_menu function call. And then use unregister_widget to unregister default menu widget.

It would be much easier to add a filter in a theme to change these arguments for the existing custom menu widget. That would also be more consistent how other widgets/functions in the file default-widgets.php have hooks applied, for example the WP_Widget_Pages:

wp_list_pages( apply_filters( 'widget_pages_args', array(
	'title_li'    => '',
	'echo'        => 0,
	'sort_column' => $sortby,
	'exclude'     => $exclude
) ) );

I hope I explained my point.

I'll take a look at the SVN instructions (I use git daily, so I hope I don't have troubles) you linked to and create a patch file.

#4 in reply to: ↑ 3 @johnbillion
5 years ago

Replying to cyman:

I'll take a look at the SVN instructions (I use git daily, so I hope I don't have troubles) you linked to and create a patch file.

In that case, you can use Git to create an svn-compatible patch to make things easier for yourself.

#5 follow-up: @johnbillion
5 years ago

Related: #22364

#6 in reply to: ↑ 5 @cyman
5 years ago

I attached the patch file per these instructions.

Fingers crossed I did everything right :)

#7 follow-up: @SergeyBiryukov
5 years ago

  • Focuses docs added
  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 4.1

The filter needs to be documented as per the documentation standards.

#8 in reply to: ↑ 7 @cyman
5 years ago

Replying to SergeyBiryukov:

The filter needs to be documented as per the documentation standards.

I will handle the docs this week, ok?

@cyman
5 years ago

Added the inline documentation.

#10 @cyman
5 years ago

Online docs added, what's the next step? (fyi: I am contributing to the core for the first time.)

#11 follow-up: @DrewAPicture
5 years ago

  • Keywords dev-feedback added

Hi Cyman,

Thanks a lot for adding the hook documentation.

It occurs to me that instead of introducing a new filter here -- which would effectively be a duplicate of wp_nav_menu_args -- perhaps we should simply introduce a "context" parameter to the existing filter hook that would allow targeting of this particular context. Maybe something like 'menu-widget' (this context), vs 'none' or (main context).

#12 @DrewAPicture
5 years ago

  • Keywords has-patch added

#13 @DrewAPicture
5 years ago

  • Focuses docs removed
  • Keywords needs-docs added

#14 in reply to: ↑ 11 ; follow-up: @cyman
5 years ago

Replying to DrewAPicture:

It occurs to me that instead of introducing a new filter here -- which would effectively be a duplicate of wp_nav_menu_args -- perhaps we should simply introduce a "context" parameter to the existing filter hook that would allow targeting of this particular context. Maybe something like 'menu-widget' (this context), vs 'none' or (main context).

I would not agree, all the other native WP widgets have the filters applied. This was the only exception in default-widgets.php.

While adding a context to the existing filter sounds like a great idea, I think that we should then also consider doing the same for the other native WP widgets in the default-widgets.php file - keeping the code consistent, right?

@DrewAPicture
5 years ago

#15 in reply to: ↑ 14 @DrewAPicture
5 years ago

  • Keywords commit added; needs-docs removed

Replying to cyman:

While adding a context to the existing filter sounds like a great idea, I think that we should then also consider doing the same for the other native WP widgets in the default-widgets.php file - keeping the code consistent, right?

You were correct the first time. I think adding a completely new filter here falls more in-line with core conventions for widgets.

In 29463.diff I've broken the args array out to a separate variable for readability, as well as passed the $nav_menu object and $args array containing display arguments for the current widget. Access to $nav_menu and $args should make targeting specific widgets, menus, and sidebars much simpler.

#16 @johnbillion
5 years ago

  • Keywords 4.2-early added; commit removed
  • Milestone changed from 4.1 to Future Release
  • Type changed from enhancement to task (blessed)

This should go in but it's a bit late for 4.1.

#17 @iseulde
5 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

#18 @MikeHansenMe
5 years ago

  • Keywords dev-feedback removed

Patch still applies.

@DrewAPicture
5 years ago

#19 @DrewAPicture
5 years ago

  • Keywords commit added
  • Type changed from task (blessed) to enhancement

29463.2.diff fixes a couple of things missing in 29463.diff. Good to go.

#20 @SergeyBiryukov
5 years ago

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

In 31325:

Add 'widget_nav_menu_args' filter for Custom Menu widget arguments.

props cyman, DrewAPicture.
fixes #29463.

Note: See TracTickets for help on using tickets.