WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#30152 closed enhancement (fixed)

Twenty Fifteen: archive pagination links accessibility

Reported by: afercia Owned by: iandstewart
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: Bundled Theme Keywords: needs-patch
Focuses: ui, accessibility Cc:

Description

Archive pagination links have the text "Page" hidden with display: none so it's "hidden" also for screen readers.
I would suggest to make that text available to screen readers and hide it only visually. Working on a patch.
Here's how the links look on desktop and mobile:

https://cldup.com/opKQoeFx4B.png

Suppose you're in page 2 of 5, screen readers will announce:

PREVIOUS (link)
1 (link) 3 (link) 4 (link) 5 (link)
NEXT (link)

Which is not clear, "1"... what? Would be far better this way:

PREVIOUS page (link)
page 1 (link) page 3 (link) page 4 (link) page 5 (link)
NEXT page (link)

And on small screens (screen readers are used on mobile devices too):

PREVIOUS page (link)
NEXT page (link)

On mobile though, the current page "Page" text is the only one displayed (and announced).
I'm considering to give just to screen reader users the ability to use also the hidden links.

Will try to take care of the uppercase thing too.

Attachments (8)

30152.patch (3.0 KB) - added by afercia 6 years ago.
30152.2.patch (3.5 KB) - added by afercia 6 years ago.
Updated patch with appropriate heading text for search results pagination
30152.3.patch (3.9 KB) - added by afercia 6 years ago.
Updated patch for the new core template tags
30152.4.patch (3.9 KB) - added by afercia 6 years ago.
30152.5.patch (4.3 KB) - added by iamtakashi 6 years ago.
Updated patch
30152.6.patch (1.2 KB) - added by SergeyBiryukov 6 years ago.
30152.7.patch (1.4 KB) - added by SergeyBiryukov 6 years ago.
30152.8.patch (1.2 KB) - added by SergeyBiryukov 6 years ago.

Download all attachments as: .zip

Change History (29)

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


6 years ago

@afercia
6 years ago

#2 follow-up: @afercia
6 years ago

  • Keywords has-patch needs-testing added

After some investigation I'd propose to:

  • keep it simple
  • serve all users the same content

Proposed patch just makes the "Page" text available to screen readers and adds it where needed to make things more clear. Tries to manage the "uppercase thing" where possible, without changing the theme appearance.

Small change also for the Next/Previous post navigation, to make screen readers read out:

  • "Previous post (pause) {title}"
  • "Next post (pause) {title}"

Strings are a bit long now and with a lot of HTML exposed to translators... improvements and updated patch welcome.

Please notice that with many pagination links, the "dots" … will be displayed, this can be an accessibility issue since there's no information provided about the "jump" and dots won't be read out when tabbing through links.
NVDA reads dots just when reading letter by letter (right/left arrow keys) and will read "dot dot dot". Btw this should be managed in core in paginate_links(). Accessibility team feedback needed :) See screenshot below.

A note on the way to reset the "screen-reader-text" rule: as "Page" is inside an inline element, width, height, and overflow don't apply. The "clip" property applies only to absolutely positioned element so all we need here is a switch between position: static and position: absolute.

Needs testing :)

dot dot dot:
https://cldup.com/NTXduNWP7n.png

@afercia
6 years ago

Updated patch with appropriate heading text for search results pagination

This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.


6 years ago

#4 @iandstewart
6 years ago

@afercia can you test and update with the updated pagination?

@afercia
6 years ago

Updated patch for the new core template tags

#5 @afercia
6 years ago

Accessibility team, anyone interested: please test :)
Outstanding issues:

  • with the new core template tags the heading can't be changed or filtered so it's impossible to have, for example, "Search results navigation" as heading in the search. With custom post types (say "Products") it will still be "Post navigation". Maybe this deserves a separate ticket.
  • the "jump" between page links, visually represented with "dot dot dot", doesn't give any useful information... will users understand why after "Page 1 link" they hear, say, "Page 14 link"?
  • the current page item should have some special text to announce it's... current :)

@afercia
6 years ago

#6 @afercia
6 years ago

Please forget patch .3: prev and next text had to be switched, done in .4

Last edited 6 years ago by afercia (previous) (diff)

This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.


6 years ago

@iamtakashi
6 years ago

Updated patch

#8 @iamtakashi
6 years ago

The patch was out of sync so I updated and adjusted a few things that I've noticed.

#9 @davidakennedy
6 years ago

  • Keywords needs-testing removed

I've tested this and everything works as expected from an accessibility perspective.

#10 @iandstewart
6 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.1

#11 @iandstewart
6 years ago

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

In 30316:

Twenty Fifteen: adding context to pagination links for screenreaders.

Props afercia, iamtakashi, fixes #30152

#12 in reply to: ↑ 2 @SergeyBiryukov
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to afercia:

Small change also for the Next/Previous post navigation, to make screen readers read out:

  • "Previous post (pause) {title}"
  • "Next post (pause) {title}"

Strings are a bit long now and with a lot of HTML exposed to translators... improvements and updated patch welcome.

The problem is that we're trying to merge two different strings here, a standalone "Next" and "Next post". Some languages would want to translate the "Next" differently in each case (e.g. Next/Previous becomes Forward/Back).

30152.6.patch is an attempt to clean things up a bit:

  • Uses the existing Next/Previous strings from the slider.
  • Adds context for the 'post:' string, so that a reworded 'next' or 'previous' could be added in the translation.
  • Removes extra HTML from the strings.
  • Should be read out the same way it does now.
  • Should work with RTL, as it does not affect text direction.

30152.7.patch is another iteration which keeps the placeholders just in case.

#13 @afercia
6 years ago

Tested both last patches with Firefox + NVDA and yes they're read out the same way, NVDA is happy. Thanks very much Sergey.

Wondering if something similar to what was done for the Publish postbox in the admin would be a better solution:

<span aria-hidden="true">Edit</span>
<span class="screen-reader-text">Edit status</span>

#14 @SergeyBiryukov
6 years ago

Oh yes, that would be better. I was looking for a way to hide the "Next" string from screen readers, and forgot about aria-hidden. I'll update the patch in a couple of hours.

#15 @SergeyBiryukov
6 years ago

30152.8.patch is the updated patch that uses aria-hidden.

#16 @afercia
6 years ago

Thanks very much Sergey, tested 30152.8 and tested again also index, archives and search results pagination. Seems all is fine. For the records, here's how Firefox + NVDA read out the_post_navigation():

navigation landmark
Post navigation
heading level 2

Previous post:
Markup: Image Alignment
visited link

Next post:
Hello world!
visited link

#17 @lancewillett
6 years ago

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

In 30834:

Twenty Fifteen: update archive pagination links to use aria-hidden for better accessibility.

Props SergeyBiryukov, fixes #30152.

#18 @netweb
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The twentyfifteen.pot needs updating after r30834

#19 @netweb
6 years ago

  • Keywords needs-patch added; has-patch commit removed

This ticket was mentioned in Slack in #core-themes by netweb. View the logs.


6 years ago

#21 @ocean90
6 years ago

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

In 30845:

Twenty Fifteen: Update the .pot file for [30834].

fixes #30152.

Note: See TracTickets for help on using tickets.