WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 5 months ago

#41859 closed defect (bug) (fixed)

Add aria-current to the paginated posts links output by wp_link_pages()

Reported by: afercia Owned by: afercia
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Themes Keywords: aria-current has-patch
Focuses: accessibility Cc:

Description

When using the <!--nextpage--> tag one or multiple times in post, wp_link_pages() generates pagination links for the post. In the set of pagination links, aria-current would help assistive technologies users orientate themselves and understand which the current active "page" is.

See also #40833 that introduced aria-current for in paginate_links(), typically used on archive pages for posts navigation.

The actual links markup gets generated by _wp_link_page(). Worth noting this markup could be improved also for other things, for example there's no "current" CSS class used on the active paeg number.

Themes usually add their own markup around the links to improve things a bit. For example, Twenty Seventeen uses this:

wp_link_pages( array(
	'before'      => '<div class="page-links">' . __( 'Pages:', 'twentyseventeen' ),
	'after'       => '</div>',
	'link_before' => '<span class="page-number">',
	'link_after'  => '</span>',
) );

However, some improvements to the default output would be a nice improvement.

Attachments (4)

41859.patch (2.2 KB) - added by antonioeatgoat 7 months ago.
Improved default parameters and highlighted current item
41859.2.patch (2.5 KB) - added by antonioeatgoat 6 months ago.
New patch, following suggestions of #comment:7. Unit tests are still missing.
41859.diff (4.7 KB) - added by afercia 6 months ago.
41859.2.diff (4.8 KB) - added by afercia 5 months ago.

Download all attachments as: .zip

Change History (15)

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


7 months ago

#2 @afercia
7 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.0

@antonioeatgoat
7 months ago

Improved default parameters and highlighted current item

#3 @antonioeatgoat
7 months ago

  • Keywords dev-feedback added

The patch uses by default some of the parameters used by Twenty Seventeen, however this upsets the theme, because there is a repeated nested span with the same class. So likely not the best way to achieve it, maybe changing the class name used by default in the function would be a solution.

Then it forces the usage of a class to wrap the numbers, as wp_link_pages() does. I am not a big fun of the solution str_replace( '<a href="', '<a class="page-number" href="', $link ); , however it's the only way that I see for now with the current structure of the function. It would be very nice to improve consistency between paginate_links() and wp_link_pages() because they use parameters and CSS class positions/names in a quite different way, despite they do basically the same thing.

Last edited 7 months ago by antonioeatgoat (previous) (diff)

#4 @afercia
7 months ago

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

#5 @afercia
7 months ago

  • Keywords has-patch added; needs-patch removed

#6 @alexstine
7 months ago

I just tested the patch with the Twenty Seventeen theme on WP 4.9. It's working fine. The pagination is more accessible this way.

#7 @afercia
6 months ago

  • Keywords dev-feedback removed

@antonioeatgoat thanks for the patch and @alexstine thanks for testing!

A few things:

  • there should be a @since notation for the new aria_current argument, as done for example in https://core.trac.wordpress.org/changeset/41371
  • I wouldn't change the default paragraph tag in a div, this might produce some layout/css breakage for themes that expect a paragraph
  • to avoid str_replace(), what about to add the CSS class in the private function _wp_link_page()? Seems the simplest solution to me, then add the same CSS class in the new span element
  • I'd use a page-link name for the CSS class, to avoid conflicts
  • maybe use === instead of == to compare the index with the page number?
  • in tests/phpunit/tests/post/template.php there are a few tests to update, this can also be done later :)

@antonioeatgoat
6 months ago

New patch, following suggestions of #comment:7. Unit tests are still missing.

#8 @antonioeatgoat
6 months ago

@afercia, to add the CSS class to _wp_link_page() is definitely the best solution. I've actually made confusion, because I was sure that that function was used also in other places, for this reason I avoided to edit it. In addition _wp_link_page() is used many times there and I missed it, so str_replace() was really a terrible solution. My bad, I apologize.

Just uploaded the new patch following your suggestions, only the unit tests are still missing.

Last edited 6 months ago by antonioeatgoat (previous) (diff)

@afercia
6 months ago

#9 @afercia
6 months ago

41859.diff updates the related tests.

@afercia
5 months ago

#10 @afercia
5 months ago

Thinking at slightly better CSS class names. For analogy with the paginated links for the archive pages ("nav-links" and "page-numbers ") I'd say to use "post-nav-links" and "post-page-number/s". Updated in 41859.2.diff.

#11 @afercia
5 months ago

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

In 42440:

Accessibility: use aria-current for the paginated post links output by wp_link_pages().

Continues the introduction in core of the aria-current attribute after [41683], [41359], and [41371].

  • changes the wp_link_pages() (see the nextpage quicktag) output to use an aria-current attribute on the current item
  • adds post-nav-links and post-page-numbers CSS classes to help themes style these links
  • updates the related tests

Props antonioeatgoat, alexstine.
Fixes #41859.

Note: See TracTickets for help on using tickets.