WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 4 months ago

#40666 reviewing enhancement

allow arbitrary HTML attributes on <a> produced by Walker_Category

Reported by: pbiron Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: dev-feedback has-patch
Focuses: accessibility Cc:

Description

Walker_Nav_Menu currently allows arbitrary HTML attributes to be added to the <a> tags it generates via the nav_menu_link_attributes filter.

It would be useful to do the same in Walker_Category (i.e., add a category_menu_link_attributes filter).

Related: #40359

Attachments (1)

40666.diff (2.1 KB) - added by pbiron 5 months ago.

Download all attachments as: .zip

Change History (8)

@pbiron
5 months ago

#1 @pbiron
5 months ago

  • Keywords dev-feedback has-patch added

To the best of my knowledge, in core Walker_Category is only used by wp_list_categories() and wp_list_categories() is only used by `WP_Widget_Categories'.

As a result, I'm not sure that the name of new filter introduced in my patch (category_menu_link_attributes) is appropriate. I named it that to mirror the new filter introduced in #40359...which was named after the existing filter in Walker_Nav_Menu.

I'd like some feedback on the naming (and inline docs) for this new filter.

#2 @pbiron
5 months ago

  • Focuses accessibility added

Another thing I noticed while putting together this patch is that there is a difference in the way wp_list_categories() and wp_list_pages() invoke Walker::walk() (via walk_category_tree() and walk_page_tree(), respectively).

Namely, wp_list_pages() passes the current page as a param to walk_page_tree(), whereas wp_list_categories() does not pass the current category as a param to walk_category_tree(), passing it instead $r['current_category'].

As a result, those hooking into the new filter proposed here need to write their funcs slightly different than the similar filters for nav_menu_link_attributes and page_menu_link_attributes, as in:

add_filter( 'category_menu_link_attributes', 'add_aria_current_category', 10, 5 );

function
add_aria_current_category( $atts, $category, $depth, $args, $id )
{
	if ( $category->term_id === $id ||
			( isset( $args['current_category'] ) &&
			$category->term_id === $args['current_category'] ) ) {
		$atts['aria-current'] = 'page' ;
		}

	return $atts ;
}

I think it might make sense to include a note about this different in the inline docs (resulting in the note appearing in DevHub. Alternatively, the note could be added a "comment" on the DevHub page for the new filter. I'd like feedback on this as well.

Last edited 5 months ago by pbiron (previous) (diff)

This ticket was mentioned in Slack in #accessibility by pbiron. View the logs.


5 months ago

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


5 months ago

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


5 months ago

#6 @afercia
4 months ago

@SergeyBiryukov since you already owned #40359 would you like to own this too? :)

#7 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.