[wp-trac] [WordPress Trac] #47056: Add 'wp_nav_menu_item_custom_fields' hook in Walker_Nav_Menu_Edit::start_el()

[WordPress Trac]

#47056: Add 'wp_nav_menu_item_custom_fields' hook in
Walker_Nav_Menu_Edit::start_el()
------------------------------------+------------------------------
 Reporter:  MikeSchinkel            |       Owner:  (none)
     Type:  enhancement             |      Status:  new
 Priority:  normal                  |   Milestone:  Awaiting Review
Component:  Menus                   |     Version:  5.2
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |     Focuses:
------------------------------------+------------------------------

Comment (by birgire):

 Replying to [comment:9 MikeSchinkel]:
 > Replying to [comment:4 birgire]:
 > > Some plugins seems to add the nav menu ID to the hook, like
 [https://plugins.trac.wordpress.org/browser/sku-shortlink-for-
 woocommerce/trunk/vendor/wponion/wponion/core/wp/nav-menu/class-
 walker.php#L80 here] and [https://plugins.trac.wordpress.org/browser/menu-
 icons/trunk/vendor/codeinwp/menu-item-custom-fields/walker-nav-menu-
 edit.php#L77 here].
 >
 > Just a follow up on this.  Looking at core source, that parameters is
 commented [https://github.com/WordPress/wordpress-develop/blob/master/src
 /wp-admin/includes/class-walker-nav-menu-edit.php#L56 to be unused], which
 is why I did not include in my patch.
 >
 > I personally don't have an issue to include it, if some plugin actually
 does pass it, but I visually tracied the code I don't see how it would
 pass it. I think that might have been a legacy parameter that is no longer
 used.
 >
 > But if it is in use, okay my me to include. OTOH if it does not, would
 be a shame to include it.
 >

 Thanks for bringing that up. I noticed this when looking at the inline
 docs for the patch, but my understanding was that it meant that it was not
 used within the method. (Maybe these lines could be better explained in
 another doc ticket)

 What I also had in mind was that if we would e.g. only use 3 input
 arguments for the new hook, then plugins might get PHP notices for
 callbacks that expect 4 or 5 inputs.

 But we should definitely look better into that.

 PS:

 For referencing here is the inline documentation for the related
 {{{start_el()}}} methods.

 One would expect same types for the inputs (e.g. the $args), but I guess
 it might need to be rechecked and synced e.g. in another ticket.

 https://core.trac.wordpress.org/browser/tags/5.2/src/wp-admin/includes
 /class-walker-nav-menu-edit.php#L44

 {{{
 class Walker_Nav_Menu_Edit extends Walker_Nav_Menu {

         // ... cut ...

         /**
          * Start the element output.
          *
          * @see Walker_Nav_Menu::start_el()
          * @since 3.0.0
          *
          * @global int $_wp_nav_menu_max_depth
          *
          * @param string $output Used to append additional content (passed
 by reference).
          * @param object $item   Menu item data object.
          * @param int    $depth  Depth of menu item. Used for padding.
          * @param array  $args   Not used.
          * @param int    $id     Not used.
          */
         public function start_el( &$output, $item, $depth = 0, $args =
 array(), $id = 0 ) {

 }}}

 https://core.trac.wordpress.org/browser/tags/5.2/src/wp-includes/class-
 walker-nav-menu.php#L104

 {{{
 class Walker_Nav_Menu extends Walker {

         // ... cut ...

         /**
          * Starts the element output.
          *
          * @since 3.0.0
          * @since 4.4.0 The {@see 'nav_menu_item_args'} filter was added.
          *
          * @see Walker::start_el()
          *
          * @param string   $output Used to append additional content
 (passed by reference).
          * @param WP_Post  $item   Menu item data object.
          * @param int      $depth  Depth of menu item. Used for padding.
          * @param stdClass $args   An object of wp_nav_menu() arguments.
          * @param int      $id     Current item ID.
          */
         public function start_el( &$output, $item, $depth = 0, $args =
 array(), $id = 0 ) {

 }}}

 https://core.trac.wordpress.org/browser/tags/5.2/src/wp-includes/class-wp-
 walker.php#L79

 {{{
 class Walker {

         // ... cut ...

         /**
          * Start the element output.
          *
          * The $args parameter holds additional values that may be used
 with the child
          * class methods. Includes the element output also.
          *
          * @since 2.1.0
          * @abstract
          *
          * @param string $output            Used to append additional
 content (passed by reference).
          * @param object $object            The data object.
          * @param int    $depth             Depth of the item.
          * @param array  $args              An array of additional
 arguments.
          * @param int    $current_object_id ID of the current item.
          */
         public function start_el( &$output, $object, $depth = 0, $args =
 array(), $current_object_id = 0 ) {}

 }}}


 Here are some relevant lines from {{{wp_get_nav_menu_to_edit()}}}
 function:

 {{{
 $walker_class_name = apply_filters( 'wp_edit_nav_menu_walker',
 'Walker_Nav_Menu_Edit', $menu_id );

 }}}

 https://core.trac.wordpress.org/browser/tags/5.2/src/wp-admin/includes
 /nav-menu.php#L1015


 {{{
 $result .= walk_nav_menu_tree( array_map( 'wp_setup_nav_menu_item',
 $menu_items ), 0, (object) array( 'walker' => $walker ) );

 }}}

 https://core.trac.wordpress.org/browser/tags/5.2/src/wp-admin/includes
 /nav-menu.php#L1049

 We should also checkout the relevant {{{display_element()}}} methods.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/47056#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform